# Deep Review: 20260327-123931-pr-248

| | |
|---|---|
| **Date** | 2026-03-27 12:39 |
| **Repo** | [rancher-sandbox/rancher-desktop-daemon](https://github.com/rancher-sandbox/rancher-desktop-daemon) |
| **Round** | 3 |
| **Author** | [@Nino-K](https://github.com/Nino-K) |
| **PR** | [#248](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/248) — App controller Stop and Start LimaVM |
| **Branch** | `app-controller-stop-start-limavm` |
| **Commits** | `aadeeb4` Fix ObservedGeneration in App Controller<br>`1c4a636` Add additional BATs for App Controller<br>`904a42c` Update api_app.md to reflect condition definition<br>`5c50ee6` Prevent Running condition cycle when VM is stopped |
| **Reviewers** | Claude Opus 4.6, Codex GPT 5.4, Gemini 3.1 Pro |
| **Verdict** | **Merge with fixes** — missing regression test for the restart-path fix; unresolved reviewer feedback on BATS patterns |
| **Wall-clock time** | `21 min 9 s` |

---

## Executive Summary

This PR wires `App.spec.running` through to `LimaVM.spec.running`, mirrors the LimaVM `Running` condition back onto the App, updates the API design doc, and fixes a condition oscillation bug when the VM is stopped. The controller changes are correct and well-targeted. The main gap: no test exercises the controller-restart path that commit `5c50ee6` specifically fixes, so that regression could return undetected.

---

## Critical Issues

None.

---

## Important Issues

1. **New tests do not cover the controller-restart regression** — `bats/tests/32-app-controllers/app.bats:157-209` [Codex GPT 5.4] (important, gap)

```bash
@test "wait for LimaVM Running condition to become true after start" {
    rdd ctl wait --for=condition=Running \
        limavm/"${VM_NAME}" --namespace "${RDD_NAMESPACE}" --timeout=120s
}
```

The new tests verify steady-state start/stop mirroring but never recreate the condition that commit `5c50ee6` fixes: `phase == phaseUnknown` after a controller restart with `spec.running=false`. Without exercising that path, a future refactor could remove the `shouldRun` guard at `limavm_lifecycle.go:131` while all tests still pass.

Fix: add a test that stops the VM, restarts the service (`rdd svc restart`), then asserts that `Running` stays `False/Stopped` instead of flipping to `Unknown/Reconciling`.

---

## Suggestions

1. **Misleading commit message on `aadeeb4`** — `pkg/controllers/app/app/controllers/app_controller.go` [Claude Opus 4.6] (suggestion, regression)

```diff
-    log.Error(cmErr, "Failed to fetch input ConfigMap during cleanup")
+    return ctrl.Result{}, fmt.Errorf("failed to fetch input ConfigMap during cleanup: %w", cmErr)
```

The commit title says "Fix ObservedGeneration in App Controller," but the diff only converts two `log.Error` calls to `return error`. `ObservedGeneration` was already `cond.ObservedGeneration` at the merge base and this commit did not change it. The message body compounds the mismatch: "The LimaVM's `ObservedGeneration` is now correctly propagated."

Fix: amend the commit message to describe the actual change, e.g., "Return errors from ConfigMap Get failures instead of logging and continuing."

2. **Doc says "in the app-namespace" for a cluster-scoped resource** — `docs/design/api_app.md:11` [Codex GPT 5.4, Gemini 3.1 Pro] (suggestion, regression)

Line 9 clarifies that `App` is cluster-scoped, but line 11 says the CLI creates it "in the app-namespace." Cluster-scoped resources do not belong to a namespace. The configuration setting controls `spec.namespace`, which dictates where child resources are created.

Fix:
```diff
-create the `App` object in the "app-namespace", which is a configuration setting
+create the cluster-scoped `App` object, setting its `spec.namespace` to the configured "app-namespace"
```

3. **BATS tests use `$(jq_output ...)` instead of `run -0 jq_output`** — `bats/tests/32-app-controllers/app.bats:175,179,187,189` [Claude Opus 4.6, Gemini 3.1 Pro] (suggestion, regression)

Four places extract values via `$(jq_output ...)` command substitution. When `jq_output` fails inside `$(...)`, BATS produces an opaque error rather than showing the actual output. This echoes Jan's unresolved review feedback.

Fix: use `run -0 jq_output` to capture each value, matching the pattern Jan suggested:
```diff
-    limavm_ts=$(jq_output '.status.conditions[] | select(.type == "Running") | .lastTransitionTime')
+    run -0 jq_output '.status.conditions[] | select(.type == "Running") | .lastTransitionTime'
+    local limavm_ts=$output
```

Apply the same pattern to lines 179, 187, and 189. For the `observedGeneration` test, replace the `[[ ]]` comparison with `assert_output` for better failure diagnostics.

---

## Design Observations

### Strengths

- **Oscillation fix is minimal and precisely targeted.** A single `shouldRun &&` guard at `limavm_lifecycle.go:131` breaks the `False/Stopped` → `Unknown/Reconciling` → `False/Stopped` cycle without touching other code paths. The comment explains the mechanism clearly, including why the App controller's `Owns()` watch amplifies each status update into a re-enqueue. [Claude Opus 4.6, Codex GPT 5.4, Gemini 3.1 Pro]

- **Error handling promotion is correct.** Converting the two `log.Error`-and-continue paths to `return error` prevents the reconciler from proceeding with unreliable state. In the cleanup path, this avoids removing the finalizer when the ConfigMap's existence is unconfirmed. [Claude Opus 4.6, Gemini 3.1 Pro]

- **`lastTransitionTime` mirroring preserves the LimaVM's actual transition timestamp** rather than replacing it with the App controller's reconcile time. This keeps the condition meaningful for staleness checks and debugging. [Codex GPT 5.4]

- **API doc revision is thorough.** Documenting that conditions are mirrored (not independently set), that `lastTransitionTime` reflects the LimaVM's timeline, and that `spec.namespace` is immutable after creation gives future developers the right mental model. [Claude Opus 4.6]

---

## Testing Assessment

The new tests cover the end-to-end start/stop lifecycle well: Running condition mirrored as True after start, timestamp preservation, observedGeneration semantics, and Running condition mirrored as False after stop. Untested scenarios, ranked by risk:

1. **Controller-restart regression** — The fix at `limavm_lifecycle.go:131` prevents oscillation when the controller restarts with a stopped VM. No test exercises this path; a future refactor could remove the guard undetected.
2. **Transitional condition mirroring** — Tests wait for final states (`True/Started`, `False/Stopped`) but do not verify intermediate conditions (`Unknown/Reconciling`, `False/Starting`) are mirrored. Harder to capture reliably in BATS; lower risk.

---

## Documentation Assessment

The `api_app.md` updates document mirrored condition semantics, the condition table, and example YAML. Two gaps:

- Line 11's "in the app-namespace" wording conflicts with the cluster-scoped clarification at line 9 (see Suggestion 2).
- `api_lima.md` does not document the `Running=Unknown, Reason=Reconciling` state that `api_app.md:99` now references as mirrored from LimaVM.

---

## Commit Structure

Commit `aadeeb4` ("Fix ObservedGeneration in App Controller") has a message that does not match its diff. The diff changes error handling; ObservedGeneration was not modified. This will confuse `git log` and `git bisect` users. The message should describe the actual change.

---

## Unresolved Feedback

- **[Comment 2](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/248#discussion_r2997145382)** (@jandubois) — Use `run -0 jq_output` + `local var=$output` instead of `$(jq_output ...)`. Not addressed by subsequent commits.
- **[Comment 3](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/248#discussion_r2997152488)** (@jandubois) — Use `run -0 jq_output` + `assert_output` instead of manual `[[ ]]` comparison. Not addressed.

---

## Agent Performance Retro

### [Claude] Opus 4.6

- **Duration**: 6 min 12 s
- **Unique contributions**: Caught the misleading commit message on `aadeeb4` — the only agent to verify the commit message against the actual diff.
- **Accuracy**: No false positives. All findings verified against the code.
- **Depth**: Used git blame and traced into callers. Correctly identified the error handling change. Read the API design doc updates in context.
- **Coverage**: Reviewed all 4 files. No coverage misses.
- **Signal-to-noise**: High. Two actionable suggestions, clear unresolved feedback tracking. No filler findings.

### [Codex] GPT 5.4

- **Duration**: 3 min 21 s
- **Unique contributions**: Raised the controller-restart regression test gap as IMPORTANT — the only agent to flag that the tests don't exercise the specific path commit `5c50ee6` fixes.
- **Accuracy**: No false positives. The restart-path gap is genuine.
- **Depth**: Traced into `limavm_lifecycle.go:131` to understand the `shouldRun` guard. Identified the `api_lima.md` documentation gap (missed by other agents).
- **Coverage**: Reviewed all 4 files. No coverage misses.
- **Signal-to-noise**: Highest. Concise review, one substantive important finding, clean design observations.

### [Gemini] 3.1 Pro

- **Duration**: 3 min 49 s
- **Unique contributions**: None — all findings were also raised by at least one other agent.
- **Accuracy**: No false positives. The doc wording finding and unresolved feedback are both valid.
- **Depth**: Read the diff and traced into the controller. Correctly identified the oscillation fix mechanism. Did not use git blame or trace commit-level diffs.
- **Coverage**: Reviewed all 4 files. No coverage misses.
- **Signal-to-noise**: Moderate. One suggestion and unresolved feedback tracking, but no unique insights. Found no important issues.

### Summary

| Metric | Claude Opus 4.6 | Codex GPT 5.4 | Gemini 3.1 Pro |
|--------|-----------------|----------------|----------------|
| Duration | 6:12 | 3:21 | 3:49 |
| Critical | 0 | 0 | 0 |
| Important | 0 | 1 | 0 |
| Suggestion | 2 | 0 | 1 |
| Design observations | 4 | 2 | 2 |
| False positives | 0 | 0 | 0 |
| Unique insights | 1 | 1 | 0 |
| Files reviewed | 4 | 4 | 4 |
| Coverage misses | 0 | 0 | 0 |

Codex delivered the most value this round: it raised the only important finding (missing regression test) in the shortest time. Claude provided complementary depth with the commit message mismatch catch. Gemini confirmed existing findings but contributed no unique signal.

---

## Appendix: Original Reviews

### Claude Opus 4.6

I have all the context I need. Let me compile the review.

---

## Executive Summary

This PR adds BATS integration tests for Running condition mirroring between App and LimaVM, fixes a condition oscillation bug in the LimaVM lifecycle controller when `shouldRun=false`, corrects silent error swallowing in the App controller's ConfigMap Get paths, and updates the App API design doc to document mirrored condition semantics. The changes are sound and well-targeted; the key lifecycle fix is correct and clearly documented.

**Verdict: Approve with minor suggestions**

---

## Findings

### Critical Issues

None.

### Important Issues

None.

### Suggestions

1. **Misleading commit message** — `aadeeb4` (suggestion, regression)

   The commit is titled "Fix ObservedGeneration in App Controller" but the diff only changes error handling — converting two `log.Error` calls to `return error`. `ObservedGeneration` was already `app.Generation` at the merge base (line 193 of `app_controller.go`) and was not modified by this commit. The commit body also claims "The LimaVM's `ObservedGeneration` is now correctly propagated" which contradicts the actual code change.

   Fix: Amend the commit message to describe what the diff actually does, e.g., "Return errors from ConfigMap Get failures instead of logging and continuing."

2. **BATS tests use `$(jq_output ...)` instead of `run -0 jq_output`** — `bats/tests/32-app-controllers/app.bats:175,179,187,189` (suggestion, regression)

   Four places extract values via `$(jq_output ...)` command substitution instead of the `run -0 jq_output ...; local var=$output` pattern used elsewhere in the file (e.g., line 168). When `jq_output` fails inside `$(...)`, BATS produces an opaque error message (just "command failed") rather than showing the actual output. This is the same feedback Jan gave in PR comments 2 and 3.

   ```bash
   # Current (line 175):
   limavm_ts=$(jq_output '.status.conditions[] | select(.type == "Running") | .lastTransitionTime')
   ```

   Fix: Use `run -0 jq_output` to capture the value, matching Jan's suggestion:

   ```diff
   -    limavm_ts=$(jq_output '.status.conditions[] | select(.type == "Running") | .lastTransitionTime')
   +    run -0 jq_output '.status.conditions[] | select(.type == "Running") | .lastTransitionTime'
   +    local limavm_ts=$output
   ```

   Apply the same pattern to lines 179, 187, and 189. For the observedGeneration test (lines 184–191), the `[[ ]]` comparison can also be replaced with `run -0 jq_output` + `assert_output` for better failure diagnostics:

   ```diff
   -    local cond_gen
   -    cond_gen=$(jq_output '.status.conditions[] | select(.type == "Running") | .observedGeneration')
   -
   -    [[ "${cond_gen}" == "${app_gen}" ]]
   +    run -0 jq_output '.status.conditions[] | select(.type == "Running") | .observedGeneration'
   +    assert_output "${app_gen}"
   ```

---

## Design Observations

### Strengths

- **The oscillation fix is minimal and precisely targeted.** Adding a single `shouldRun &&` guard at `limavm_lifecycle.go:131` breaks the False/Stopped → Unknown/Reconciling → False/Stopped cycle without changing any other code path. The comment explains the exact mechanism clearly, including why the guard is necessary (App controller's `Owns()` watch amplifies each status update into a re-enqueue).

- **Error handling promotion is correct.** Converting the two `log.Error`-and-continue paths to `return error` (lines 70 and 168 of `app_controller.go`) prevents the reconciler from proceeding with unreliable state. In the cleanup path, this avoids removing the finalizer when we can't confirm the ConfigMap's existence. In the normal path, this prevents spec propagation during transient failures.

- **The API doc revision is thorough.** Documenting that conditions are mirrored (not independently set), that `lastTransitionTime` reflects the LimaVM's timeline, and that `spec.namespace` is immutable after creation gives future developers the right mental model.

---

## Testing Assessment

The new tests cover the end-to-end start/stop lifecycle well: Running condition mirrored as True after start, timestamp preservation, observedGeneration semantics, and Running condition mirrored as False after stop. Specific untested scenarios, ranked by risk:

1. **Condition oscillation regression** — The fix prevents the cycle, but there is no test that detects it if the guard is accidentally removed. The existing tests pass implicitly (they don't time out), but an explicit stability assertion (e.g., verify no condition flapping over N seconds) would catch regressions directly. However, this is difficult to express in BATS and the current coverage is pragmatic.

2. **Transitional condition mirroring** — The tests wait for final states (True/Started, False/Stopped) but don't verify intermediate conditions (Unknown/Reconciling, False/Starting) are mirrored. These are harder to capture reliably in integration tests and are lower risk.

---

## Documentation Assessment

The `api_app.md` updates are accurate and complete. The condition table now matches the LimaVM condition table, the mirroring semantics are explained, and the example YAML reflects the actual singleton naming (`name: app`, cluster-scoped).

---

## Commit Structure

The commit `aadeeb4` ("Fix ObservedGeneration in App Controller") has a message that does not match its diff. The diff changes error handling; ObservedGeneration was untouched. This will confuse future readers doing `git log` or `git bisect`. Consider amending the message to describe the actual change.

---

## Unresolved Feedback

- **[Comment 2](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/248#discussion_r2997145382)** — Jan suggests using `run -0 jq_output` + `local var=$output` instead of `$(jq_output ...)`. Not addressed by any subsequent commit.

- **[Comment 3](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/248#discussion_r2997152488)** — Jan suggests using `run -0 jq_output` + `assert_output` instead of manual extraction and `[[ ]]` comparison. Not addressed.

---

## Coverage Summary

| File | Status |
|------|--------|
| `bats/tests/32-app-controllers/app.bats` | **Finding 2** — style suggestion for `jq_output` pattern |
| `docs/design/api_app.md` | **Reviewed, no issues** |
| `pkg/controllers/app/app/controllers/app_controller.go` | **Reviewed, no issues** — error handling fix is correct |
| `pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go` | **Reviewed, no issues** — oscillation fix is correct and well-documented |

### Codex GPT 5.4

**Verdict**

Needs changes.

**Executive Summary**

This PR extends the App controller so `App.spec.running` drives `LimaVM.spec.running`, mirrors the LimaVM `Running` condition back onto `App.status.conditions`, and updates the App API design doc to match that behavior. The controller changes themselves look sound in context, and the targeted App BATS suite passed locally, but the new tests still do not cover the exact controller-restart path fixed by `5c50ee6`, so the regression this PR is fixing could return unnoticed.

**Findings**

**Critical Issues**

None.

**Important Issues**

1. **The new integration coverage misses the controller-restart regression that `5c50ee6` fixes** — `bats/tests/32-app-controllers/app.bats:157-209` (important, gap)

```bash
@test "wait for LimaVM Running condition to become true after start" {
    rdd ctl wait --for=condition=Running \
        limavm/"${VM_NAME}" --namespace "${RDD_NAMESPACE}" --timeout=120s
}

@test "wait for LimaVM Running condition to become false after stop" {
    rdd ctl wait --for=condition=Running=False \
        limavm/"${VM_NAME}" --namespace "${RDD_NAMESPACE}" --timeout=60s
}
```

The new tests at lines 157-209 verify steady-state start/stop mirroring, but they never recreate the condition that commit `5c50ee6` changed in `pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go:131`: `phase == phaseUnknown` after a controller restart with `spec.running=false`. That restart path is the whole reason for the new `shouldRun` guard at line 131, and without exercising it the PR still has no regression test for the bug it claims to fix. A future refactor could remove or break that guard while all new tests still pass.

Fix: add an integration test that stops the VM, restarts the relevant controller or full service, then verifies `limavm.status.conditions[Running]` and `app.status.conditions[Running]` stay `False/Stopped` instead of flipping to `Unknown/Reconciling`. Using `rdd svc restart` or a control-plane restart helper would give direct coverage of the fixed path.

**Suggestions**

None.

**Design Observations**

**Strengths**

- The `shouldRun` guard in [`pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go:131`](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go:131) is the right place to break the App-owned watch feedback loop. It fixes the oscillation at the source instead of adding special handling in the App controller.
- Mirroring `lastTransitionTime` from the child condition in [`pkg/controllers/app/app/controllers/app_controller.go:188`](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/app/app/controllers/app_controller.go:188) preserves the LimaVM’s actual transition timestamp, which keeps the App condition meaningful for staleness/debugging rather than turning it into “last copied by parent.”

**Testing Assessment**

1. The highest-risk missing case is a real controller restart while the VM is already stopped. That is the path changed at [`pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go:131`](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go:131), and it is not exercised by [`bats/tests/32-app-controllers/app.bats:157`](/Users/jan/git/rancher-desktop-daemon/bats/tests/32-app-controllers/app.bats:157) through [`bats/tests/32-app-controllers/app.bats:209`](/Users/jan/git/rancher-desktop-daemon/bats/tests/32-app-controllers/app.bats:209).
2. There is still no App-level assertion for the transient restart case where LimaVM reports `Running=Unknown/Reconciling` after service restart with `spec.running=true`. The doc now describes that state, but the App controller’s mirroring of it is untested.

Local verification: `bats/lib/bats-core/bin/bats bats/tests/32-app-controllers/app.bats` passed.

**Documentation Assessment**

[`docs/design/api_app.md:11`](/Users/jan/git/rancher-desktop-daemon/docs/design/api_app.md:11) still says the CLI and GUI create the `App` object “in the app-namespace,” which now conflicts with the clarified statement at line 9 that `App` is cluster-scoped. That sentence should be rewritten to say the App is cluster-scoped and that `spec.namespace` controls where owned namespaced resources are created.

[`docs/design/api_lima.md`](/Users/jan/git/rancher-desktop-daemon/docs/design/api_lima.md) also still omits the `Running=Unknown, Reason=Reconciling` state that [`docs/design/api_app.md:99`](/Users/jan/git/rancher-desktop-daemon/docs/design/api_app.md:99) now documents as mirrored from LimaVM.

**Acknowledged Limitations**

- “`TODO: Non-blocking stop: send SIGINT and return immediately; the watcher detects the Exiting event and triggers a reconcile.`” — [`pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go:451`](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go:451). This PR does not make that limitation worse; the new stop/start wiring just makes the blocking stop path reachable from the App controller as well as direct LimaVM edits.

**Coverage Summary**

- `bats/tests/32-app-controllers/app.bats` — Finding 1
- `docs/design/api_app.md` — Reviewed, no issues
- `pkg/controllers/app/app/controllers/app_controller.go` — Reviewed, no issues
- `pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go` — Reviewed, no issues

### Gemini 3.1 Pro

### Executive Summary

This PR correctly implements start/stop control and condition mirroring from the `LimaVM` resource back to the `App` singleton. The changes include proper error propagation in the App controller, a critical guard in the LimaVM lifecycle to prevent infinite reconcile loops when stopping unwatched VMs, and thorough test coverage for the mirrored state. The implementation is robust, with only a minor documentation gap and some testing patterns remaining to be addressed.

### Findings

#### Critical Issues
None.

#### Important Issues
None.

#### Suggestions

1. **Misleading cluster-scoped resource documentation** — `docs/design/api_app.md:11`

The documentation clarifies that the `App` object is cluster-scoped. However, the next sentence states that the `App` object is created "in the app-namespace". Cluster-scoped resources do not belong to a namespace. The configuration setting actually controls the `spec.namespace` field, which dictates where the `App` creates its child resources.

Fix: Update the wording to accurately describe how the namespace setting is applied.

```diff
-Both the [rdd create](cmd_app.md#rdd-create) command and the [GUI](gui.md) app create the `App` object in the "app-namespace", which is a configuration setting stored in the `config` ConfigMap in the `rdd-system` namespace (`rancher-desktop` by default)[^hardcoded].
+Both the [rdd create](cmd_app.md#rdd-create) command and the [GUI](gui.md) app create the cluster-scoped `App` object, setting its `spec.namespace` to the configured "app-namespace" (stored in the `config` ConfigMap in the `rdd-system` namespace, defaulting to `rancher-desktop`)[^hardcoded].
```

### Design Observations

#### Strengths
- **Reconcile Loop Oscillation Fix**: The addition of the `shouldRun` guard in `limavm_lifecycle.go` demonstrates a deep understanding of controller-runtime watch behavior. It correctly identifies that an unwatched, stopped VM is the expected state when `spec.running=false` and elegantly avoids the intermediate `Unknown/Reconciling` state that was causing a `False/Stopped` -> `Unknown/Reconciling` oscillation cycle triggered by the `App` controller's `Owns()` watcher.
- **Defensive Error Handling**: Replacing `log.Error()` with `return ctrl.Result{}, fmt.Errorf(...)` during `inputConfigMap` cleanup in `app_controller.go` correctly leverages the controller's backoff retry mechanism rather than silently dropping API failures and improperly advancing the cleanup sequence.

### Testing Assessment

The added BATS integration tests successfully cover both starting and stopping paths, including condition status, `lastTransitionTime`, and `observedGeneration` mirroring validation. 

### Unresolved Feedback

- **@jandubois on `bats/tests/32-app-controllers/app.bats` (lines 190-193)**: [Link](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/248#discussion_r2997152488)
  The reviewer suggested using `run -0 jq_output ...` followed by `assert_output "${limavm_gen}"`. The committed code uses `cond_gen=$(jq_output ...)` and `[[ "${cond_gen}" == "${app_gen}" ]]`. While functional, manual Bash comparisons fail silently, whereas `assert_output` provides explicit diffs in CI logs when tests fail.
- **@jandubois on `bats/tests/32-app-controllers/app.bats` (lines 174-175)**: [Link](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/248#discussion_r2997145382)
  The same manual comparison pattern (`[[ "${limavm_ts}" == "${app_ts}" ]]`) was used for the timestamp validation instead of capturing the first output and asserting against it during the second execution.

### Coverage Summary

- `bats/tests/32-app-controllers/app.bats` — Reviewed, unresolved feedback noted
- `docs/design/api_app.md` — Finding 1
- `pkg/controllers/app/app/controllers/app_controller.go` — Reviewed, no issues
- `pkg/controllers/lima/limavm/controllers/limavm_lifecycle.go` — Reviewed, no issues