flake: Camera2D current undo test still fails after #296

[dsarno]

Summary

camera.test_configure_current_sibling_unmark_single_undo is still flaky after #296. PR #300 hit the same Camera2D current-state undo symptom in Godot tests / macOS, even though PR #300 does not touch camera code and the same test matrix passed locally plus on Linux/Windows CI.

This looks like a recurrence of the camera-current race tracked in #140 and #278, which #296 closed by widening the settle window, checking Viewport.get_camera_2d() for Camera2D, and cleaning camera-suite fixtures per test.

New failing example

• PR: Harden lifecycle reliability (#297, PR 3) #300
• Run: https://github.com/hi-godot/godot-ai/actions/runs/25291900443
• Failed job: Godot tests / macOS job 74144847847
• Step: Run handler tests

Failure:

Godot tests: 1043/1060 passed, 1 failed
FAIL: camera.test_configure_current_sibling_unmark_single_undo: After undo second should not be current. viewport_cam=UndoSecond:<Camera2D#1720486792976> first_in_tree=true second_in_tree=true

Why this is probably not PR #300

PR #300 only touches lifecycle reliability code (connection.gd, dispatcher.gd, sessions/registry.py, error-code parity, and related tests). It does not modify camera_handler.gd or test_camera.gd.

Observed signal on the same head SHA:

• Local script/ci-godot-tests: 1045/1060 passed, 0 failed
• CI Godot tests / Linux: passed
• CI Godot tests / Windows: passed
• CI Godot tests / macOS: failed with the camera undo symptom above

Prior context

• flake: camera.test_configure_current_sibling_unmark_single_undo on macOS needs root-cause investigation #140 tracked the original macOS-only camera.test_configure_current_sibling_unmark_single_undo flake and noted that fix(camera): verify is_current after commit to close macOS headless race #120's _verify_current_after_commit covered the forward/apply side but not the undo-side viewport lag.
• Track flaky macOS Camera2D current undo test #278 tracked a newer recurrence on PR [codex] Fix self-update orphan server recovery #275, later broadened beyond macOS by a Windows failure.
• Stabilize camera current-state handling #296 closed Track flaky macOS Camera2D current undo test #278 by increasing the bounded settle budget, tightening Camera2D settled-state checks against Viewport.get_camera_2d(), and clearing current cameras during camera test teardown.

This PR #300 failure suggests #296 reduced but did not eliminate the undo-side race.

Suggested investigation

• Reproduce on macOS headless by looping camera.test_configure_current_sibling_unmark_single_undo against a post-Stabilize camera current-state handling #296 tree.
• Instrument the undo path around _apply_clear_current, _apply_make_current, _is_current_settled, and the test's post-undo assertion with per-attempt state: first.is_current(), second.is_current(), viewport.get_camera_2d(), and elapsed settle attempts.
• Check whether the undo action executes clear_current on Second and make_current on First in the expected order, and whether the viewport slot lags even after both bound methods return.
• Decide whether the production handler needs a stronger post-undo settle path, or whether the test should wait for a deterministic editor/viewport boundary before asserting.
• Keep this distinct from the separate reload-smoke camera.test_make_current_does_not_cross_classes flake; this occurrence is the sibling undo/current-state test.

Acceptance

• Root cause or stable mitigation documented for the post-Stabilize camera current-state handling #296 recurrence.
• The camera suite survives repeated macOS headless CI runs without this failure.
• If the final answer is an upstream Godot editor/viewport timing bug, file/link the upstream issue and mark any local mitigation as a workaround.

[dsarno]

I checked the linked run against latest origin/beta.

Baseline checked:

• origin/beta fetched on 2026-05-04
• beta HEAD: bdd546a455ea89d0c5b37e61ff76fb1edaaa7e66 (Harden lifecycle reliability (#297, PR 3) (#300))
• linked job: https://github.com/hi-godot/godot-ai/actions/runs/25297392425/job/74164056375

The linked job is not the exact macOS undo assertion from this issue body. It is a Windows reload-smoke failure after 10 plugin reloads:

FAIL: 1 test(s) failed after reload churn (passed=1037)
  FAIL: camera.test_get_returns_current_when_path_empty: Expected current, got first

That still looks like the same underlying class of problem: our camera tests and camera_get("") contract are still depending on immediate/near-immediate Godot Camera2D current-state convergence in headless editor conditions.

Latest beta already has the stronger workaround from #296-ish work:

• _CURRENT_SETTLE_ATTEMPTS := 8
• _CURRENT_SETTLE_DELAY_MSEC := 10
• _viewport_current_camera(scene_root) checked before walking cameras
• _is_effective_current(cam) using is_current() plus viewport camera identity
• 2D stale-camera displacement/nudge logic
• camera tests polling with OS.delay_msec(10) loops

Since the linked beta run still fell through camera_get("") to resolved_via = "first", the current approach is still probabilistic. Raising retry counts again is unlikely to be a deterministic fix.

Working theory:

Camera2D.make_current(), Camera2D.is_current(), and Viewport.get_camera_2d() are not giving us a synchronous, deterministic editor-test boundary after create/undo/reload in headless CI. The plugin and GDScript tests run on Godot's main thread. OS.delay_msec() blocks that thread; it does not guarantee the editor/viewport machinery has advanced to the state the assertion expects. That explains why the failures move between "after make_current", "after undo", and now "camera_get empty path after reload churn".

Recommended deterministic local fix:

1. Keep calling Godot make_current() / clear_current() for real editor behavior.
2. Add a handler-owned logical current marker for cameras, updated inside the same undo/redo action as the camera switch.
3. Use effective current resolution for MCP-facing reads: logical marker first, then Godot viewport/current state as fallback.
4. Make camera_get("") and camera_list report this effective current state instead of letting a transient Godot viewport lag return resolved_via = "first".
5. Keep one narrow diagnostic/integration test that observes raw Godot viewport convergence, but do not let the whole handler suite depend on synchronous Camera2D.is_current() convergence after create/undo/reload.

The important distinction: the product/tool contract is "the camera this plugin just made current is the camera camera_get("") resolves as current." We can make that deterministic in our handler state. The raw Godot viewport state is still best-effort synced, but it should not be the only source of truth for MCP semantics in headless editor tests.

This would also turn future failures into clearer diagnostics: if logical current says GetCurrent but viewport still says another camera, that is an upstream/editor timing observation; if logical current itself is wrong, that is our undo/action bookkeeping bug.