From a530acd696f1f76182cbca512bd91b40088b43a7 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 4 Jul 2026 16:24:04 +0900 Subject: [PATCH 1/2] Remove dispatcher CliConstants regex fallback and require the project runner pin The fallback parsed MINIMUM_REQUIRED_PROJECT_RUNNER_VERSION out of C# source with a regex and substituted a semantically wrong value (the setup minimum instead of the pinned latest release), while hiding pin synchronization failures. The pin JSON is now the only runtime source for resolving the project runner version, so a missing pin fails fast with the existing resolution guidance instead of silently running a stale runner. - Delete readDispatcherPinFromCliConstants, its candidate paths, and the CliConstants regex constants; replace fallback tests with missing-pin error coverage - Log a warning in CliPinSynchronizer when the package source pin is missing instead of returning false silently - Keep editor bootstrap running when pin sync fails so one bad pin cannot take down the rest of initialization --- .../Tests/Editor/CliPinSynchronizerTests.cs | 38 ++++++++++ .../UnityCliLoopEditorBootstrapper.cs | 5 +- .../Infrastructure/CLI/CliPinSynchronizer.cs | 8 +- .../internal/dispatcher/dispatcher_pin.go | 75 +------------------ .../internal/dispatcher/dispatcher_test.go | 58 +++++++------- 5 files changed, 75 insertions(+), 109 deletions(-) diff --git a/Assets/Tests/Editor/CliPinSynchronizerTests.cs b/Assets/Tests/Editor/CliPinSynchronizerTests.cs index 3bc7741cbc..0edb4ef9be 100644 --- a/Assets/Tests/Editor/CliPinSynchronizerTests.cs +++ b/Assets/Tests/Editor/CliPinSynchronizerTests.cs @@ -111,6 +111,44 @@ public void SyncProjectPinFile_WhenPackagePinChanges_ShouldUpdateProjectPin() } } + [Test] + public void SyncProjectPinFile_WhenSourcePinMissing_ShouldLogWarningAndSkip() + { + // Tests that a missing package source pin now emits a warning instead of silently returning false. + string root = CreateTestRoot(); + string packageRoot = Path.Combine(root, "package"); + string projectRoot = Path.Combine(root, "project"); + + try + { + Directory.CreateDirectory(packageRoot); + Directory.CreateDirectory(projectRoot); + string sourcePath = Path.Combine( + packageRoot, + UnityCliLoopConstants.ULOOP_PROJECT_RUNNER_PIN_FILE_NAME); + LogAssert.Expect( + LogType.Warning, + new System.Text.RegularExpressions.Regex( + $"Unity CLI Loop skipped {System.Text.RegularExpressions.Regex.Escape(UnityCliLoopConstants.ULOOP_PROJECT_RUNNER_PIN_FILE_NAME)} synchronization because the package source pin was not found")); + + bool changed = CliPinSynchronizer.SyncProjectPinFile(packageRoot, projectRoot); + + Assert.That(changed, Is.False); + Assert.That(File.Exists(sourcePath), Is.False); + Assert.That( + File.Exists( + Path.Combine( + projectRoot, + UnityCliLoopConstants.ULOOP_DIR, + UnityCliLoopConstants.ULOOP_PROJECT_RUNNER_PIN_FILE_NAME)), + Is.False); + } + finally + { + Directory.Delete(root, recursive: true); + } + } + [Test] public void SyncProjectPinFile_WhenPackageRootMissing_ShouldSkipWrite() { diff --git a/Packages/src/Editor/CompositionRoot/UnityCliLoopEditorBootstrapper.cs b/Packages/src/Editor/CompositionRoot/UnityCliLoopEditorBootstrapper.cs index a49a66d506..bcaeb48377 100644 --- a/Packages/src/Editor/CompositionRoot/UnityCliLoopEditorBootstrapper.cs +++ b/Packages/src/Editor/CompositionRoot/UnityCliLoopEditorBootstrapper.cs @@ -20,7 +20,10 @@ internal UnityCliLoopEditorBootstrapper() internal void Initialize() { - CliPinSynchronizer.SyncCurrentProjectPin(); + // Why: pin synchronization is now mandatory for the dispatcher, but a false result (missing + // source pin, or destination already matches) is signalled via the return value and must not + // abort the remaining startup steps. CliPinSynchronizer logs its own warnings on failure. + _ = CliPinSynchronizer.SyncCurrentProjectPin(); UnityCliLoopApplicationServices applicationServices = _applicationRegistration.Register(); ApplicationEditorStartup.Initialize(applicationServices.DomainReloadDetectionService); FirstPartyToolsEditorStartup.Initialize(); diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPinSynchronizer.cs b/Packages/src/Editor/Infrastructure/CLI/CliPinSynchronizer.cs index e65506f4ab..372322c4e6 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliPinSynchronizer.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliPinSynchronizer.cs @@ -11,9 +11,9 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure /// internal static class CliPinSynchronizer { - internal static void SyncCurrentProjectPin() + internal static bool SyncCurrentProjectPin() { - SyncProjectPinFile(UnityCliLoopConstants.PackageResolvedPath, ResolveCurrentProjectRoot(UnityEngine.Application.dataPath)); + return SyncProjectPinFile(UnityCliLoopConstants.PackageResolvedPath, ResolveCurrentProjectRoot(UnityEngine.Application.dataPath)); } internal static string ResolveCurrentProjectRoot(string assetsPath) @@ -61,6 +61,10 @@ private static bool SyncProjectPinFileByName(string packageRoot, string projectR if (!File.Exists(sourcePath)) { + // Why: the pin JSON is the only runtime source for the dispatcher's project runner version, + // so a missing package source must be visible instead of silently skipping synchronization. + Debug.LogWarning( + $"Unity CLI Loop skipped {pinFileName} synchronization because the package source pin was not found at {sourcePath}."); return false; } diff --git a/cli/dispatcher/internal/dispatcher/dispatcher_pin.go b/cli/dispatcher/internal/dispatcher/dispatcher_pin.go index 38c7b8f853..0e2465467c 100644 --- a/cli/dispatcher/internal/dispatcher/dispatcher_pin.go +++ b/cli/dispatcher/internal/dispatcher/dispatcher_pin.go @@ -10,12 +10,7 @@ import ( "strings" ) -var ( - dispatcherMinimumProjectRunnerVersionPattern = regexp.MustCompile(`MINIMUM_REQUIRED_PROJECT_RUNNER_VERSION\s*=\s*"([^"]+)"`) - dispatcherMinimumVersionPattern = regexp.MustCompile(`MINIMUM_REQUIRED_DISPATCHER_VERSION\s*=\s*"([^"]+)"`) - dispatcherRequiredProtocolVersionPattern = regexp.MustCompile(`REQUIRED_CLI_PROTOCOL_VERSION\s*=\s*(\d+)`) - dispatcherProjectRunnerVersionPattern = regexp.MustCompile(`^[0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z][0-9A-Za-z.-]*)?(?:\+[0-9A-Za-z][0-9A-Za-z.-]*)?$`) -) +var dispatcherProjectRunnerVersionPattern = regexp.MustCompile(`^[0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z][0-9A-Za-z.-]*)?(?:\+[0-9A-Za-z][0-9A-Za-z.-]*)?$`) type dispatcherPin struct { SchemaVersion int `json:"schemaVersion"` @@ -51,16 +46,6 @@ func loadDispatcherPin(projectRoot string) (dispatcherPin, error) { } } - for _, constantsPath := range dispatcherCliConstantsCandidatePaths(projectRoot) { - pin, err := readDispatcherPinFromCliConstants(constantsPath) - if err == nil { - return pin, nil - } - if !errors.Is(err, os.ErrNotExist) { - return dispatcherPin{}, err - } - } - if invalidPackagePinError != nil { return dispatcherPin{}, invalidPackagePinError } @@ -88,26 +73,6 @@ func dispatcherPinCandidatePaths(projectRoot string) []dispatcherPinCandidatePat return paths } -func dispatcherCliConstantsCandidatePaths(projectRoot string) []string { - paths := []string{ - filepath.Join(projectRoot, "Packages", "src", "Editor", "Domain", "CliConstants.cs"), - filepath.Join(projectRoot, "Packages", dispatcherUnityPackageName, "Editor", "Domain", "CliConstants.cs"), - } - packageCachePattern := filepath.Join( - projectRoot, - "Library", - "PackageCache", - dispatcherUnityPackageName+"@*", - "Editor", - "Domain", - "CliConstants.cs") - matches, err := filepath.Glob(packageCachePattern) - if err == nil { - paths = append(paths, matches...) - } - return paths -} - func readDispatcherPin(pinPath string) (dispatcherPin, error) { content, err := os.ReadFile(pinPath) if err != nil { @@ -138,44 +103,6 @@ func readDispatcherPin(pinPath string) (dispatcherPin, error) { return pin, nil } -func readDispatcherPinFromCliConstants(constantsPath string) (dispatcherPin, error) { - content, err := os.ReadFile(constantsPath) - if err != nil { - return dispatcherPin{}, err - } - text := string(content) - versionMatch := dispatcherMinimumProjectRunnerVersionPattern.FindStringSubmatch(text) - if len(versionMatch) != 2 { - return dispatcherPin{}, fmt.Errorf("%s does not define MINIMUM_REQUIRED_PROJECT_RUNNER_VERSION", constantsPath) - } - projectRunnerVersion := normalizeDispatcherVersion(versionMatch[1]) - if err := validateDispatcherProjectRunnerVersion(projectRunnerVersion); err != nil { - return dispatcherPin{}, fmt.Errorf("%s defines invalid MINIMUM_REQUIRED_PROJECT_RUNNER_VERSION: %w", constantsPath, err) - } - dispatcherVersionMatch := dispatcherMinimumVersionPattern.FindStringSubmatch(text) - if len(dispatcherVersionMatch) != 2 { - return dispatcherPin{}, fmt.Errorf("%s does not define MINIMUM_REQUIRED_DISPATCHER_VERSION", constantsPath) - } - minimumDispatcherVersion := normalizeDispatcherVersion(dispatcherVersionMatch[1]) - if err := validateDispatcherProjectRunnerVersion(minimumDispatcherVersion); err != nil { - return dispatcherPin{}, fmt.Errorf("%s defines invalid MINIMUM_REQUIRED_DISPATCHER_VERSION: %w", constantsPath, err) - } - protocolVersion := 0 - protocolMatch := dispatcherRequiredProtocolVersionPattern.FindStringSubmatch(text) - if len(protocolMatch) == 2 { - _, _ = fmt.Sscanf(protocolMatch[1], "%d", &protocolVersion) - } - - return dispatcherPin{ - SchemaVersion: 1, - PackageName: dispatcherUnityPackageName, - ProjectRunnerVersion: projectRunnerVersion, - RequiredProtocolVersion: protocolVersion, - MinimumDispatcherVersion: minimumDispatcherVersion, - SourcePath: constantsPath, - }, nil -} - func normalizeDispatcherVersion(value string) string { trimmed := strings.TrimSpace(value) if strings.HasPrefix(trimmed, "v") || strings.HasPrefix(trimmed, "V") { diff --git a/cli/dispatcher/internal/dispatcher/dispatcher_test.go b/cli/dispatcher/internal/dispatcher/dispatcher_test.go index f373bb6c8d..3ea232275a 100644 --- a/cli/dispatcher/internal/dispatcher/dispatcher_test.go +++ b/cli/dispatcher/internal/dispatcher/dispatcher_test.go @@ -770,8 +770,8 @@ func TestLoadDispatcherPinRejectsInvalidMinimumDispatcherVersion(t *testing.T) { } } -func TestLoadDispatcherPinFallsBackToCliConstants(t *testing.T) { - // Verifies old package layouts can still resolve a CLI version from CliConstants.cs. +func TestLoadDispatcherPinFailsWhenPinFileMissing(t *testing.T) { + // Verifies loadDispatcherPin now requires a pin JSON and does not fall back to CliConstants.cs. projectRoot := createDispatcherUnityProject(t) constantsPath := filepath.Join(projectRoot, "Packages", "src", "Editor", "Domain", "CliConstants.cs") if err := os.MkdirAll(filepath.Dir(constantsPath), 0o755); err != nil { @@ -784,44 +784,38 @@ public const string MINIMUM_REQUIRED_DISPATCHER_VERSION = "1.0.0";` t.Fatalf("failed to write constants: %v", err) } - pin, err := loadDispatcherPin(projectRoot) - if err != nil { - t.Fatalf("loadDispatcherPin failed: %v", err) - } - if pin.ProjectRunnerVersion != "3.0.0-beta.56" { - t.Fatalf("projectRunnerVersion mismatch: %s", pin.ProjectRunnerVersion) - } - if pin.RequiredProtocolVersion != 3 { - t.Fatalf("protocol mismatch: %d", pin.RequiredProtocolVersion) + _, err := loadDispatcherPin(projectRoot) + + if err == nil { + t.Fatal("expected pin resolution to fail when no pin JSON is present") } - if pin.MinimumDispatcherVersion != "1.0.0" { - t.Fatalf("minimumDispatcherVersion mismatch: %s", pin.MinimumDispatcherVersion) + if !strings.Contains(err.Error(), "project runner pin not found") { + t.Fatalf("expected 'project runner pin not found' message, got: %v", err) } } -func TestLoadDispatcherPinFromCliConstantsNormalizesVersionPrefix(t *testing.T) { - // Verifies v-prefixed fallback constants are normalized before dispatcher resolution. +func TestRunDispatcherMissingPinEmitsPinResolutionGuidance(t *testing.T) { + // Verifies the dispatcher surfaces the pin-resolution error envelope with NextActions guidance when the pin is missing. projectRoot := createDispatcherUnityProject(t) - constantsPath := filepath.Join(projectRoot, "Packages", "src", "Editor", "Domain", "CliConstants.cs") - if err := os.MkdirAll(filepath.Dir(constantsPath), 0o755); err != nil { - t.Fatalf("failed to create constants directory: %v", err) - } - content := `public const int REQUIRED_CLI_PROTOCOL_VERSION = 3; -public const string MINIMUM_REQUIRED_PROJECT_RUNNER_VERSION = "v3.0.0-beta.59"; -public const string MINIMUM_REQUIRED_DISPATCHER_VERSION = "v1.0.0";` - if err := os.WriteFile(constantsPath, []byte(content), 0o644); err != nil { - t.Fatalf("failed to write constants: %v", err) - } + t.Setenv(nativepath.CacheDirEnvName, t.TempDir()) + t.Setenv(dispatcherDisableSelfUpdateEnvName, "1") + t.Chdir(projectRoot) - pin, err := loadDispatcherPin(projectRoot) - if err != nil { - t.Fatalf("loadDispatcherPin failed: %v", err) + deps := defaultDispatcherRunDeps() + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runDispatcherWithDeps(context.Background(), []string{"compile"}, &stdout, &stderr, deps) + + if code == 0 { + t.Fatalf("expected non-zero exit when pin is missing, stderr=%s", stderr.String()) } - if pin.ProjectRunnerVersion != "3.0.0-beta.59" { - t.Fatalf("projectRunnerVersion mismatch: %s", pin.ProjectRunnerVersion) + stderrText := stderr.String() + if !strings.Contains(stderrText, "Could not resolve the required uloop project runner") { + t.Fatalf("expected pin resolution error message, got: %s", stderrText) } - if pin.MinimumDispatcherVersion != "1.0.0" { - t.Fatalf("minimumDispatcherVersion mismatch: %s", pin.MinimumDispatcherVersion) + if !strings.Contains(stderrText, "project-runner-pin.json") { + t.Fatalf("expected NextActions to reference project-runner-pin.json, got: %s", stderrText) } } From a6c8bf10e90bc79e0f258344de61d9f6ac0be19b Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 5 Jul 2026 14:03:09 +0900 Subject: [PATCH 2/2] Assert the full pin sync warning including the source path The prefix-only regex would keep passing if the warning dropped the missing source path, which is the part that makes the diagnostic actionable. Exact-match the whole message instead. --- Assets/Tests/Editor/CliPinSynchronizerTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Assets/Tests/Editor/CliPinSynchronizerTests.cs b/Assets/Tests/Editor/CliPinSynchronizerTests.cs index 0edb4ef9be..08dddbcfe4 100644 --- a/Assets/Tests/Editor/CliPinSynchronizerTests.cs +++ b/Assets/Tests/Editor/CliPinSynchronizerTests.cs @@ -128,8 +128,7 @@ public void SyncProjectPinFile_WhenSourcePinMissing_ShouldLogWarningAndSkip() UnityCliLoopConstants.ULOOP_PROJECT_RUNNER_PIN_FILE_NAME); LogAssert.Expect( LogType.Warning, - new System.Text.RegularExpressions.Regex( - $"Unity CLI Loop skipped {System.Text.RegularExpressions.Regex.Escape(UnityCliLoopConstants.ULOOP_PROJECT_RUNNER_PIN_FILE_NAME)} synchronization because the package source pin was not found")); + $"Unity CLI Loop skipped {UnityCliLoopConstants.ULOOP_PROJECT_RUNNER_PIN_FILE_NAME} synchronization because the package source pin was not found at {sourcePath}."); bool changed = CliPinSynchronizer.SyncProjectPinFile(packageRoot, projectRoot);