Fix real bugs from independent review (docs/review-handoff.md)

A second AI session read the whole codebase in parallel (read-only, no
code changes) and left a handoff doc. Addressed the correctness findings:

- PhysicsWorld.Sync's early return compared _bodies.Count to live.Count,
  not their contents — same size, different membership (destroy one
  tracked GameObject, gain one untracked-because-no-collider one; or any
  Restore where the scene has both a Rigidbody+collider object and a
  Rigidbody-without-one) skipped cleanup entirely, leaking the native
  Box3D body forever. Fixed by checking the actual stale set. Two new
  regression tests reproduce the review's own two scenarios via a new
  Lingua_GetBodyCount native export, asserting on the native table's own
  count rather than PhysicsWorld's C#-side bookkeeping.
- TryCreateBody stored handle -1 (native shim refused: invalid world, or
  its 8192-slot body table full) as if it were real — every later
  GetBodyTransform on it silently teleported the GameObject to the
  origin with a degenerate rotation, no error anywhere. Now checked and
  warned once, same as the missing-collider case.
- PluginHost.Load didn't roll back anything when Configure threw partway
  through: Schedule.Add/Events.Subscribe registrations it already made
  stayed forever, and its ALC was never unloaded — neither loaded (no
  _loaded entry) nor cleanly unloadable. Fixed with try/catch: best-effort
  Shutdown (the only thing that knows which services this plugin
  provided), RemoveAllFrom on both Schedule and EventBus, best-effort ALC
  unload, rethrow. New fixture plugin (sandbox.failing-configure, mirrors
  sandbox.echo's own real-load pattern) registers a system against a
  shared Ping component then throws, so FailedConfigureRollbackTests can
  assert the dangling system actually stops firing — an earlier version
  tried to prove this via AssemblyLoadContext.All instead, which passed
  even against the deliberately-reverted buggy code (the ALC turned out
  to get collected either way once its only references went out of
  scope); watching the dangling system is what actually distinguishes
  rolled-back from not, confirmed by deliberately reverting the fix and
  watching this specific test fail before restoring it.
- Engine.Host's "r <id>" left a plugin unloaded on a failed reload with
  no honest indication of that, and retrying threw "not loaded" instead
  of ever reaching Load again. Added PluginHost.IsLoaded so the handler
  only calls Unload when there's something to unload, and the failure
  message now says the plugin is unloaded, not just "failed."
- AssetService.ReloadWithRetry's `when (attempt < 4)` guard meant the
  5th and final IOException fell out of the loop and propagated from a
  discarded fire-and-forget Task — no log, no event, nothing. Now logged.
- EditorState.Selected kept pointing at a GameObject Restore had already
  destroyed after ExitPlay, so Inspector/gizmo would silently keep
  editing something no longer in the world. EditorPlugin.DrawUi now
  compares IsPlaying against its own previous frame (not just reacting to
  the Stop button) so this is caught whether Play was exited via the
  button or the "stop" stdin command — the same stdin-vs-real-control gap
  already hit once earlier this session — and re-resolves the selection
  by name.
- README.md and kernel-contract.md's Event Bus/Time rows had fallen a
  milestone behind (still said "no asset system yet" and "no fixed-step
  accumulator yet" after both shipped).

Full suite: 98 tests.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N1qPfzq8TDCUMFMV3UwV5N

LinguaEngine.sln

@@ -85,6 +85,10 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "PhysicsDemoGame", "samples\
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "PhysicsDemoGame.Tests", "tests\PhysicsDemoGame.Tests\PhysicsDemoGame.Tests.csproj", "{0201686F-0E2A-400F-8166-CBD11B65D724}"
 EndProject
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "sandbox.failing-configure", "sandbox.failing-configure", "{AB8A31F6-ADF4-A607-5774-F0AC7E353102}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Sandbox.FailingConfigure", "plugins\sandbox.failing-configure\Sandbox.FailingConfigure\Sandbox.FailingConfigure.csproj", "{428A0279-9B16-473D-92BC-858D4ABB15E2}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -407,6 +411,18 @@ Global
 		{0201686F-0E2A-400F-8166-CBD11B65D724}.Release|x64.Build.0 = Release|Any CPU
 		{0201686F-0E2A-400F-8166-CBD11B65D724}.Release|x86.ActiveCfg = Release|Any CPU
 		{0201686F-0E2A-400F-8166-CBD11B65D724}.Release|x86.Build.0 = Release|Any CPU
+		{428A0279-9B16-473D-92BC-858D4ABB15E2}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{428A0279-9B16-473D-92BC-858D4ABB15E2}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{428A0279-9B16-473D-92BC-858D4ABB15E2}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{428A0279-9B16-473D-92BC-858D4ABB15E2}.Debug|x64.Build.0 = Debug|Any CPU
+		{428A0279-9B16-473D-92BC-858D4ABB15E2}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{428A0279-9B16-473D-92BC-858D4ABB15E2}.Debug|x86.Build.0 = Debug|Any CPU
+		{428A0279-9B16-473D-92BC-858D4ABB15E2}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{428A0279-9B16-473D-92BC-858D4ABB15E2}.Release|Any CPU.Build.0 = Release|Any CPU
+		{428A0279-9B16-473D-92BC-858D4ABB15E2}.Release|x64.ActiveCfg = Release|Any CPU
+		{428A0279-9B16-473D-92BC-858D4ABB15E2}.Release|x64.Build.0 = Release|Any CPU
+		{428A0279-9B16-473D-92BC-858D4ABB15E2}.Release|x86.ActiveCfg = Release|Any CPU
+		{428A0279-9B16-473D-92BC-858D4ABB15E2}.Release|x86.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -449,5 +465,7 @@ Global
 		{83FF5113-4091-0AE3-A9AD-50B7BB917258} = {9760813E-A577-3C56-DE20-0218A108BABD}
 		{1AFDA567-D86A-43C8-91B9-34CC13F48FEF} = {83FF5113-4091-0AE3-A9AD-50B7BB917258}
 		{0201686F-0E2A-400F-8166-CBD11B65D724} = {0AB3BF05-4346-4AA6-1389-037BE0695223}
+		{AB8A31F6-ADF4-A607-5774-F0AC7E353102} = {07D57EEB-2F50-60C4-C011-FE4FA775C9A8}
+		{428A0279-9B16-473D-92BC-858D4ABB15E2} = {AB8A31F6-ADF4-A607-5774-F0AC7E353102}
 	EndGlobalSection
 EndGlobal

README.md

@@ -116,8 +116,23 @@ and is unit-tested on its own — including the case a screenshot can't
 easily catch, dragging an object parented under a non-uniformly-scaled
 parent.
 
-No physics yet — see the build order (M0–M4) in
-[`docs/kernel-contract.md`](docs/kernel-contract.md) for what's next.
+**M4 in progress.** `engine.physics` wraps Box3D and `engine.audio` wraps
+miniaudio, both through a deliberately narrow C shim — neither library's
+own config structs (large, with function pointers and, for Box3D, at
+least one type that "cannot be directly copied") cross the P/Invoke
+boundary, only plain scalars and handles do. `samples/PhysicsDemo` ties
+physics, audio, input, and rendering together in one running project with
+`engine.editor` deliberately left out of its `project.json` — the
+shippable configuration, not the dev one. `.github/workflows/build.yml`
+builds and tests both native shims and the full solution on real
+`ubuntu-latest` and `windows-latest` runners, since nothing on a single
+Linux dev machine can otherwise verify the Windows half of M4's own "done
+when." Still open: the build pipeline hasn't actually run on GitHub yet
+(pending a `workflow` OAuth scope grant), and M4's "one small game"
+target is the physics demo so far, not yet a scored 20-minute one.
+
+See the build order (M0–M4) in
+[`docs/kernel-contract.md`](docs/kernel-contract.md) for the rest.
 
 Design and implementation are argued over in the same place: the doc is
 still the thing to disagree with before code changes to match.

docs/kernel-contract.md

@@ -53,8 +53,8 @@ nothing.
 | 02 | **Scheduler** | Frame stages, topological system ordering, parallel execution of systems with disjoint declared access, and debug-mode enforcement of that access — see §7. Structural changes (adding/removing a `GameObject` or `Component`) are queued and applied at the stage boundary, so a running system never sees a collection mutate under it. |
 | 03 | **Plugin Host** | Manifest parsing, dependency resolution, ALC loading, unloading, reload. |
 | 04 | **Service Registry** | Publishing and discovering interfaces between plugins. Control path, not the hot path. |
-| 05 | **Event Bus** | `Publish`/`Subscribe`, ownership-tracked and leak-safe the same way as the Scheduler's systems — see §7. `PluginHost` publishes `PluginLoaded`/`PluginUnloaded`; nothing publishes `GameObject` created/destroyed or asset-reloaded facts yet — the first because `GameWorld` doesn't touch the bus at all (a publish on every structural change would tax the hot path for listeners that usually don't exist), the second because there's no asset system yet. |
-| 06 | **Time & Log** | Frame clock (`DeltaTime`, `ElapsedTime`, `FrameCount`) and logging, both on `IPluginContext`. No fixed-step accumulator yet — deferred to M4, alongside the physics system it would actually drive. |
+| 05 | **Event Bus** | `Publish`/`Subscribe`, ownership-tracked and leak-safe the same way as the Scheduler's systems — see §7. `PluginHost` publishes `PluginLoaded`/`PluginUnloaded`; `engine.assets` publishes `TextureReloaded` when a watched file changes on disk (M2). Still nothing publishes `GameObject` created/destroyed facts — `GameWorld` doesn't touch the bus at all, deliberately: a publish on every structural change would tax the hot path for listeners that usually don't exist. |
+| 06 | **Time & Log** | Frame clock (`DeltaTime`, `ElapsedTime`, `FrameCount`, `FixedDeltaTime`) and logging, both on `IPluginContext`. The fixed-step accumulator (`Time.ConsumeFixedSteps`) shipped with M4, driving `Stage.FixedUpdate` — `engine.physics` is its first real consumer. |
 
 `GameObject.Transform` is the one field embedded directly rather than
 modeled as a `Component` subclass — it's a plain struct holding local
@@ -436,7 +436,7 @@ cost of changing course is still zero.
 | **M1** | **Window, input, a triangle.** Three separate plugins over Silk.NET. First real-load test of the data channel. | The triangle's color changes by editing system code, with no app restart. |
 | **M2** | **Assets and scenes.** Hot-reloading asset plugin, scene format, `World` serialization. | Swapping a texture on disk changes the picture with nothing stopped; a scene loads and saves. |
 | **M3** | **Editor as plugins — done.** Shell (`engine.editor`, ImGui over the live scene, see §5 and the new `Stage.Present`), reflection-based inspector and hierarchy, Play/Stop on snapshots, a real 3D translate gizmo driven by the actual camera. | Entering Play takes under 100 ms — the original complaint about Unity is closed. Proven twice: `WorldSnapshotTests` (kernel, 300 `GameObject`s) and a real editor run against `samples/WindowDemo` (13ms, logged by `PlayModeController`). |
-| **M4** | **One small game, end to end.** `engine.physics` over Box3D, audio, a Linux + Windows build pipeline. A 20-minute game, shipped as an executable. | The build runs on both platforms with no editor plugins in the shipped binary. |
+| **M4** | **One small game, end to end — in progress.** `engine.physics` (Box3D) and `engine.audio` (miniaudio) done, both over a narrow, scalars-only P/Invoke shim; `samples/PhysicsDemo` ties physics/audio/input/render together with `engine.editor` deliberately excluded from its `project.json`; `.github/workflows/build.yml` builds and tests on real `ubuntu-latest`/`windows-latest` runners. Not yet: the workflow has actually run on GitHub (blocked on an OAuth `workflow` scope grant), and the demo is a physics sandbox, not yet a scored 20-minute game. | The build runs on both platforms with no editor plugins in the shipped binary. |
 
 ---
 

native/physics-native/lingua_physics.c

@@ -83,6 +83,21 @@ LINGUA_API void Lingua_DestroyWorld( int32_t worldHandle )
 	g_worldUsed[worldHandle] = false;
 }
 
+// Exists so C# can assert on the native side's own bookkeeping directly —
+// PhysicsWorld's C#-side _bodies dictionary can look correct while the
+// native table underneath it has leaked a body Sync should have destroyed
+// (see PhysicsWorldTests for the regression this catches).
+LINGUA_API int32_t Lingua_GetBodyCount( void )
+{
+	int32_t count = 0;
+	for ( int32_t i = 0; i < LINGUA_MAX_BODIES; i++ )
+	{
+		if ( g_bodyUsed[i] )
+			count++;
+	}
+	return count;
+}
+
 LINGUA_API void Lingua_WorldStep( int32_t worldHandle, float timeStep, int32_t subStepCount )
 {
 	if ( !ValidWorld( worldHandle ) )

plugins/engine.assets/Engine.Assets/AssetService.cs

@@ -84,6 +84,16 @@ internal sealed class AssetService(IEventBus events, ILogger log) : IAssetServic
             {
                 await Task.Delay(50);
             }
+            catch (IOException ex)
+            {
+                // The 5th and final attempt — the `when` guard above only
+                // covers attempts 0-3, so this is the one case that used
+                // to fall out of the loop and propagate from a
+                // fire-and-forget Task (`_ = Task.Run(...)`) as an
+                // unobserved exception: no log, no event, nothing to show
+                // the reload silently never happened.
+                log.Warn($"Giving up reloading '{path}' after 5 attempts: {ex.Message}");
+            }
         }
     }
 

plugins/engine.editor/Engine.Editor/EditorPlugin.cs

@@ -37,6 +37,7 @@ public sealed class EditorPlugin : IPlugin
     private PlayModeController? _playMode;
     private IEngineWindow? _window;
     private ICameraService? _camera;
+    private bool _wasPlaying;
 
     public void Configure(IPluginContext ctx)
     {
@@ -107,10 +108,50 @@ public sealed class EditorPlugin : IPlugin
         ImGui.Text(_playMode.IsPlaying ? "(Playing)" : "(Edit mode)");
         ImGui.End();
 
+        // Found by independent review: ExitPlay's Restore destroys every
+        // GameObject and rebuilds fresh instances from the snapshot, but
+        // EditorState.Selected kept pointing at the old, now-detached one
+        // — Inspector and the gizmo would silently keep editing an object
+        // no longer in the world. Comparing against last frame's own
+        // IsPlaying (not just reacting to the button above) catches this
+        // uniformly regardless of *how* Play exited — the Stop button
+        // here, or the "stop" stdin command Engine.Host already processed
+        // before this Render-stage system ran this frame; the button
+        // alone would miss the second path entirely, the same
+        // stdin-vs-real-control gap already hit once this session.
+        if (_wasPlaying && !_playMode.IsPlaying)
+        {
+            var previousName = _state.Selected?.Name;
+            _state.Selected = previousName is null ? null : FindByName(world.Roots, previousName);
+        }
+        _wasPlaying = _playMode.IsPlaying;
+
         HierarchyPanel.Draw(world, _state);
         InspectorPanel.Draw(_state);
         _gizmo.Draw(_state, _camera!, _window!);
 
         _controller.Render();
     }
+
+    // Best-effort by name, not identity — Restore rebuilds fresh
+    // GameObject instances, so there's no identity to match against
+    // anymore. Two siblings sharing a name (nothing stops it, same caveat
+    // HierarchyPanel's own PushID-by-hashcode already documents) means
+    // this picks the first match, not necessarily "the same one" — an
+    // acceptable approximation for reselecting after Play, not a
+    // guarantee.
+    private static GameObject? FindByName(IReadOnlyList<GameObject> roots, string name)
+    {
+        foreach (var go in roots)
+        {
+            if (go.Name == name)
+                return go;
+
+            var found = FindByName(go.Children, name);
+            if (found is not null)
+                return found;
+        }
+
+        return null;
+    }
 }

plugins/engine.editor/Engine.Editor/EditorState.cs

@@ -7,7 +7,10 @@ namespace Engine.Editor;
 /// Hierarchy panel last clicked, so the Inspector panel (built against this
 /// same instance) knows what to show. One instance per EditorPlugin, not
 /// static: reloading engine.editor should start with nothing selected, not
-/// hold a reference to a GameObject that may not even exist anymore.
+/// hold a reference to a GameObject that may not even exist anymore. The
+/// same staleness can happen without a reload too — see EditorPlugin.
+/// DrawUi's IsPlaying-transition check, which re-resolves this after
+/// ExitPlay's Restore replaces every GameObject with a fresh instance.
 /// </summary>
 internal sealed class EditorState
 {

plugins/engine.physics/Engine.Physics/Native.cs

@@ -29,6 +29,12 @@ internal static class Native
     [DllImport(Lib)]
     public static extern void Lingua_WorldStep(int worldHandle, float timeStep, int subStepCount);
 
+    /// <summary>Total live bodies across every world — a test-only escape
+    /// hatch (see PhysicsWorldTests) to assert on the native side's own
+    /// bookkeeping, not just PhysicsWorld's C#-side dictionary.</summary>
+    [DllImport(Lib)]
+    public static extern int Lingua_GetBodyCount();
+
     [DllImport(Lib)]
     public static extern int Lingua_CreateBoxBody(
         int worldHandle,

plugins/engine.physics/Engine.Physics/PhysicsWorld.cs

@@ -20,7 +20,7 @@ internal sealed class PhysicsWorld : IDisposable
     private readonly int _handle;
     private readonly ILogger _log;
     private readonly Dictionary<GameObject, int> _bodies = [];
-    private readonly HashSet<GameObject> _warnedMissingCollider = [];
+    private readonly HashSet<GameObject> _warnedFailed = [];
 
     public PhysicsWorld(Vector3 gravity, ILogger log)
     {
@@ -39,14 +39,23 @@ internal sealed class PhysicsWorld : IDisposable
                 TryCreateBody(go);
         }
 
-        if (_bodies.Count == live.Count)
+        // Comparing counts alone would miss this: destroying one tracked
+        // GameObject and gaining a different untracked one in the same
+        // Sync leaves _bodies.Count == live.Count with the sets actually
+        // different — the stale native body would never get destroyed and
+        // would keep simulating forever. Restore (every ExitPlay) hits
+        // this reliably whenever the scene has both a Rigidbody+collider
+        // GameObject and a Rigidbody-without-collider one, since the
+        // latter never enters _bodies to begin with.
+        var stale = _bodies.Keys.Where(go => !live.Contains(go)).ToList();
+        if (stale.Count == 0)
             return;
 
-        foreach (var stale in _bodies.Keys.Where(go => !live.Contains(go)).ToList())
+        foreach (var go in stale)
         {
-            Native.Lingua_DestroyBody(_bodies[stale]);
-            _bodies.Remove(stale);
-            _warnedMissingCollider.Remove(stale);
+            Native.Lingua_DestroyBody(_bodies[go]);
+            _bodies.Remove(go);
+            _warnedFailed.Remove(go);
         }
     }
 
@@ -111,11 +120,25 @@ internal sealed class PhysicsWorld : IDisposable
         }
         else
         {
-            if (_warnedMissingCollider.Add(go))
+            if (_warnedFailed.Add(go))
                 _log.Warn($"'{go.Name}' has a Rigidbody but no BoxCollider/SphereCollider — no physics body created.");
             return;
         }
 
+        // -1 means the native shim refused (an invalid world handle, or
+        // its fixed-capacity body table — 8192 — is full). Storing it
+        // anyway would silently "work": every later Lingua_GetBodyTransform
+        // call on handle -1 fails ValidBody's check and leaves the out
+        // params at their P/Invoke-zeroed default, teleporting the
+        // GameObject to the origin with a degenerate all-zero rotation
+        // every FixedUpdate — no exception, no log, just a wrong position.
+        if (handle < 0)
+        {
+            if (_warnedFailed.Add(go))
+                _log.Warn($"'{go.Name}' failed to create a native physics body (world invalid or body table full).");
+            return;
+        }
+
         _bodies[go] = handle;
     }
 

plugins/sandbox.failing-configure/Sandbox.FailingConfigure/FailingConfigurePlugin.cs

@@ -0,0 +1,41 @@
+using Engine.Kernel.Plugins;
+using Engine.Kernel.Scheduling;
+using Engine.Kernel.World;
+using Sandbox.Echo.Contracts;
+
+namespace Sandbox.FailingConfigure;
+
+/// <summary>
+/// A test fixture, not a real plugin — exists only so
+/// FailedConfigureRollbackTests can exercise PluginHost.Load's rollback
+/// path against a real ALC/assembly load, the same way sandbox.echo exists
+/// for the successful-load path. Registers a system that increments a
+/// shared Ping component (so there's a real, deterministic side effect for
+/// the test to check — did the dangling system actually get removed, not
+/// just "did the ALC eventually get GC'd," which turned out to happen
+/// either way regardless of whether rollback ran, making it useless as a
+/// regression signal here), then throws — reproducing "Configure got
+/// partway through before failing," not "Configure failed immediately."
+/// </summary>
+public sealed class FailingConfigurePlugin : IPlugin
+{
+    public void Configure(IPluginContext ctx)
+    {
+        ctx.Schedule.Add(Stage.Update, Tick).Writes<Ping>();
+        ctx.Events.Subscribe<PluginLoaded>(_ => { });
+
+        throw new InvalidOperationException("deliberate failure for PluginHostTests");
+    }
+
+    public void Shutdown(IPluginContext ctx)
+    {
+        ctx.Schedule.RemoveAllFrom("sandbox.failing-configure");
+        ctx.Events.RemoveAllFrom("sandbox.failing-configure");
+    }
+
+    private static void Tick(IWorld world)
+    {
+        foreach (var go in world.Query<Ping>())
+            go.GetComponent<Ping>()!.Count++;
+    }
+}

plugins/sandbox.failing-configure/Sandbox.FailingConfigure/Sandbox.FailingConfigure.csproj

@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <ItemGroup>
+    <ProjectReference Include="..\..\..\src\Engine.Kernel\Engine.Kernel.csproj" />
+    <!-- Reusing sandbox.echo's own Ping component, not defining a new one:
+         it's already a shared, cross-ALC-safe Default-ALC type, and gives
+         FailedConfigureRollbackTests a real, deterministic side effect to
+         assert on (Ping.Count) instead of a GC-timing-sensitive check. -->
+    <ProjectReference Include="..\..\sandbox.echo\Sandbox.Echo.Contracts\Sandbox.Echo.Contracts.csproj" />
+  </ItemGroup>
+</Project>

plugins/sandbox.failing-configure/plugin.json

@@ -0,0 +1,7 @@
+{
+  "id":        "sandbox.failing-configure",
+  "version":   "0.1.0",
+  "assembly":  "Sandbox.FailingConfigure.dll",
+  "dependsOn": {},
+  "reloadable": true
+}

src/Engine.Host/Program.cs

@@ -230,13 +230,24 @@ if (windowed)
                     Console.WriteLine($"Reloading '{cmd.Argument}'...");
                     try
                     {
-                        host.Unload(cmd.Argument);
+                        // Not unconditional: a plugin whose previous
+                        // reload already failed inside Load (a bad
+                        // Configure — now rolled back, see PluginHost.
+                        // Load) isn't loaded any more, and Unload would
+                        // just throw "not loaded" on this retry instead of
+                        // ever reaching Load again.
+                        if (host.IsLoaded(cmd.Argument))
+                            host.Unload(cmd.Argument);
+
                         host.Load(pluginDirectory);
                         Console.WriteLine($"Reloaded '{cmd.Argument}'. World state and the window were untouched.");
                     }
                     catch (Exception ex)
                     {
-                        Console.Error.WriteLine($"Failed to reload '{cmd.Argument}': {ex.Message}");
+                        Console.Error.WriteLine(
+                            $"Failed to reload '{cmd.Argument}': {ex.Message} " +
+                            $"'{cmd.Argument}' is now unloaded (not just still running the old code) — " +
+                            $"fix the error and run 'r {cmd.Argument}' again.");
                     }
 
                     break;

src/Engine.Kernel/Plugins/PluginHost.cs

@@ -73,7 +73,34 @@ public sealed class PluginHost(
         events.RegisterPlugin(manifest.Id, implAssembly);
         var ctx = new PluginContext(manifest.Id, world, services, schedule, events, time);
 
-        instance.Configure(ctx);
+        try
+        {
+            instance.Configure(ctx);
+        }
+        catch
+        {
+            // Configure can fail after already doing real work — Schedule.
+            // Add, Events.Subscribe, Services.Provide calls all happen
+            // before whatever line actually throws. None of that unwinds
+            // on its own, and because _loaded never gets an entry for this
+            // id below, the plugin ends up neither loaded nor unloadable:
+            // its ALC stays rooted forever and any systems/subscriptions it
+            // did register before throwing keep running. Best-effort
+            // Shutdown first — it's the only thing that knows which
+            // services this specific plugin provided, so it's the only way
+            // to Revoke them — then the two RemoveAllFrom calls PluginHost
+            // itself can make unconditionally, then unload the ALC. Each
+            // step is wrapped so a broken Shutdown/unload can't hide the
+            // real Configure failure being rethrown below.
+            try { instance.Shutdown(ctx); } catch { /* best-effort */ }
+
+            schedule.RemoveAllFrom(manifest.Id);
+            events.RemoveAllFrom(manifest.Id);
+
+            try { alc.Unload(); } catch { /* best-effort */ }
+
+            throw;
+        }
 
         _loaded[manifest.Id] = new LoadedPlugin(manifest, alc, instance, ctx);
         events.Publish(new PluginLoaded(manifest.Id));
@@ -112,6 +139,16 @@ public sealed class PluginHost(
         return loadedIds;
     }
 
+    /// <summary>
+    /// Lets a caller check before calling Unload/Load instead of catching
+    /// InvalidOperationException to find out — Engine.Host's live-reload
+    /// command needs this: a plugin whose previous reload attempt failed
+    /// partway through Load isn't loaded any more (see Load's own rollback
+    /// on a Configure failure), so unconditionally trying Unload first
+    /// would itself throw "not loaded" on every retry.
+    /// </summary>
+    public bool IsLoaded(string pluginId) => _loaded.ContainsKey(pluginId);
+
     /// <summary>
     /// Runs Shutdown(), then unloads the plugin's ALC. Returns a weak
     /// reference to the ALC so a caller can verify it actually collected —

tests/Engine.ConformanceHarness/Engine.ConformanceHarness.csproj

@@ -32,6 +32,11 @@
          the harness's Default ALC, which defeats the point of a leak test. -->
     <ProjectReference Include="..\..\plugins\sandbox.echo\Sandbox.Echo\Sandbox.Echo.csproj"
                        ReferenceOutputAssembly="false" />
+
+    <!-- Same build-order-only reasoning, for PluginHostTests' Configure-
+         throws-partway-through fixture. -->
+    <ProjectReference Include="..\..\plugins\sandbox.failing-configure\Sandbox.FailingConfigure\Sandbox.FailingConfigure.csproj"
+                       ReferenceOutputAssembly="false" />
   </ItemGroup>
 
   <!-- Flatten plugin.json + both built DLLs into one directory under this
@@ -54,6 +59,15 @@
       <Link>plugins\sandbox.echo\Sandbox.Echo.dll</Link>
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
+
+    <None Include="..\..\plugins\sandbox.failing-configure\plugin.json">
+      <Link>plugins\sandbox.failing-configure\plugin.json</Link>
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
+    <None Include="..\..\plugins\sandbox.failing-configure\Sandbox.FailingConfigure\bin\$(Configuration)\$(TargetFramework)\Sandbox.FailingConfigure.dll">
+      <Link>plugins\sandbox.failing-configure\Sandbox.FailingConfigure.dll</Link>
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
   </ItemGroup>
 
 </Project>

tests/Engine.ConformanceHarness/FailedConfigureRollbackTests.cs

@@ -0,0 +1,100 @@
+using Engine.Kernel.Diagnostics;
+using Engine.Kernel.Events;
+using Engine.Kernel.Plugins;
+using Engine.Kernel.Scheduling;
+using Engine.Kernel.Services;
+using Engine.Kernel.World;
+using Sandbox.Echo.Contracts;
+
+namespace Engine.ConformanceHarness;
+
+/// <summary>
+/// Found by independent review, not by any test: PluginHost.Load didn't
+/// roll back anything when Configure threw partway through — Schedule.Add/
+/// Events.Subscribe calls it already made stayed registered forever, and
+/// the ALC it loaded into was never unloaded. Sandbox.FailingConfigure
+/// exists specifically to load real, into a real collectible ALC, and
+/// throw after registering a system, exactly reproducing that shape.
+///
+/// The system it registers increments a shared Ping component — a real,
+/// deterministic side effect. An earlier version of this test tried to
+/// prove the rollback via AssemblyLoadContext.All instead (did the failed
+/// load's ALC get collected?), but that passed even against the
+/// deliberately-reverted buggy PluginHost.Load: the ALC turned out to get
+/// collected either way once its only references (local variables inside
+/// Load) went out of scope, regardless of whether Schedule/EventBus still
+/// held onto its assembly. Watching whether the dangling system still
+/// fires is the thing that actually distinguishes rolled-back from not.
+/// </summary>
+public class FailedConfigureRollbackTests
+{
+    private static string PluginDirectory =>
+        Path.Combine(AppContext.BaseDirectory, "plugins", "sandbox.failing-configure");
+
+    private static PluginHost NewHost(Schedule schedule, GameWorld world) =>
+        new(world, new ServiceRegistry(), schedule, new EventBus(), new Time());
+
+    [Fact]
+    public void Load_Rethrows_The_Configure_Exception()
+    {
+        var host = NewHost(new Schedule(), new GameWorld());
+
+        var ex = Assert.Throws<InvalidOperationException>(() => host.Load(PluginDirectory));
+        Assert.Equal("deliberate failure for PluginHostTests", ex.Message);
+    }
+
+    [Fact]
+    public void Load_Fails_The_Same_Way_On_Retry_Instead_Of_Already_Loaded()
+    {
+        var host = NewHost(new Schedule(), new GameWorld());
+
+        Assert.Throws<InvalidOperationException>(() => host.Load(PluginDirectory));
+
+        // Before the fix, _loaded never got an entry either way — this
+        // alone wouldn't have caught the bug — but it's still the right
+        // thing to be true: retrying isn't "already loaded," it fails the
+        // same way every time.
+        var ex = Assert.Throws<InvalidOperationException>(() => host.Load(PluginDirectory));
+        Assert.Equal("deliberate failure for PluginHostTests", ex.Message);
+    }
+
+    [Fact]
+    public void IsLoaded_Is_False_After_A_Failed_Load_So_Retry_Does_Not_Need_Unload_First()
+    {
+        var host = NewHost(new Schedule(), new GameWorld());
+
+        Assert.Throws<InvalidOperationException>(() => host.Load(PluginDirectory));
+        Assert.False(host.IsLoaded("sandbox.failing-configure"));
+
+        // Mirrors Engine.Host's own "r <id>" handler: only Unload first if
+        // IsLoaded says so. Before that check existed, this exact retry
+        // sequence threw "Plugin 'sandbox.failing-configure' is not
+        // loaded" instead of ever reaching Load again.
+        if (host.IsLoaded("sandbox.failing-configure"))
+            host.Unload("sandbox.failing-configure");
+
+        Assert.Throws<InvalidOperationException>(() => host.Load(PluginDirectory));
+    }
+
+    [Fact]
+    public void Load_Removes_The_System_Configure_Registered_Before_Throwing()
+    {
+        var world = new GameWorld();
+        var schedule = new Schedule();
+        var host = NewHost(schedule, world);
+
+        var ping = world.CreateGameObject("Pinger").AddComponent<Ping>();
+
+        Assert.Throws<InvalidOperationException>(() => host.Load(PluginDirectory));
+
+        // Without rollback, the Tick system FailingConfigurePlugin
+        // registered before throwing is still sitting in Schedule and
+        // fires here, incrementing Count. With rollback (schedule.
+        // RemoveAllFrom in the catch block), it's gone, and this does
+        // nothing.
+        schedule.RunStage(Stage.Update, world);
+        schedule.RunStage(Stage.Update, world);
+
+        Assert.Equal(0, ping.Count);
+    }
+}

tests/Engine.Physics.Tests/PhysicsWorldTests.cs

@@ -144,4 +144,76 @@ public class PhysicsWorldTests
         var velocity = physics.GetLinearVelocity(box);
         Assert.True(velocity.X > 0f, $"expected positive X velocity after impulse, got {velocity.X}");
     }
+
+    // Sync's old early-return compared _bodies.Count to live.Count, not
+    // their contents — a real leak found by independent review, not by
+    // any of the tests above (all of them either never remove a
+    // GameObject, or do so in a way that changes the count). Both tests
+    // below reproduce the review's own two scenarios and assert on
+    // Native.Lingua_GetBodyCount() directly: the native table's own count
+    // is what actually leaks, and PhysicsWorld's C#-side _bodies
+    // dictionary alone can't prove it didn't.
+
+    [Fact]
+    public void Sync_DestroysStaleBody_EvenWhenLiveCountStaysTheSame()
+    {
+        var world = new GameWorld();
+        var log = new RecordingLogger();
+        using var physics = new PhysicsWorld(new Vector3(0, -10, 0), log);
+
+        var a = world.CreateGameObject("A");
+        a.Transform = Transform.Identity;
+        a.AddComponent<Rigidbody>().Type = BodyType.Dynamic;
+        a.AddComponent<BoxCollider>();
+
+        // B has a Rigidbody but no collider — it counts toward "live"
+        // every Sync (any Rigidbody does) but never gets a native body.
+        var b = world.CreateGameObject("B");
+        b.Transform = Transform.Identity;
+        b.AddComponent<Rigidbody>().Type = BodyType.Dynamic;
+
+        physics.Sync(world);
+        Assert.Equal(1, Native.Lingua_GetBodyCount());
+
+        world.Destroy(a);
+        // live = {B} (1), stale _bodies = {A} (1) — same count as before
+        // destroying A, which is exactly what let the old bug's early
+        // return skip cleanup.
+        physics.Sync(world);
+
+        Assert.Equal(0, Native.Lingua_GetBodyCount());
+    }
+
+    [Fact]
+    public void Sync_DestroysStaleBody_AcrossARestore()
+    {
+        var world = new GameWorld();
+        var log = new RecordingLogger();
+        using var physics = new PhysicsWorld(new Vector3(0, -10, 0), log);
+
+        var a = world.CreateGameObject("A");
+        a.Transform = Transform.Identity;
+        a.AddComponent<Rigidbody>().Type = BodyType.Dynamic;
+        a.AddComponent<BoxCollider>();
+
+        var b = world.CreateGameObject("B"); // Rigidbody, no collider
+        b.Transform = Transform.Identity;
+        b.AddComponent<Rigidbody>().Type = BodyType.Dynamic;
+
+        physics.Sync(world);
+        Assert.Equal(1, Native.Lingua_GetBodyCount());
+
+        // Restore destroys A and B and rebuilds fresh instances A'/B' from
+        // the snapshot. The next Sync creates A' (a new body) while old
+        // A's body is now orphaned — _bodies briefly holds {A, A'} (2)
+        // and live holds {A', B'} (2), the same count, which is exactly
+        // what the old bug's early return let slip through as "nothing
+        // stale to clean up."
+        var snapshot = world.Snapshot();
+        world.Restore(snapshot);
+        physics.Sync(world);
+
+        // Exactly one live native body (A'), not two (leaked old A + A').
+        Assert.Equal(1, Native.Lingua_GetBodyCount());
+    }
 }