Skip to content

Commit 65c743a

Browse files
dsarnoclaude
andauthored
Dev-section follow-ups: Restarting… feedback + consolidate buttons + parent-walk (#408)
* Show "Restarting…" feedback while dev-mode restart is in flight Live smoke uncovered a 5s editor freeze (from _wait_for_port_free) with no acknowledgement of the click — the user wonders if the button did anything. Mirror the existing _crash_restart_btn / _version_restart_btn pattern: set _server_restart_in_progress on click, dispatch via call_deferred + a 0.15s paint window, then clear the flag after the spawn lands. _update_dev_section_buttons checks the flag and renders "Restarting…" + disabled while in flight. Test path is non-tree, so _on_dev_restart_pressed runs synchronously when is_inside_tree() is false — keeps the existing dispatch test green without needing await scaffolding. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Consolidate dev-section to one Restart Dev Server + small ✕ stop Per PR review feedback: two restart-flavored buttons in the same section felt redundant, and the new Restart Server stayed disabled when only an adopted external dev server was running because brand detection misses orphaned multiprocessing.spawn workers. Replaces both _dev_server_btn (3-state Switch/Exit/Start toggle) and _dev_restart_btn (kill+respawn preserving mode) with a single primary button + a small stop affordance: - _dev_primary_btn — full-width "Restart Dev Server" / "Start Dev Server" (label adapts to whether something's running). Always enabled. Click → force_restart_or_start_dev_server which kills any godot-ai server (managed or dev), waits for the port to free, and lands in --reload mode. Same "Restarting…" feedback during the blocking wait. - _dev_stop_btn — compact "✕" beside the primary. Enabled only when a dev server is actually running. Click → stop_dev_server (existing). Intentionally never targets the managed server (lifecycle owns that). Plugin's force_restart_server_preserving_mode is renamed/retargeted as force_restart_or_start_dev_server. The previous "preserve managed mode" semantics drop — clicking Restart now always lands in dev mode, since that's the only mode contributors editing Python source actually want. The lifecycle's force_restart_server stays for the version-drift / crash recovery paths that legitimately preserve managed mode. One verb, one mental model. Adapted tests cover the truth table. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Make force_restart_or_start_dev_server cover orphan workers Previous version branched on has_managed_server / is_dev_server_running, both of which miss orphaned multiprocessing.spawn uvicorn workers — when the parent reloader dies (or the editor is restarted while the worker keeps holding the port), the worker's cmdline is just "spawn_main" without godot_ai branding, so neither detection path fires. Result: clicking Restart skipped the kill and went straight to start_dev_server, which then failed to bind :8000. Switch the kill arm's condition to _is_port_in_use(port). The user clicking Restart is explicit consent to take over the port, so killing whoever's there (regardless of brand) is the right semantics. Lifecycle state still resets when has_managed_server() so the next adoption pass sees a clean slate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Three follow-ups from live PR #406 review 1. Fix Restart/Start label bug. After clicking the primary button, the newly-spawned --reload server's LISTEN holder is uvicorn's `multiprocessing.spawn` worker — that PID's cmdline doesn't contain the godot_ai brand, so `is_dev_server_running()` returned false and the label stayed "Start Dev Server" instead of flipping to "Restart Dev Server" once the server was up. Walk up the parent chain (bounded depth 5) inside `_pid_cmdline_is_godot_ai` so a worker whose parent reloader IS branded gets recognised. Adds `_pid_parent` helper covering POSIX (`ps -o ppid=`) and Windows (PowerShell CIM). 2. Drop the dock's "Mode override" dropdown. The dropdown advertised toggling between the github-repo local plugin and the installed ZIP version, but it didn't actually do that — it just nudged `is_dev_checkout()` for the update banner, which is more usefully driven by the GODOT_AI_MODE env var (still supported, still tested in test_clients.gd). Removes ~50 lines of dead UI plumbing and the stale "fresh check paints over a clean slate" comment. 3. Delete the orphan live_demo test file from a debugging session that never landed cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ae3f769 commit 65c743a

4 files changed

Lines changed: 318 additions & 287 deletions

File tree

plugin/addons/godot_ai/mcp_dock.gd

Lines changed: 92 additions & 164 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,6 @@ const LogViewerScript := preload("res://addons/godot_ai/dock_panels/log_viewer.g
3939
const PortPickerPanelScript := preload("res://addons/godot_ai/dock_panels/port_picker_panel.gd")
4040

4141
const DEV_MODE_SETTING := "godot_ai/dev_mode"
42-
## Index ↔ persisted-value mapping for the mode-override dropdown. The array
43-
## index is the OptionButton item id; the string is what's written to the
44-
## EditorSetting and read by `ClientConfigurator.mode_override()`.
45-
const MODE_OVERRIDE_VALUES := ["", "user", "dev"]
46-
const MODE_OVERRIDE_LABELS := ["Auto", "Force user", "Force dev"]
4742
const CLIENT_STATUS_REFRESH_COOLDOWN_MSEC := 15 * 1000
4843
const CLIENT_STATUS_REFRESH_TIMEOUT_MSEC := 30 * 1000
4944
static var COLOR_MUTED := Color(0.7, 0.7, 0.7)
@@ -170,14 +165,17 @@ var _client_action_generations: Dictionary = {}
170165
var _dev_section: VBoxContainer
171166
var _server_label: Label
172167
var _reload_btn: Button
173-
var _mode_override_btn: OptionButton
174168
var _setup_section: VBoxContainer
175169
var _setup_container: VBoxContainer
176-
var _dev_server_btn: Button
177-
## Same-version Python edits get adopted as compatible, so neither the
178-
## drift nor the crash Restart button surfaces — this is the unconditional
179-
## kick contributors need to pick up source changes without a version bump.
180-
var _dev_restart_btn: Button
170+
## Primary dev-section button — always (re)starts a `--reload` dev server.
171+
## Same-version Python edits get adopted as compatible by the lifecycle, so
172+
## neither the drift nor the crash Restart button surfaces; this is the
173+
## unconditional kick contributors need to pick up source changes without
174+
## a version bump.
175+
var _dev_primary_btn: Button
176+
## Small "✕" affordance next to the primary — stops the dev server without
177+
## spawning a replacement. Disabled when no dev server is running.
178+
var _dev_stop_btn: Button
181179
var _log_viewer: LogViewerScript
182180

183181
var _last_connected := false
@@ -527,23 +525,6 @@ func _build_ui() -> void:
527525

528526
_dev_section.add_child(btn_row)
529527

530-
# Dev-only override for testing the update-banner flow; persisted via EditorSettings.
531-
var mode_row := HBoxContainer.new()
532-
mode_row.add_theme_constant_override("separation", 6)
533-
var mode_label := Label.new()
534-
mode_label.text = "Mode override"
535-
mode_label.tooltip_text = "Force dev or user mode for testing the update flow. Normally leave on Auto. GODOT_AI_MODE env var is the fallback when this is Auto."
536-
mode_row.add_child(mode_label)
537-
_mode_override_btn = OptionButton.new()
538-
_mode_override_btn.size_flags_horizontal = Control.SIZE_EXPAND_FILL
539-
for i in MODE_OVERRIDE_LABELS.size():
540-
_mode_override_btn.add_item(MODE_OVERRIDE_LABELS[i], i)
541-
_mode_override_btn.tooltip_text = mode_label.tooltip_text
542-
_mode_override_btn.select(_mode_override_index_from_setting())
543-
_mode_override_btn.item_selected.connect(_on_mode_override_selected)
544-
mode_row.add_child(_mode_override_btn)
545-
_dev_section.add_child(mode_row)
546-
547528
# --- Setup section (dev-only or when uv missing) ---
548529
_setup_section = VBoxContainer.new()
549530
_setup_section.add_theme_constant_override("separation", 6)
@@ -1062,50 +1043,6 @@ func _apply_dev_mode_visibility() -> void:
10621043
_setup_section.visible = dev or uv_missing
10631044

10641045

1065-
func _mode_override_index_from_setting() -> int:
1066-
var es := EditorInterface.get_editor_settings()
1067-
if es == null or not es.has_setting(ClientConfigurator.MODE_OVERRIDE_SETTING):
1068-
return 0
1069-
var v := str(es.get_setting(ClientConfigurator.MODE_OVERRIDE_SETTING)).strip_edges().to_lower()
1070-
return maxi(MODE_OVERRIDE_VALUES.find(v), 0)
1071-
1072-
1073-
## Called whenever `is_dev_checkout()`'s answer could have changed — repaints
1074-
## the install label/tooltip, rebuilds the setup container (Mode row, Dev
1075-
## Server button vs uv status), and clears any stale update banner so a
1076-
## fresh check paints over a clean slate. The Update button state is reset
1077-
## too: a prior install attempt may have left it disabled with text like
1078-
## "Dev checkout — update via git" or "Extract failed"; without this reset,
1079-
## flipping the dropdown and re-checking would re-open the banner with the
1080-
## stale button text.
1081-
func _refresh_install_mode_ui() -> void:
1082-
_install_label.text = _install_mode_text()
1083-
_install_label.tooltip_text = _install_mode_tooltip()
1084-
_refresh_setup_status()
1085-
_update_banner.visible = false
1086-
if _update_manager != null:
1087-
_update_manager.clear_pending_download()
1088-
if _update_btn != null:
1089-
_update_btn.text = "Update"
1090-
_update_btn.disabled = false
1091-
1092-
1093-
func _on_mode_override_selected(index: int) -> void:
1094-
var value: String = MODE_OVERRIDE_VALUES[index] if index >= 0 and index < MODE_OVERRIDE_VALUES.size() else ""
1095-
var es := EditorInterface.get_editor_settings()
1096-
if es != null:
1097-
es.set_setting(ClientConfigurator.MODE_OVERRIDE_SETTING, value)
1098-
_refresh_install_mode_ui()
1099-
## Cancel any in-flight startup check before firing a new one, otherwise
1100-
## the next `request()` returns ERR_BUSY and the dropdown flip silently
1101-
## fails to re-check. `call_deferred` lets the cancel settle before the
1102-
## new request goes out.
1103-
if _update_manager != null:
1104-
_update_manager.cancel_check()
1105-
_update_manager.check_for_updates.call_deferred()
1106-
print("MCP | mode override -> %s" % (value if value else "auto"))
1107-
1108-
11091046
# --- Button handlers ---
11101047

11111048
func _on_reload_plugin() -> void:
@@ -1256,24 +1193,31 @@ func _refresh_setup_status() -> void:
12561193
return
12571194
for child in _setup_container.get_children():
12581195
child.queue_free()
1259-
_dev_server_btn = null
1260-
_dev_restart_btn = null
1196+
_dev_primary_btn = null
1197+
_dev_stop_btn = null
12611198

12621199
var is_dev := ClientConfigurator.is_dev_checkout()
12631200
if is_dev:
12641201
_setup_container.add_child(_make_status_row("Mode", "Dev (venv)", Color.CYAN))
1265-
_dev_server_btn = Button.new()
1266-
_dev_server_btn.size_flags_horizontal = Control.SIZE_EXPAND_FILL
1267-
_dev_server_btn.pressed.connect(_on_dev_server_pressed)
1268-
_update_dev_server_btn()
1269-
_setup_container.add_child(_dev_server_btn)
1270-
1271-
_dev_restart_btn = Button.new()
1272-
_dev_restart_btn.text = "Restart Server"
1273-
_dev_restart_btn.size_flags_horizontal = Control.SIZE_EXPAND_FILL
1274-
_dev_restart_btn.pressed.connect(_on_dev_restart_pressed)
1275-
_update_dev_restart_btn()
1276-
_setup_container.add_child(_dev_restart_btn)
1202+
1203+
var btn_row := HBoxContainer.new()
1204+
btn_row.add_theme_constant_override("separation", 4)
1205+
btn_row.size_flags_horizontal = Control.SIZE_EXPAND_FILL
1206+
1207+
_dev_primary_btn = Button.new()
1208+
_dev_primary_btn.text = "Restart Dev Server"
1209+
_dev_primary_btn.size_flags_horizontal = Control.SIZE_EXPAND_FILL
1210+
_dev_primary_btn.pressed.connect(_on_dev_primary_pressed)
1211+
btn_row.add_child(_dev_primary_btn)
1212+
1213+
_dev_stop_btn = Button.new()
1214+
_dev_stop_btn.text = "✕"
1215+
_dev_stop_btn.tooltip_text = "Stop the dev server without spawning a replacement."
1216+
_dev_stop_btn.pressed.connect(_on_dev_stop_pressed)
1217+
btn_row.add_child(_dev_stop_btn)
1218+
1219+
_setup_container.add_child(btn_row)
1220+
_update_dev_section_buttons()
12771221
return
12781222

12791223
# User mode — check for uv
@@ -1358,96 +1302,74 @@ func _make_status_row(label_text: String, value_text: String, value_color: Color
13581302
return row
13591303

13601304

1361-
## Pure helper — given the two independent server states, return the button
1362-
## label and tooltip. Factored out so tests can cover all three states without
1363-
## spinning up a real server or plugin.
1364-
static func _dev_server_btn_state(has_managed: bool, dev_running: bool) -> Dictionary:
1365-
var port := ClientConfigurator.http_port()
1366-
if has_managed:
1367-
return {
1368-
"text": "Switch to dev mode (--reload)",
1369-
"tooltip": "Stops the plugin's managed server and replaces it with a --reload dev server on port %d. The dev server auto-restarts when you edit Python sources." % port,
1370-
}
1371-
if dev_running:
1372-
return {
1373-
"text": "Exit dev mode",
1374-
"tooltip": "Stops the external dev server on port %d so the plugin's managed server can take over on next reload." % port,
1375-
}
1376-
return {
1377-
"text": "Start dev server",
1378-
"tooltip": "Spawns a --reload dev server on port %d. Auto-restarts when you edit Python sources." % port,
1379-
}
1380-
1381-
1382-
func _update_dev_server_btn() -> void:
1383-
if _dev_server_btn == null:
1384-
return
1385-
if _plugin == null:
1386-
return
1387-
## Defensive guard against the self-update mixed-state window — see the
1388-
## comment in `_update_status` for the full story. Same #168.
1389-
if not (_plugin.has_method("has_managed_server") and _plugin.has_method("is_dev_server_running")):
1390-
return
1391-
var state := _dev_server_btn_state(_plugin.has_managed_server(), _plugin.is_dev_server_running())
1392-
_dev_server_btn.text = state["text"]
1393-
_dev_server_btn.tooltip_text = state["tooltip"]
1394-
1395-
1396-
func _on_dev_server_pressed() -> void:
1397-
if _plugin == null:
1398-
return
1399-
if _plugin.has_managed_server():
1400-
# Managed server running — swap it for a --reload dev server.
1401-
# start_dev_server() calls _stop_server() internally before spawning.
1402-
_plugin.start_dev_server()
1403-
elif _plugin.is_dev_server_running():
1404-
_plugin.stop_dev_server()
1405-
else:
1406-
_plugin.start_dev_server()
1407-
_update_dev_section_buttons.call_deferred()
1408-
1409-
1410-
## Pure helper for the Restart Server button — enabled iff something is
1411-
## running on the HTTP port we can kick. Static so `test_dock_dev_server_btn`
1412-
## can cover the truth table without a real plugin.
1413-
static func _restart_server_btn_state(has_managed: bool, dev_running: bool) -> Dictionary:
1305+
## Pure helper for the primary "Restart Dev Server" button. Always enabled
1306+
## (clicking with nothing running just spawns fresh); tooltip adapts to
1307+
## whether a kill+respawn or fresh spawn is what'll happen.
1308+
static func _dev_primary_btn_state(has_managed: bool, dev_running: bool) -> Dictionary:
14141309
var port := ClientConfigurator.http_port()
14151310
if has_managed or dev_running:
14161311
return {
1417-
"enabled": true,
1312+
"text": "Restart Dev Server",
14181313
"tooltip": (
1419-
"Kill the server on port %d and respawn from current source, "
1420-
+ "preserving managed vs --reload mode. Use this to pick up "
1421-
+ "Python server-source changes that don't bump the version."
1314+
"Kill the server on port %d and start a fresh --reload dev server. "
1315+
+ "Use this to pick up Python source changes that don't bump the version."
14221316
) % port,
14231317
}
14241318
return {
1425-
"enabled": false,
1426-
"tooltip": "No godot-ai server is running on port %d." % port,
1319+
"text": "Start Dev Server",
1320+
"tooltip": "Spawn a --reload dev server on port %d. Auto-restarts when you edit Python sources." % port,
14271321
}
14281322

14291323

1430-
func _update_dev_restart_btn() -> void:
1431-
if _dev_restart_btn == null:
1324+
## Pure helper for the small "✕" stop button — only enabled when a dev
1325+
## server is actually running. Stops without respawning; intentionally
1326+
## never targets a managed server (that's the lifecycle's responsibility).
1327+
static func _dev_stop_btn_state(dev_running: bool) -> Dictionary:
1328+
if dev_running:
1329+
return {"enabled": true, "tooltip": "Stop the dev server without spawning a replacement."}
1330+
return {"enabled": false, "tooltip": "No --reload dev server to stop."}
1331+
1332+
1333+
func _on_dev_primary_pressed() -> void:
1334+
if _plugin == null or _server_restart_in_progress:
14321335
return
1433-
if _plugin == null:
1336+
if not _plugin.has_method("force_restart_or_start_dev_server"):
14341337
return
1435-
## See _update_dev_server_btn — same #168 self-update mixed-state guard.
1436-
if not (_plugin.has_method("has_managed_server") and _plugin.has_method("is_dev_server_running")):
1338+
_server_restart_in_progress = true
1339+
_update_dev_section_buttons()
1340+
if not is_inside_tree():
1341+
## Test path — no scene tree means no timer; run synchronously
1342+
## so suite assertions see the dispatch without `await`.
1343+
_plugin.force_restart_or_start_dev_server()
1344+
_server_restart_in_progress = false
14371345
return
1438-
var state := _restart_server_btn_state(_plugin.has_managed_server(), _plugin.is_dev_server_running())
1439-
_dev_restart_btn.disabled = not state["enabled"]
1440-
_dev_restart_btn.tooltip_text = state["tooltip"]
1346+
call_deferred("_perform_dev_restart_after_feedback")
14411347

14421348

1443-
func _on_dev_restart_pressed() -> void:
1349+
func _on_dev_stop_pressed() -> void:
14441350
if _plugin == null:
14451351
return
1446-
if _plugin.has_method("force_restart_server_preserving_mode"):
1447-
_plugin.force_restart_server_preserving_mode()
1352+
if _plugin.has_method("stop_dev_server"):
1353+
_plugin.stop_dev_server()
14481354
_update_dev_section_buttons.call_deferred()
14491355

14501356

1357+
func _perform_dev_restart_after_feedback() -> void:
1358+
## Brief paint cycle so the user sees "Restarting…" before the
1359+
## blocking _wait_for_port_free freezes the editor for up to 5s.
1360+
await get_tree().create_timer(0.15).timeout
1361+
if _plugin != null:
1362+
_plugin.force_restart_or_start_dev_server()
1363+
## start_dev_server's spawn happens via a 0.5s SceneTree timer; give
1364+
## it time to land plus a buffer for the WS reconnect before clearing
1365+
## the busy state. The unconditional clear matches sibling restart
1366+
## buttons — overshoot is fine because subsequent _update_status calls
1367+
## refresh the button against live plugin state.
1368+
await get_tree().create_timer(2.0).timeout
1369+
_server_restart_in_progress = false
1370+
_update_dev_section_buttons()
1371+
1372+
14511373
## Single-scan refresh of every dev-section button state. Both buttons
14521374
## key off the same `has_managed_server` / `is_dev_server_running` pair,
14531375
## and the latter scrapes lsof/ps — so doing the discovery once and
@@ -1460,14 +1382,20 @@ func _update_dev_section_buttons() -> void:
14601382
return
14611383
var has_managed: bool = _plugin.has_managed_server()
14621384
var dev_running: bool = _plugin.is_dev_server_running()
1463-
if _dev_server_btn != null:
1464-
var server_state := _dev_server_btn_state(has_managed, dev_running)
1465-
_dev_server_btn.text = server_state["text"]
1466-
_dev_server_btn.tooltip_text = server_state["tooltip"]
1467-
if _dev_restart_btn != null:
1468-
var restart_state := _restart_server_btn_state(has_managed, dev_running)
1469-
_dev_restart_btn.disabled = not restart_state["enabled"]
1470-
_dev_restart_btn.tooltip_text = restart_state["tooltip"]
1385+
if _dev_primary_btn != null:
1386+
if _server_restart_in_progress:
1387+
_dev_primary_btn.disabled = true
1388+
_dev_primary_btn.text = "Restarting…"
1389+
_dev_primary_btn.tooltip_text = "Killing the current server and respawning…"
1390+
else:
1391+
var primary_state := _dev_primary_btn_state(has_managed, dev_running)
1392+
_dev_primary_btn.disabled = false
1393+
_dev_primary_btn.text = primary_state["text"]
1394+
_dev_primary_btn.tooltip_text = primary_state["tooltip"]
1395+
if _dev_stop_btn != null:
1396+
var stop_state := _dev_stop_btn_state(dev_running)
1397+
_dev_stop_btn.disabled = (not stop_state["enabled"]) or _server_restart_in_progress
1398+
_dev_stop_btn.tooltip_text = stop_state["tooltip"]
14711399

14721400

14731401
func _on_install_uv() -> void:

0 commit comments

Comments
 (0)