From aa156446b5e51df9cb92fe93efaadc404d78a92c Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 11:21:07 +0900 Subject: [PATCH 1/5] feat(package): Enable pause points by source file and line Add File/Line parameters to enable-pause-point alongside the existing Id marker path. PausePointUseCase.Enable now validates that exactly one of Id or File+Line is provided, rejects the File/Line path when CompilationPipeline.codeOptimization is Release (Debug symbols are required to resolve a patch location), resolves File/Line via SourcePausePointResolver, patches the resolved method via SourcePausePointPatcher, derives the pause point id as ":" from the originally requested line (not the resolved/rounded line) so repeated calls at the same location stay idempotent, and returns ResolvedLine/ResolvedMethod plus any Patcher warning merged with the existing domain-reload warning. cli/common/tools/default-tools.json is updated in the same commit so DefaultToolsCatalogDriftTests keeps passing. A new frozen fixture file backs a full Resolver -> Patcher -> Registry integration test driven through the public tool surface. This is commit 1 of PR 5 in the source pause point plan; Clear() is intentionally unchanged here (Unpatch-on-clear wiring is commit 2). --- Assets/Tests/Editor/PausePointTests.cs | 122 ++++++++++++++++++ Assets/Tests/Editor/PausePointToolsFixture.cs | 15 +++ .../Editor/PausePointToolsFixture.cs.meta | 11 ++ .../PausePoint/PausePointTools.cs | 105 ++++++++++++++- .../PausePoint/SourcePausePointConstants.cs | 8 ++ cli/common/tools/default-tools.json | 12 +- 6 files changed, 267 insertions(+), 6 deletions(-) create mode 100644 Assets/Tests/Editor/PausePointToolsFixture.cs create mode 100644 Assets/Tests/Editor/PausePointToolsFixture.cs.meta diff --git a/Assets/Tests/Editor/PausePointTests.cs b/Assets/Tests/Editor/PausePointTests.cs index 6338740356..89bbb1ac57 100644 --- a/Assets/Tests/Editor/PausePointTests.cs +++ b/Assets/Tests/Editor/PausePointTests.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Newtonsoft.Json.Linq; @@ -10,6 +11,7 @@ using io.github.hatayama.UnityCliLoop.FirstPartyTools; using io.github.hatayama.UnityCliLoop.Infrastructure; using io.github.hatayama.UnityCliLoop.Runtime; +using io.github.hatayama.UnityCliLoop.Tests.PausePointToolsFixtures; namespace io.github.hatayama.UnityCliLoop.Tests.Editor { @@ -474,6 +476,112 @@ public async Task Enable_WhenPlayModeInactiveAndDomainReloadDisabled_ReturnsNoWa Assert.That(response.Warning, Is.Empty); } + // NOTE: Enabling by File/Line is rejected in Debug-only when + // CompilationPipeline.codeOptimization == CodeOptimization.Release. There is no seam to + // fake that Editor-global static property in an EditMode test, and flipping it for real + // would trigger a recompilation mid-test (forbidden by this repo's Unity Freeze Prevention + // guardrails). This branch is verified manually/E2E instead (see PR 6). + + [Test] + public async Task Enable_WhenFileAndLineResolveToRealMethod_PatchesAndCapturesVariablesOnHit() + { + // Verifies the File/Line path resolves a real fixture method, patches it via Harmony + // through the full public tool surface, and a subsequent call to the patched method + // hits the registry with its locals, parameters, and instance field captured. + PausePointResponse response = await EnablePausePointByFileLineAsync(FixtureFilePath, FixtureLine); + + Assert.That(response.Success, Is.True); + Assert.That(response.Id, Is.EqualTo($"{FixtureFilePath}:{FixtureLine}")); + Assert.That(response.ResolvedLine, Is.EqualTo(FixtureLine)); + Assert.That(response.ResolvedMethod, Does.Contain("Add")); + + EnableBySourceLocationFixture fixture = new(); + int sum = fixture.Add(2, 3); + + Assert.That(sum, Is.EqualTo(5)); + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus(response.Id); + Assert.That(snapshot.IsHit, Is.True); + Assert.That( + snapshot.CapturedVariables.Select(v => v.Name), + Is.EquivalentTo(new[] { "left", "right", "sum", "Tag" })); + } + + [Test] + public async Task Enable_WhenIdAndFileBothProvided_ReturnsValidationFailureResponse() + { + // Verifies Id and File/Line are mutually exclusive. + EnablePausePointTool tool = new(); + JObject parameters = new() + { + ["id"] = "jump", + ["file"] = FixtureFilePath, + ["line"] = FixtureLine, + ["timeoutSeconds"] = 30 + }; + + PausePointResponse response = (PausePointResponse)await tool.ExecuteAsync(parameters, CancellationToken.None); + + Assert.That(response.Success, Is.False); + Assert.That(response.Message, Is.EqualTo("Specify either Id or File and Line, not both.")); + } + + [Test] + public async Task Enable_WhenFileProvidedWithoutLine_ReturnsValidationFailureResponse() + { + // Verifies File requires Line to be provided together. + EnablePausePointTool tool = new(); + JObject parameters = new() + { + ["file"] = FixtureFilePath, + ["timeoutSeconds"] = 30 + }; + + PausePointResponse response = (PausePointResponse)await tool.ExecuteAsync(parameters, CancellationToken.None); + + Assert.That(response.Success, Is.False); + Assert.That(response.Message, Is.EqualTo("File and Line must both be provided together.")); + } + + [Test] + public async Task Enable_WhenLineProvidedWithoutFile_ReturnsValidationFailureResponse() + { + // Verifies Line requires File to be provided together. + EnablePausePointTool tool = new(); + JObject parameters = new() + { + ["line"] = FixtureLine, + ["timeoutSeconds"] = 30 + }; + + PausePointResponse response = (PausePointResponse)await tool.ExecuteAsync(parameters, CancellationToken.None); + + Assert.That(response.Success, Is.False); + Assert.That(response.Message, Is.EqualTo("File and Line must both be provided together.")); + } + + [Test] + public async Task Enable_WhenLineHasNoSequencePoint_ReturnsResolverErrorAsValidationFailure() + { + // Verifies a line with no sequence point on or after it (deliberately far past the + // fixture file's end) surfaces the Resolver's error message as a Success=false + // response instead of throwing. + EnablePausePointTool tool = new(); + JObject parameters = new() + { + ["file"] = FixtureFilePath, + ["line"] = 9999, + ["timeoutSeconds"] = 30 + }; + + PausePointResponse response = (PausePointResponse)await tool.ExecuteAsync(parameters, CancellationToken.None); + + Assert.That(response.Success, Is.False); + Assert.That(response.Message, Does.Contain("No sequence point found on or after line")); + } + + private const string FixtureFilePath = "Assets/Tests/Editor/PausePointToolsFixture.cs"; + private const int FixtureLine = 12; + private static async Task EnablePausePointAsync(string id) { EnablePausePointTool tool = new(); @@ -487,6 +595,20 @@ private static async Task EnablePausePointAsync(string id) return response; } + private static async Task EnablePausePointByFileLineAsync(string file, int line) + { + EnablePausePointTool tool = new(); + JObject parameters = new() + { + ["file"] = file, + ["line"] = line, + ["timeoutSeconds"] = 30 + }; + + PausePointResponse response = (PausePointResponse)await tool.ExecuteAsync(parameters, CancellationToken.None); + return response; + } + /// /// Test double that records pause requests without mutating Unity Editor state. /// diff --git a/Assets/Tests/Editor/PausePointToolsFixture.cs b/Assets/Tests/Editor/PausePointToolsFixture.cs new file mode 100644 index 0000000000..f5ff18eef6 --- /dev/null +++ b/Assets/Tests/Editor/PausePointToolsFixture.cs @@ -0,0 +1,15 @@ +// FROZEN FIXTURE: content and line numbers are asserted by PausePointTests. +// Do not reformat or edit this file; add a new fixture file instead. +namespace io.github.hatayama.UnityCliLoop.Tests.PausePointToolsFixtures +{ + internal sealed class EnableBySourceLocationFixture + { + public string Tag = "source-fixture-instance"; + + public int Add(int left, int right) + { + int sum = left + right; + return sum; + } + } +} diff --git a/Assets/Tests/Editor/PausePointToolsFixture.cs.meta b/Assets/Tests/Editor/PausePointToolsFixture.cs.meta new file mode 100644 index 0000000000..a3fbb1d16a --- /dev/null +++ b/Assets/Tests/Editor/PausePointToolsFixture.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 41dd106203c1f42f0a3fc84e8dc90c55 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs index 0cb8a719a5..64b4fdf464 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs @@ -2,6 +2,7 @@ using System.Threading; using System.Threading.Tasks; using UnityEditor; +using UnityEditor.Compilation; using io.github.hatayama.UnityCliLoop.Runtime; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -9,12 +10,18 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { /// - /// Parameters for enabling one named pause point marker. + /// Parameters for enabling one pause point, either by a hand-written marker id or by + /// resolving a source file:line to a patch location. Exactly one of "Id" or "File"+"Line" + /// must be provided. /// public class EnablePausePointSchema : UnityCliLoopToolSchema { public string Id { get; set; } = string.Empty; + public string File { get; set; } = string.Empty; + + public int Line { get; set; } + public int TimeoutSeconds { get; set; } = UloopPausePointRegistry.DefaultTimeoutSeconds; } @@ -37,6 +44,8 @@ public class PausePointResponse : UnityCliLoopToolResponse // Only explicit validation failures set this to false. public bool Success { get; set; } = true; public string Id { get; set; } = string.Empty; + public int ResolvedLine { get; set; } + public string ResolvedMethod { get; set; } = string.Empty; public string Status { get; set; } = string.Empty; public bool IsEnabled { get; set; } public bool IsHit { get; set; } @@ -172,10 +181,10 @@ internal sealed class PausePointUseCase { public PausePointResponse Enable(EnablePausePointSchema parameters) { - string idError = ValidateId(parameters.Id); - if (idError != null) + string modeError = ValidateEnableMode(parameters); + if (modeError != null) { - return CreateValidationFailure(idError); + return CreateValidationFailure(modeError); } if (parameters.TimeoutSeconds <= 0) @@ -183,6 +192,11 @@ public PausePointResponse Enable(EnablePausePointSchema parameters) return CreateValidationFailure("TimeoutSeconds must be greater than zero."); } + if (!string.IsNullOrWhiteSpace(parameters.File)) + { + return EnableBySourceLocation(parameters); + } + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.Enable(parameters.Id, parameters.TimeoutSeconds); PausePointResponse response = PausePointResponse.FromSnapshot(snapshot); response.Warning = CreateEnableWarning(); @@ -207,6 +221,89 @@ public PausePointResponse Clear(ClearPausePointSchema parameters) return PausePointResponse.FromSnapshot(snapshot); } + // Resolves File:Line to a patch location via the Resolver, patches it via Harmony, then + // arms the same registry state machine the Id path uses, keyed by the derived source id. + private static PausePointResponse EnableBySourceLocation(EnablePausePointSchema parameters) + { + if (CompilationPipeline.codeOptimization == CodeOptimization.Release) + { + return CreateValidationFailure(SourcePausePointConstants.ReleaseCodeOptimizationRejectionMessage); + } + + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve(parameters.File, parameters.Line); + if (!resolveResult.Success) + { + return CreateValidationFailure(resolveResult.ErrorMessage); + } + + string id = BuildSourcePausePointId(parameters.File, parameters.Line); + SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + if (!patchResult.Success) + { + return new PausePointResponse + { + Success = false, + Message = patchResult.ErrorMessage, + RecommendedNextAction = patchResult.Hint + }; + } + + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.Enable(id, parameters.TimeoutSeconds); + PausePointResponse response = PausePointResponse.FromSnapshot(snapshot); + response.ResolvedLine = resolveResult.Resolution.ResolvedLine; + response.ResolvedMethod = resolveResult.Resolution.MethodDisplayName; + response.Warning = MergeWarnings(CreateEnableWarning(), patchResult.Warning); + return response; + } + + // The derived id must use the originally requested file/line (not the resolved/rounded + // line) so repeated calls at the same requested location stay idempotent. + private static string BuildSourcePausePointId(string file, int line) + { + return SourcePausePointPathNormalizer.ToForwardSlashes(file) + ":" + line; + } + + private static string MergeWarnings(string first, string second) + { + if (string.IsNullOrEmpty(first)) + { + return second; + } + + if (string.IsNullOrEmpty(second)) + { + return first; + } + + return first + " " + second; + } + + // Returns an error message when the Id/File/Line combination fails validation, or null + // when exactly one of "Id" or "File"+"Line" is provided. + private static string ValidateEnableMode(EnablePausePointSchema parameters) + { + bool hasId = !string.IsNullOrWhiteSpace(parameters.Id); + bool hasFile = !string.IsNullOrWhiteSpace(parameters.File); + bool hasLine = parameters.Line > 0; + + if (hasId && (hasFile || hasLine)) + { + return "Specify either Id or File and Line, not both."; + } + + if (!hasId && !hasFile && !hasLine) + { + return "Id must not be null or empty."; + } + + if (!hasId && hasFile != hasLine) + { + return "File and Line must both be provided together."; + } + + return null; + } + // Returns an error message when id fails validation, or null when it is valid. private static string ValidateId(string id) { diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index 3ba7f752f3..5da5a80e8e 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -43,5 +43,13 @@ internal static class SourcePausePointConstants public const string RefStructInstanceNotCapturedWarning = "The declaring type is a ref struct; this-instance fields are not captured " + "(locals and parameters are still captured normally)."; + + // Release code optimization strips most sequence points and hoists/elides locals, so the + // Resolver's PDB-driven lookup cannot reliably find a patch location; rejecting up front + // avoids patching the wrong instruction instead of failing later in a confusing way. + public const string ReleaseCodeOptimizationRejectionMessage = + "Enabling a pause point by file and line requires Debug code optimization. The project " + + "is currently set to Release; switch the Editor's Code Optimization mode to Debug " + + "(the bug icon in the main toolbar) and recompile, then retry."; } } diff --git a/cli/common/tools/default-tools.json b/cli/common/tools/default-tools.json index e434429cca..878f28d279 100644 --- a/cli/common/tools/default-tools.json +++ b/cli/common/tools/default-tools.json @@ -340,13 +340,21 @@ }, { "name": "enable-pause-point", - "description": "Enable a named UloopPausePoint.Pause marker so Unity pauses when that code path is reached", + "description": "Enable a pause point so Unity pauses when that code path is reached, either by a named UloopPausePoint.Pause marker (Id) or by resolving a source file and line (File+Line)", "inputSchema": { "type": "object", "properties": { "Id": { "type": "string", - "description": "Named pause point id passed to UloopPausePoint.Pause" + "description": "Named pause point id passed to UloopPausePoint.Pause. Mutually exclusive with File/Line" + }, + "File": { + "type": "string", + "description": "Project-relative source file path to patch a pause point into. Requires Line; mutually exclusive with Id" + }, + "Line": { + "type": "integer", + "description": "1-based source line to resolve within File. Requires File; mutually exclusive with Id" }, "TimeoutSeconds": { "type": "integer", From cd844c5fb6faa83a78de565a0dffba241470dc32 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 11:22:46 +0900 Subject: [PATCH 2/5] feat(package): Unpatch source pause points on clear Wire SourcePausePointPatcher.Unpatch/UnpatchAll into both clear paths so a source pause point's Harmony injection is actually removed, not just its registry entry: - PausePointUseCase.Clear now calls Unpatch(id) for a specific id and UnpatchAll() for --all (a safe no-op for marker-only ids that were never Harmony-patched). - PausePointStatusBridgeCommand.Clear (the path Go's wait-for-pause-point timeout auto-clear and clear-pause-point-status hit) now calls Unpatch(id) too, so a source pause point left armed past its timeout does not leave a dangling injection after Unity reports it Cleared. Infrastructure needs a new reference to the PausePoint editor assembly plus an InternalsVisibleTo grant to call the internal SourcePausePointPatcher; Tests.Editor gets the same grant so PausePointTests can prove Unpatch actually ran (re-Patch-ing the same id with a deliberately stale Mvid only reaches the Patcher's stale-assembly gate once the id is no longer in its ledger, since an already-patched id short-circuits before that gate runs). --- Assets/Tests/Editor/PausePointTests.cs | 91 +++++++++++++++++++ .../PausePoint/AssemblyInfo.cs | 5 + .../PausePoint/PausePointTools.cs | 2 + .../Api/PausePointStatusBridgeCommand.cs | 6 ++ .../UnityCLILoop.Infrastructure.asmdef | 3 +- 5 files changed, 106 insertions(+), 1 deletion(-) diff --git a/Assets/Tests/Editor/PausePointTests.cs b/Assets/Tests/Editor/PausePointTests.cs index 89bbb1ac57..e7be7149b4 100644 --- a/Assets/Tests/Editor/PausePointTests.cs +++ b/Assets/Tests/Editor/PausePointTests.cs @@ -41,6 +41,10 @@ public void TearDown() { EditorSettings.enterPlayModeOptionsEnabled = _originalEnterPlayModeOptionsEnabled; EditorSettings.enterPlayModeOptions = _originalEnterPlayModeOptions; + // Tests that enable pause points by File/Line leave a Harmony transpiler attached to + // the fixture method; clear it so later tests re-patch cleanly instead of hitting the + // Patcher's "already patched" no-op path against a previous test's ledger entry. + SourcePausePointPatcher.UnpatchAll(); UloopPausePointRegistry.ResetForTests(); } @@ -579,9 +583,96 @@ public async Task Enable_WhenLineHasNoSequencePoint_ReturnsResolverErrorAsValida Assert.That(response.Message, Does.Contain("No sequence point found on or after line")); } + [Test] + public async Task Clear_WhenSpecificIdCleared_CallsPatcherUnpatchSoTheIdCanBeFreshlyRePatched() + { + // Verifies PausePointUseCase.Clear actually calls SourcePausePointPatcher.Unpatch (not + // just the registry): after clearing, re-Patch-ing the same id with a deliberately + // stale Mvid must reach the Patcher's stale-assembly gate again, which only runs when + // the id is no longer in the Patcher's ledger (an "already patched" id short-circuits + // before that gate ever runs, per SourcePausePointPatcherTests coverage from PR 4). + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve(FixtureFilePath, FixtureLine); + Assert.That(resolveResult.Success, Is.True); + string id = $"{FixtureFilePath}:{FixtureLine}"; + + UloopPausePointRegistry.Enable(id, 30); + Assert.That(SourcePausePointPatcher.Patch(id, resolveResult.Resolution).Success, Is.True); + + ClearPausePointTool clearTool = new(); + JObject clearParameters = new() { ["id"] = id, ["all"] = false }; + await clearTool.ExecuteAsync(clearParameters, CancellationToken.None); + + SourcePausePointPatchResult rePatchResult = SourcePausePointPatcher.Patch(id, WithStaleMvid(resolveResult.Resolution)); + + Assert.That(rePatchResult.Success, Is.False); + Assert.That(rePatchResult.FailureReason, Is.EqualTo(SourcePausePointPatchFailureReason.StaleAssembly)); + } + + [Test] + public async Task ClearAll_WhenSourcePausePointsExist_CallsPatcherUnpatchAllSoIdsCanBeFreshlyRePatched() + { + // Verifies PausePointUseCase.Clear(All) calls SourcePausePointPatcher.UnpatchAll, + // using the same stale-Mvid gate signal as the --id case above to prove the ledger + // entry was actually removed rather than only clearing the registry. + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve(FixtureFilePath, FixtureLine); + Assert.That(resolveResult.Success, Is.True); + string id = $"{FixtureFilePath}:{FixtureLine}"; + + UloopPausePointRegistry.Enable(id, 30); + Assert.That(SourcePausePointPatcher.Patch(id, resolveResult.Resolution).Success, Is.True); + + ClearPausePointTool clearTool = new(); + JObject clearParameters = new() { ["all"] = true }; + await clearTool.ExecuteAsync(clearParameters, CancellationToken.None); + + SourcePausePointPatchResult rePatchResult = SourcePausePointPatcher.Patch(id, WithStaleMvid(resolveResult.Resolution)); + + Assert.That(rePatchResult.Success, Is.False); + Assert.That(rePatchResult.FailureReason, Is.EqualTo(SourcePausePointPatchFailureReason.StaleAssembly)); + } + + [Test] + public void PausePointStatusBridgeCommand_Clear_CallsPatcherUnpatchSoTheIdCanBeFreshlyRePatched() + { + // Verifies the CLI bridge's Clear (the path Go's wait-for-pause-point timeout + // auto-clear and clear-pause-point-status hit) also calls + // SourcePausePointPatcher.Unpatch, using the same stale-Mvid gate signal as the tool + // tests above to prove the ledger entry was actually removed. + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve(FixtureFilePath, FixtureLine); + Assert.That(resolveResult.Success, Is.True); + string id = $"{FixtureFilePath}:{FixtureLine}"; + + UloopPausePointRegistry.Enable(id, 30); + Assert.That(SourcePausePointPatcher.Patch(id, resolveResult.Resolution).Success, Is.True); + + JObject bridgeParameters = new() { ["Id"] = id }; + PausePointStatusBridgeCommand.Clear(bridgeParameters); + + SourcePausePointPatchResult rePatchResult = SourcePausePointPatcher.Patch(id, WithStaleMvid(resolveResult.Resolution)); + + Assert.That(rePatchResult.Success, Is.False); + Assert.That(rePatchResult.FailureReason, Is.EqualTo(SourcePausePointPatchFailureReason.StaleAssembly)); + } + private const string FixtureFilePath = "Assets/Tests/Editor/PausePointToolsFixture.cs"; private const int FixtureLine = 12; + private static SourcePausePointResolution WithStaleMvid(SourcePausePointResolution resolution) + { + return new SourcePausePointResolution( + resolution.AssemblyName, + Guid.NewGuid().ToString(), + resolution.MetadataToken, + resolution.MethodDisplayName, + resolution.IsStatic, + resolution.IsDeclaringTypeValueType, + resolution.InstructionIndex, + resolution.IlOffset, + resolution.ResolvedLine, + resolution.Locals, + resolution.Parameters); + } + private static async Task EnablePausePointAsync(string id) { EnablePausePointTool tool = new(); diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs index 7c0a946fb0..5eb2919a62 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs @@ -3,3 +3,8 @@ [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointResolver")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointCapture")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointPatcher")] +// PausePointStatusBridgeCommand.Clear (Infrastructure) unpatches source pause points on clear; +// PausePointTests (Tests.Editor) exercises the Resolver/Patcher pipeline end-to-end and needs +// SourcePausePointPatcher visibility to prove Unpatch actually detaches the ledger entry. +[assembly: InternalsVisibleTo("UnityCLILoop.Infrastructure")] +[assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor")] diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs index 64b4fdf464..b504a62f46 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs @@ -207,6 +207,7 @@ public PausePointResponse Clear(ClearPausePointSchema parameters) { if (parameters.All) { + SourcePausePointPatcher.UnpatchAll(); UloopPausePointClearAllResult clearAllResult = UloopPausePointRegistry.ClearAll(); return PausePointResponse.FromClearAll(clearAllResult); } @@ -217,6 +218,7 @@ public PausePointResponse Clear(ClearPausePointSchema parameters) return CreateValidationFailure(idError); } + SourcePausePointPatcher.Unpatch(parameters.Id); UloopPausePointSnapshot snapshot = UloopPausePointRegistry.Clear(parameters.Id); return PausePointResponse.FromSnapshot(snapshot); } diff --git a/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs b/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs index c5e3bba70f..58e6904476 100644 --- a/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs +++ b/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs @@ -1,6 +1,7 @@ using System; using Newtonsoft.Json.Linq; +using io.github.hatayama.UnityCliLoop.FirstPartyTools; using io.github.hatayama.UnityCliLoop.Runtime; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -23,6 +24,11 @@ public static PausePointStatusResponse Execute(JToken paramsToken) public static PausePointStatusResponse Clear(JToken paramsToken) { string id = ReadId(paramsToken); + // This is the path the Go CLI's wait-for-pause-point timeout auto-clear and + // clear-pause-point-status hit; a source pause point left un-unpatched here would keep + // its Harmony injection attached (inert but never cleaned up) after the marker itself + // reports Cleared. + SourcePausePointPatcher.Unpatch(id); UloopPausePointSnapshot snapshot = UloopPausePointRegistry.Clear(id); return PausePointStatusResponse.FromSnapshot(snapshot); } diff --git a/Packages/src/Editor/Infrastructure/UnityCLILoop.Infrastructure.asmdef b/Packages/src/Editor/Infrastructure/UnityCLILoop.Infrastructure.asmdef index 7da9fee33e..c4b5e8b5b1 100644 --- a/Packages/src/Editor/Infrastructure/UnityCLILoop.Infrastructure.asmdef +++ b/Packages/src/Editor/Infrastructure/UnityCLILoop.Infrastructure.asmdef @@ -6,7 +6,8 @@ "GUID:5c4588558a3624eacbce0f50007cf1eb", "GUID:fc3fd32eddbee40e39c2d76dc184957b", "GUID:5079a8d3a72924a81aa1cbc25f65ed1b", - "GUID:527f26a36b5043c2bd4d4036d04cd76d" + "GUID:527f26a36b5043c2bd4d4036d04cd76d", + "GUID:94d8abc693f543a691a4645a5ff42e5c" ], "includePlatforms": [ "Editor" From c15d7fd0f35c630633217e74697c8e51823361c0 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 11:40:31 +0900 Subject: [PATCH 3/5] Route source pause point unpatch through a registry hook The previous commit (feat(package): Unpatch source pause points on clear) wired SourcePausePointPatcher.Unpatch/UnpatchAll into PausePointStatusBridgeCommand.Clear by adding a direct asmdef reference and InternalsVisibleTo grant from the Infrastructure asmdef to the FirstPartyTools.PausePoint.Editor asmdef. Running the full Unity EditMode suite revealed this broke OnionAssemblyDependencyTests.InfrastructureAsmdef_WhenLoaded_DependsOnApplicationRuntimeAndDoesNotReferencePresentation, which asserts Infrastructure's asmdef reference set is a fixed, closed list that excludes individual FirstPartyTools implementation asmdefs; Infrastructure may only depend on the shared PausePointsRuntime assembly, InternalApiBridge, Application, Domain, and ToolContracts. Fix the root cause instead of adjusting the test: move the unpatch wiring out of Infrastructure entirely by adding an Action OnCleared and an Action OnClearedAll hook to UloopPausePointRegistry (Runtime assembly, already an allowed Infrastructure dependency). SourcePausePointPatcher's static constructor wires Unpatch/UnpatchAll into these hooks; Registry.Clear/ClearAll invoke them internally. Both PausePointUseCase.Clear and PausePointStatusBridgeCommand.Clear go back to calling only UloopPausePointRegistry.Clear/ClearAll, with neither referencing SourcePausePointPatcher directly. The now-unneeded Infrastructure asmdef reference and InternalsVisibleTo grant are removed. --- .../FirstPartyTools/PausePoint/AssemblyInfo.cs | 2 -- .../FirstPartyTools/PausePoint/PausePointTools.cs | 5 +++-- .../PausePoint/SourcePausePointPatcher.cs | 11 +++++++++++ .../Api/PausePointStatusBridgeCommand.cs | 10 ++++------ .../UnityCLILoop.Infrastructure.asmdef | 3 +-- .../Runtime/PausePoints/UloopPausePointRegistry.cs | 13 +++++++++++++ 6 files changed, 32 insertions(+), 12 deletions(-) diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs index 5eb2919a62..99cb7a0a06 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs @@ -3,8 +3,6 @@ [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointResolver")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointCapture")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointPatcher")] -// PausePointStatusBridgeCommand.Clear (Infrastructure) unpatches source pause points on clear; // PausePointTests (Tests.Editor) exercises the Resolver/Patcher pipeline end-to-end and needs // SourcePausePointPatcher visibility to prove Unpatch actually detaches the ledger entry. -[assembly: InternalsVisibleTo("UnityCLILoop.Infrastructure")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor")] diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs index b504a62f46..cf6c04c070 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs @@ -207,7 +207,9 @@ public PausePointResponse Clear(ClearPausePointSchema parameters) { if (parameters.All) { - SourcePausePointPatcher.UnpatchAll(); + // Registry.ClearAll unpatches any source pause points via the hook + // SourcePausePointPatcher wires into it; this use case never references the + // Patcher directly. UloopPausePointClearAllResult clearAllResult = UloopPausePointRegistry.ClearAll(); return PausePointResponse.FromClearAll(clearAllResult); } @@ -218,7 +220,6 @@ public PausePointResponse Clear(ClearPausePointSchema parameters) return CreateValidationFailure(idError); } - SourcePausePointPatcher.Unpatch(parameters.Id); UloopPausePointSnapshot snapshot = UloopPausePointRegistry.Clear(parameters.Id); return PausePointResponse.FromSnapshot(snapshot); } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs index 6676f52bc7..ecb07e0847 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs @@ -32,6 +32,17 @@ internal static class SourcePausePointPatcher private static readonly Dictionary> InjectionsByMethod = new(); private static readonly Dictionary MethodById = new(); + // The registry lives in a Runtime assembly this Editor-only tool assembly may depend on, + // but not the reverse (patching is an outer/implementation concern the registry's inner + // layer must not know about). Wiring these hooks here - rather than having every Clear + // caller reference this class directly - lets the Infrastructure CLI bridge call + // UloopPausePointRegistry.Clear/ClearAll without ever referencing this assembly. + static SourcePausePointPatcher() + { + UloopPausePointRegistry.OnCleared = Unpatch; + UloopPausePointRegistry.OnClearedAll = UnpatchAll; + } + public static SourcePausePointPatchResult Patch(string id, SourcePausePointResolution resolution) { Debug.Assert(!string.IsNullOrEmpty(id), "id must not be null or empty."); diff --git a/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs b/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs index 58e6904476..16ddbe49e9 100644 --- a/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs +++ b/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs @@ -1,7 +1,6 @@ using System; using Newtonsoft.Json.Linq; -using io.github.hatayama.UnityCliLoop.FirstPartyTools; using io.github.hatayama.UnityCliLoop.Runtime; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -24,11 +23,10 @@ public static PausePointStatusResponse Execute(JToken paramsToken) public static PausePointStatusResponse Clear(JToken paramsToken) { string id = ReadId(paramsToken); - // This is the path the Go CLI's wait-for-pause-point timeout auto-clear and - // clear-pause-point-status hit; a source pause point left un-unpatched here would keep - // its Harmony injection attached (inert but never cleaned up) after the marker itself - // reports Cleared. - SourcePausePointPatcher.Unpatch(id); + // Registry.Clear unpatches any source pause point via the hook + // SourcePausePointPatcher wires into it, so this bridge - which must not reference + // that Editor-only tool assembly directly - never leaves a Harmony injection attached + // after the marker itself reports Cleared. UloopPausePointSnapshot snapshot = UloopPausePointRegistry.Clear(id); return PausePointStatusResponse.FromSnapshot(snapshot); } diff --git a/Packages/src/Editor/Infrastructure/UnityCLILoop.Infrastructure.asmdef b/Packages/src/Editor/Infrastructure/UnityCLILoop.Infrastructure.asmdef index c4b5e8b5b1..7da9fee33e 100644 --- a/Packages/src/Editor/Infrastructure/UnityCLILoop.Infrastructure.asmdef +++ b/Packages/src/Editor/Infrastructure/UnityCLILoop.Infrastructure.asmdef @@ -6,8 +6,7 @@ "GUID:5c4588558a3624eacbce0f50007cf1eb", "GUID:fc3fd32eddbee40e39c2d76dc184957b", "GUID:5079a8d3a72924a81aa1cbc25f65ed1b", - "GUID:527f26a36b5043c2bd4d4036d04cd76d", - "GUID:94d8abc693f543a691a4645a5ff42e5c" + "GUID:527f26a36b5043c2bd4d4036d04cd76d" ], "includePlatforms": [ "Editor" diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs index ee29dc492b..68cf004502 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs @@ -26,6 +26,15 @@ internal static class UloopPausePointRegistry // not just the latest hit, to report every marker that interrupted them. private static readonly List _hitSnapshots = new(); + // Source pause points are patched into IL by an Editor-only tool assembly this Runtime + // assembly must not reference directly (patching is an outer/implementation concern; this + // registry is the inner layer). That tool assembly wires its own Unpatch/UnpatchAll into + // these hooks the first time it patches a method, so every Clear/ClearAll caller - + // including the Infrastructure CLI bridge, which also must not reference the tool + // assembly - removes the underlying Harmony patch without knowing it exists. + public static Action OnCleared { get; set; } + public static Action OnClearedAll { get; set; } + public static UloopPausePointSnapshot Enable(string id, int timeoutSeconds) { Debug.Assert(!string.IsNullOrWhiteSpace(id), "id must not be null or empty"); @@ -43,6 +52,8 @@ public static UloopPausePointSnapshot Clear(string id) { Debug.Assert(!string.IsNullOrWhiteSpace(id), "id must not be null or empty"); + OnCleared?.Invoke(id); + DateTime now = NowUtc(); if (!Entries.ContainsKey(id)) { @@ -66,6 +77,8 @@ public static UloopPausePointSnapshot Clear(string id) public static UloopPausePointClearAllResult ClearAll() { + OnClearedAll?.Invoke(); + DateTime now = NowUtc(); int clearedCount = 0; foreach (UloopPausePointEntry entry in Entries.Values) From f6dbd3b610dd02901572e10e49f9d64eceb7239b Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 11:44:48 +0900 Subject: [PATCH 4/5] Return captured variables from wait-for-pause-point Enabling a pause point by File/Line already captures local variables, parameters, and this fields into the registry snapshot, but the CLI's wait-for-pause-point/pause-point-status commands only surfaced marker bookkeeping fields (Status, IsHit, EditorState, etc.). A user hitting a source pause point had no way to see what values were actually captured. Add CapturedVariables and CapturedVariablesTruncated to both the C# PausePointStatusResponse (Infrastructure bridge) and the Go pausePointStatusResponse, mirroring UloopCapturedVariable's fields field-for-field so the Go CLI passes Unity's captured values straight through to its JSON stdout. normalizePausePointStatusResponse is left untouched since it only derives Expired/RemainingMilliseconds for older Unity packages. --- Assets/Tests/Editor/PausePointTests.cs | 20 ++++ .../Api/PausePointStatusBridgeCommand.cs | 45 ++++++- .../projectrunner/pause_point_wait.go | 51 +++++--- .../projectrunner/pause_point_wait_test.go | 110 ++++++++++++++++++ 4 files changed, 207 insertions(+), 19 deletions(-) diff --git a/Assets/Tests/Editor/PausePointTests.cs b/Assets/Tests/Editor/PausePointTests.cs index e7be7149b4..ec7e03cd74 100644 --- a/Assets/Tests/Editor/PausePointTests.cs +++ b/Assets/Tests/Editor/PausePointTests.cs @@ -305,6 +305,26 @@ public void PausePointStatusBridge_WhenMarkerExpired_ReturnsRecoveryAction() Is.EqualTo("Clear this marker, then re-enable it with the same Id and TimeoutSeconds values.")); } + [Test] + public void PausePointStatusBridge_WhenPausePointHitWithCapturedVariables_ReturnsCapturedVariables() + { + // Verifies the CLI status bridge surfaces captured variables and the truncated flag + // from the registry snapshot, not just the marker/hit bookkeeping fields. + UloopPausePointRegistry.Enable("jump", 30); + UloopCapturedVariable[] capturedVariables = + { + new("speed", UloopCapturedVariableScope.Local, "System.Int32", "5", string.Empty, string.Empty, 0) + }; + UloopPausePointRegistry.HitWithCapturedVariables("jump", capturedVariables, true); + JObject parameters = new() { ["id"] = "jump" }; + + PausePointStatusResponse response = PausePointStatusBridgeCommand.Execute(parameters); + + Assert.That(response.CapturedVariablesTruncated, Is.True); + Assert.That(response.CapturedVariables.Select(v => v.Name), Is.EquivalentTo(new[] { "speed" })); + Assert.That(response.CapturedVariables[0].Value, Is.EqualTo("5")); + } + [Test] public void Enable_WhenSamePausePointWasHit_ClearsLatestHitSnapshot() { diff --git a/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs b/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs index 16ddbe49e9..c8e12fd86c 100644 --- a/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs +++ b/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Generic; +using System.Linq; using Newtonsoft.Json.Linq; using io.github.hatayama.UnityCliLoop.Runtime; @@ -66,6 +68,9 @@ public class PausePointStatusResponse : UnityCliLoopToolResponse public int LastHitSequence { get; set; } public string Message { get; set; } = string.Empty; public string RecommendedNextAction { get; set; } = string.Empty; + public IReadOnlyList CapturedVariables { get; set; } = + Array.Empty(); + public bool CapturedVariablesTruncated { get; set; } internal static PausePointStatusResponse FromSnapshot(UloopPausePointSnapshot snapshot) { @@ -93,7 +98,11 @@ internal static PausePointStatusResponse FromSnapshot(UloopPausePointSnapshot sn FirstHitSequence = snapshot.FirstHitSequence, LastHitSequence = snapshot.LastHitSequence, Message = snapshot.Message, - RecommendedNextAction = snapshot.RecommendedNextAction + RecommendedNextAction = snapshot.RecommendedNextAction, + CapturedVariables = snapshot.CapturedVariables + .Select(PausePointStatusCapturedVariable.FromCapturedVariable) + .ToList(), + CapturedVariablesTruncated = snapshot.CapturedVariablesTruncated }; } } @@ -122,4 +131,38 @@ internal static PausePointStatusEditorState FromSnapshot(UloopPausePointEditorSt }; } } + + /// + /// One variable captured at a source pause point, mirroring Runtime.UloopCapturedVariable's + /// fields for the CLI polling bridge response. + /// + public class PausePointStatusCapturedVariable + { + public string Name { get; set; } = string.Empty; + public string Scope { get; set; } = string.Empty; + public string TypeName { get; set; } = string.Empty; + public string Value { get; set; } = string.Empty; + public string UnityObjectKind { get; set; } = string.Empty; + public string UnityObjectPath { get; set; } = string.Empty; + public int UnityObjectInstanceId { get; set; } + + internal static PausePointStatusCapturedVariable FromCapturedVariable(UloopCapturedVariable capturedVariable) + { + if (capturedVariable == null) + { + throw new ArgumentNullException(nameof(capturedVariable)); + } + + return new PausePointStatusCapturedVariable + { + Name = capturedVariable.Name, + Scope = capturedVariable.Scope, + TypeName = capturedVariable.TypeName, + Value = capturedVariable.Value, + UnityObjectKind = capturedVariable.UnityObjectKind, + UnityObjectPath = capturedVariable.UnityObjectPath, + UnityObjectInstanceId = capturedVariable.UnityObjectInstanceId + }; + } + } } diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index 975756a6ec..5f6956b3cb 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -46,24 +46,26 @@ type pausePointStatusOptions struct { } type pausePointStatusResponse struct { - Id string `json:"Id"` - Status string `json:"Status"` - IsEnabled bool `json:"IsEnabled"` - IsHit bool `json:"IsHit"` - HitCount int `json:"HitCount"` - TimeoutSeconds int `json:"TimeoutSeconds"` - Expired bool `json:"Expired"` - EnabledAtUtc string `json:"EnabledAtUtc"` - ElapsedSinceEnabledMilliseconds int64 `json:"ElapsedSinceEnabledMilliseconds"` - RemainingMilliseconds int64 `json:"RemainingMilliseconds"` - Generation int `json:"Generation"` - EditorState pausePointEditorState `json:"EditorState"` - FirstHitAtUtc string `json:"FirstHitAtUtc"` - LastHitAtUtc string `json:"LastHitAtUtc"` - FirstHitSequence int `json:"FirstHitSequence"` - LastHitSequence int `json:"LastHitSequence"` - Message string `json:"Message"` - RecommendedNextAction string `json:"RecommendedNextAction"` + Id string `json:"Id"` + Status string `json:"Status"` + IsEnabled bool `json:"IsEnabled"` + IsHit bool `json:"IsHit"` + HitCount int `json:"HitCount"` + TimeoutSeconds int `json:"TimeoutSeconds"` + Expired bool `json:"Expired"` + EnabledAtUtc string `json:"EnabledAtUtc"` + ElapsedSinceEnabledMilliseconds int64 `json:"ElapsedSinceEnabledMilliseconds"` + RemainingMilliseconds int64 `json:"RemainingMilliseconds"` + Generation int `json:"Generation"` + EditorState pausePointEditorState `json:"EditorState"` + FirstHitAtUtc string `json:"FirstHitAtUtc"` + LastHitAtUtc string `json:"LastHitAtUtc"` + FirstHitSequence int `json:"FirstHitSequence"` + LastHitSequence int `json:"LastHitSequence"` + Message string `json:"Message"` + RecommendedNextAction string `json:"RecommendedNextAction"` + CapturedVariables []pausePointCapturedVariable `json:"CapturedVariables"` + CapturedVariablesTruncated bool `json:"CapturedVariablesTruncated"` } type pausePointEditorState struct { @@ -72,6 +74,19 @@ type pausePointEditorState struct { CapturedAt string `json:"CapturedAt"` } +// pausePointCapturedVariable mirrors the flat Unity-side +// PausePointStatusCapturedVariable/UloopCapturedVariable DTO field-for-field: one variable +// captured at a source pause point (a local, a parameter, or a `this` instance field). +type pausePointCapturedVariable struct { + Name string `json:"Name"` + Scope string `json:"Scope"` + TypeName string `json:"TypeName"` + Value string `json:"Value"` + UnityObjectKind string `json:"UnityObjectKind"` + UnityObjectPath string `json:"UnityObjectPath"` + UnityObjectInstanceId int `json:"UnityObjectInstanceId"` +} + type pausePointWaitState string const ( diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go index a82c6cb07b..17410ae7e3 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "os" "path/filepath" + "reflect" "strings" "testing" "time" @@ -923,6 +924,115 @@ func TestRunPausePointStatusReturnsCurrentStatus(t *testing.T) { } } +// Verifies pause-point-status passes captured variables and the truncated flag through to stdout. +func TestRunPausePointStatusReturnsCapturedVariables(t *testing.T) { + originalQuery := queryPausePointStatus + defer func() { + queryPausePointStatus = originalQuery + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsEnabled: true, + IsHit: true, + CapturedVariables: []pausePointCapturedVariable{ + { + Name: "speed", + Scope: "Local", + TypeName: "System.Int32", + Value: "5", + }, + { + Name: "enemy", + Scope: "InstanceField", + TypeName: "UnityEngine.GameObject", + Value: "Enemy", + UnityObjectKind: "SceneObject", + UnityObjectPath: "MainScene:/Root/Enemy", + UnityObjectInstanceId: -1234, + }, + }, + CapturedVariablesTruncated: true, + }, nil + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runPausePointStatusCommand( + context.Background(), + unityipc.Connection{ProjectRoot: "/tmp/MyProject"}, + []string{"--id", "jump"}, + &stdout, + &stderr) + + if code != 0 { + t.Fatalf("expected success, got %d with stderr %s", code, stderr.String()) + } + var response pausePointStatusResponse + if err := json.Unmarshal(stdout.Bytes(), &response); err != nil { + t.Fatalf("stdout is not valid JSON: %v\n%s", err, stdout.String()) + } + if !response.CapturedVariablesTruncated { + t.Fatalf("expected CapturedVariablesTruncated to be true: %#v", response) + } + if len(response.CapturedVariables) != 2 { + t.Fatalf("expected 2 captured variables, got %#v", response.CapturedVariables) + } + if response.CapturedVariables[0].Name != "speed" || response.CapturedVariables[0].Value != "5" { + t.Fatalf("first captured variable mismatch: %#v", response.CapturedVariables[0]) + } + second := response.CapturedVariables[1] + if second.Name != "enemy" || second.UnityObjectKind != "SceneObject" || + second.UnityObjectPath != "MainScene:/Root/Enemy" || second.UnityObjectInstanceId != -1234 { + t.Fatalf("second captured variable mismatch: %#v", second) + } +} + +// Verifies pausePointCapturedVariable round-trips through JSON without losing or reordering fields. +func TestPausePointStatusResponseCapturedVariablesJSONRoundTrip(t *testing.T) { + original := pausePointStatusResponse{ + Id: "jump", + Status: pausePointStatusHit, + CapturedVariables: []pausePointCapturedVariable{ + { + Name: "speed", + Scope: "Local", + TypeName: "System.Int32", + Value: "5", + UnityObjectKind: "SceneObject", + UnityObjectPath: "MainScene:/Root/Enemy", + UnityObjectInstanceId: -1234, + }, + }, + CapturedVariablesTruncated: true, + } + + marshaled, err := json.Marshal(original) + if err != nil { + t.Fatalf("marshal failed: %v", err) + } + + var roundTripped pausePointStatusResponse + if err := json.Unmarshal(marshaled, &roundTripped); err != nil { + t.Fatalf("unmarshal failed: %v", err) + } + + if !reflect.DeepEqual(original.CapturedVariables, roundTripped.CapturedVariables) { + t.Fatalf("captured variables mismatch after round trip: got %#v, want %#v", + roundTripped.CapturedVariables, original.CapturedVariables) + } + if original.CapturedVariablesTruncated != roundTripped.CapturedVariablesTruncated { + t.Fatalf("capturedVariablesTruncated mismatch after round trip: got %v, want %v", + roundTripped.CapturedVariablesTruncated, original.CapturedVariablesTruncated) + } +} + // Verifies pause-point-status derives Expired from Status when older Unity packages omit the bool field. func TestRunPausePointStatusDerivesExpiredFromStatus(t *testing.T) { originalQuery := queryPausePointStatus From 4b7b6644c3149d5db22f4bb12f8429bfa7540c9b Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 11:54:09 +0900 Subject: [PATCH 5/5] Add captured variables contract shared between Go and C# Extends the get-logs contract pattern to the pause-point status response so the CapturedVariables/CapturedVariablesTruncated fields added for source file:line pause points stay in sync between the Go CLI and the Unity package: a shared JSON fixture plus a Go test and a C# test that each round-trip it through their own DTO and compare the normalized field shape. --- .../PausePointStatusResponseContractTests.cs | 125 ++++++++++++++++++ ...sePointStatusResponseContractTests.cs.meta | 11 ++ ...use_point_status_response_contract_test.go | 44 ++++++ .../pause_point_status_response_contract.json | 36 +++++ 4 files changed, 216 insertions(+) create mode 100644 Assets/Tests/Editor/PausePointStatusResponseContractTests.cs create mode 100644 Assets/Tests/Editor/PausePointStatusResponseContractTests.cs.meta create mode 100644 cli/project-runner/internal/projectrunner/pause_point_status_response_contract_test.go create mode 100644 tests/contracts/pause_point_status_response_contract.json diff --git a/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs b/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs new file mode 100644 index 0000000000..2360a072a3 --- /dev/null +++ b/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs @@ -0,0 +1,125 @@ +using System.Collections.Generic; +using System.IO; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Infrastructure; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies the Unity pause-point-status response (including captured + /// variables) stays aligned with the shared Go CLI contract. + /// + public sealed class PausePointStatusResponseContractTests + { + private const string SharedContractPath = "tests/contracts/pause_point_status_response_contract.json"; + + [Test] + public void PausePointStatusResponse_WhenSerialized_MatchesSharedContractFieldShape() + { + // Verifies C# does not add, remove, or rename fields (including CapturedVariables) without + // updating the shared CLI contract. + JObject expected = ReadSharedContractFieldShape(); + PausePointStatusResponse response = new() + { + Id = "Assets/Scripts/Enemy.cs:42", + Status = "Hit", + IsEnabled = true, + IsHit = true, + HitCount = 1, + TimeoutSeconds = 30, + Expired = false, + EnabledAtUtc = "2026-06-03T00:00:00.0000000Z", + ElapsedSinceEnabledMilliseconds = 1200, + RemainingMilliseconds = 28800, + Generation = 3, + EditorState = new PausePointStatusEditorState + { + IsPlaying = true, + IsPaused = true, + CapturedAt = "PausePointHit" + }, + FirstHitAtUtc = "2026-06-03T00:00:01.0000000Z", + LastHitAtUtc = "2026-06-03T00:00:01.0000000Z", + FirstHitSequence = 1, + LastHitSequence = 1, + Message = "Pause point hit.", + RecommendedNextAction = "Clear this marker, then re-enable it with the same Id and TimeoutSeconds values.", + CapturedVariables = new List + { + new() + { + Name = "target", + Scope = "InstanceField", + TypeName = "UnityEngine.GameObject", + Value = "Enemy", + UnityObjectKind = "SceneObject", + UnityObjectPath = "MainScene:/Root/Enemy", + UnityObjectInstanceId = -1234 + } + }, + CapturedVariablesTruncated = true + }; + string json = JsonConvert.SerializeObject( + response, + Formatting.None, + UnityCliLoopJsonResponseSerializerSettings.Settings); + JObject actual = NormalizeFieldShape(JObject.Parse(json)); + + Assert.That(JToken.DeepEquals(actual, expected), Is.True, $"Expected {expected} but got {actual}"); + } + + private static JObject ReadSharedContractFieldShape() + { + string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); + string json = File.ReadAllText(Path.Combine(projectRoot, SharedContractPath)); + return NormalizeFieldShape(JObject.Parse(json)); + } + + private static JObject NormalizeFieldShape(JObject value) + { + JObject shape = new(); + foreach (JProperty property in value.Properties()) + { + shape[property.Name] = NormalizeTokenFieldShape(property.Value); + } + return shape; + } + + private static JToken NormalizeTokenFieldShape(JToken value) + { + if (value is JObject objectValue) + { + return NormalizeFieldShape(objectValue); + } + + if (value is JArray arrayValue) + { + JArray arrayShape = new(); + if (arrayValue.Count > 0) + { + arrayShape.Add(NormalizeTokenFieldShape(arrayValue[0])); + } + return arrayShape; + } + + return new JValue(NormalizeScalarType(value.Type)); + } + + private static string NormalizeScalarType(JTokenType type) + { + return type switch + { + JTokenType.Boolean => "boolean", + JTokenType.Float => "number", + JTokenType.Integer => "number", + JTokenType.Null => "null", + JTokenType.String => "string", + _ => "unknown" + }; + } + } +} diff --git a/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs.meta b/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs.meta new file mode 100644 index 0000000000..861688b845 --- /dev/null +++ b/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7810f9394bc7741588a05981aec61339 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/cli/project-runner/internal/projectrunner/pause_point_status_response_contract_test.go b/cli/project-runner/internal/projectrunner/pause_point_status_response_contract_test.go new file mode 100644 index 0000000000..4ec501f43d --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_status_response_contract_test.go @@ -0,0 +1,44 @@ +package projectrunner + +import ( + "encoding/json" + "os" + "reflect" + "testing" +) + +const pausePointStatusResponseContractPath = "tests/contracts/pause_point_status_response_contract.json" + +// Verifies the Go pausePointStatusResponse DTO preserves every field in the shared Unity +// PausePointStatusResponse contract, including the CapturedVariables/CapturedVariablesTruncated +// fields introduced for source file:line pause points. +func TestPausePointStatusResponseMatchesSharedContract(t *testing.T) { + fixture := readPausePointStatusResponseContract(t) + + var response pausePointStatusResponse + if err := json.Unmarshal(fixture, &response); err != nil { + t.Fatalf("failed to unmarshal shared pause point status response contract: %v", err) + } + + roundTripped, err := json.Marshal(response) + if err != nil { + t.Fatalf("failed to marshal Go pause point status response: %v", err) + } + + expected := readJSONFieldShape(t, fixture) + actual := readJSONFieldShape(t, roundTripped) + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("pause point status response field shape drifted\nexpected: %#v\nactual: %#v", expected, actual) + } +} + +func readPausePointStatusResponseContract(t *testing.T) []byte { + t.Helper() + + path := findRepoRelativeFile(t, pausePointStatusResponseContractPath) + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("failed to read shared pause point status response contract: %v", err) + } + return data +} diff --git a/tests/contracts/pause_point_status_response_contract.json b/tests/contracts/pause_point_status_response_contract.json new file mode 100644 index 0000000000..dbc4145810 --- /dev/null +++ b/tests/contracts/pause_point_status_response_contract.json @@ -0,0 +1,36 @@ +{ + "Id": "Assets/Scripts/Enemy.cs:42", + "Status": "Hit", + "IsEnabled": true, + "IsHit": true, + "HitCount": 1, + "TimeoutSeconds": 30, + "Expired": false, + "EnabledAtUtc": "2026-06-03T00:00:00.0000000Z", + "ElapsedSinceEnabledMilliseconds": 1200, + "RemainingMilliseconds": 28800, + "Generation": 3, + "EditorState": { + "IsPlaying": true, + "IsPaused": true, + "CapturedAt": "PausePointHit" + }, + "FirstHitAtUtc": "2026-06-03T00:00:01.0000000Z", + "LastHitAtUtc": "2026-06-03T00:00:01.0000000Z", + "FirstHitSequence": 1, + "LastHitSequence": 1, + "Message": "Pause point hit.", + "RecommendedNextAction": "Clear this marker, then re-enable it with the same Id and TimeoutSeconds values.", + "CapturedVariables": [ + { + "Name": "target", + "Scope": "InstanceField", + "TypeName": "UnityEngine.GameObject", + "Value": "Enemy", + "UnityObjectKind": "SceneObject", + "UnityObjectPath": "MainScene:/Root/Enemy", + "UnityObjectInstanceId": -1234 + } + ], + "CapturedVariablesTruncated": true +}