From d578c3870da3a03ff1d19bd4f857b46f1e8621f3 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Fri, 17 Jul 2026 20:49:21 +0900 Subject: [PATCH 1/6] fix: make cli Go tests pass on Windows and run them in CI (#1817) --- .github/workflows/build-and-test.yml | 14 +- .../internal/dispatcher/launch_test.go | 26 +- .../internal/install/command_test.go | 10 +- cli/project-runner/go.mod | 8 +- .../control_play_mode_wait_test.go | 66 ++- .../ipc_listener_fixture_unix_test.go | 24 ++ .../ipc_listener_fixture_windows_test.go | 34 ++ .../automation/contract_file_at_ref_test.go | 5 + .../dispatcher_minimum_version_guard_test.go | 6 + .../mock_cli_executable_for_tests_test.go | 147 +++++++ .../automation/mock_git_for_tests_test.go | 7 +- .../protocol_minimum_version_guard_test.go | 11 + .../automation/testdata/mockcli/main.go | 406 ++++++++++++++++++ 13 files changed, 709 insertions(+), 55 deletions(-) create mode 100644 cli/project-runner/internal/projectrunner/ipc_listener_fixture_unix_test.go create mode 100644 cli/project-runner/internal/projectrunner/ipc_listener_fixture_windows_test.go create mode 100644 cli/release-automation/internal/automation/mock_cli_executable_for_tests_test.go create mode 100644 cli/release-automation/internal/automation/testdata/mockcli/main.go diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index ff026b022..8cd715abe 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -119,12 +119,16 @@ jobs: cache: false # Windows-only code paths (named pipe transport, winio deadlines, typed error - # classification) are otherwise never executed by CI. Scoped to the transport - # package: the wider test suite still has Windows-incompatible tests (path - # separators, missing .exe suffixes, TCP-only fixtures) that predate this job. - - name: Test native Go CLI transport on Windows + # classification, process management) are otherwise never executed by CI. + # Runs every go.work module: the formerly Windows-incompatible tests (POSIX-only + # fake executables, TCP-only fixtures, shell-script git/gh mocks) now install + # portable fixtures on Windows. + - name: Test native Go CLI on Windows shell: bash - run: cd cli/common && go test ./unityipc/ + run: | + for module in common dispatcher project-runner release-automation; do + (cd "cli/$module" && go test ./...) + done - name: Test install release filter in Git Bash shell: bash diff --git a/cli/dispatcher/internal/dispatcher/launch_test.go b/cli/dispatcher/internal/dispatcher/launch_test.go index f6f1a37a1..2e269c734 100644 --- a/cli/dispatcher/internal/dispatcher/launch_test.go +++ b/cli/dispatcher/internal/dispatcher/launch_test.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "slices" "strings" "testing" @@ -264,8 +265,9 @@ func TestRunLaunchWritesReadyResponseAfterToolReadiness(t *testing.T) { deps.findRunningUnityProcess = func(context.Context, string) (*clicore.UnityProcess, error) { return nil, nil } + fakeUnityPath := fakeUnityExecutablePath(t) deps.resolveUnityExecutablePath = func(string) (string, error) { - return "/usr/bin/true", nil + return fakeUnityPath, nil } deps.waitForUnityStartupMarker = func(context.Context, string, time.Duration, time.Duration) error { return nil @@ -506,8 +508,9 @@ func TestRunLaunchRestartWritesProcessTransitionResponse(t *testing.T) { waitedPid = pid return nil } + fakeUnityPath := fakeUnityExecutablePath(t) deps.resolveUnityExecutablePath = func(string) (string, error) { - return "/usr/bin/true", nil + return fakeUnityPath, nil } deps.waitForUnityStartupMarker = func(context.Context, string, time.Duration, time.Duration) error { return nil @@ -609,9 +612,10 @@ func TestRunLaunchRestartReportsProcessExitWaitFailure(t *testing.T) { deps.waitForUnityProcessExit = func(ctx context.Context, projectRoot string, pid int, pollInterval time.Duration, timeout time.Duration) error { return errors.New("still exiting") } + fakeUnityPath := fakeUnityExecutablePath(t) deps.resolveUnityExecutablePath = func(string) (string, error) { resolverCalled = true - return "/usr/bin/true", nil + return fakeUnityPath, nil } projectRoot := createLaunchTestProject(t) @@ -830,6 +834,22 @@ func TestResolveExistingUnityExecutablePathReportsSearchedCandidates(t *testing. } } +// fakeUnityExecutablePath returns a no-op executable standing in for Unity in +// launch tests that really spawn the resolved path. Why: /usr/bin/true does +// not exist on Windows, so Windows needs a native no-op batch file instead. +func fakeUnityExecutablePath(t *testing.T) string { + t.Helper() + + if runtime.GOOS != "windows" { + return "/usr/bin/true" + } + path := filepath.Join(t.TempDir(), "fake-unity.bat") + if err := os.WriteFile(path, []byte("@exit /b 0\r\n"), 0o755); err != nil { + t.Fatalf("failed to write fake Unity executable: %v", err) + } + return path +} + func createLaunchTestProject(t *testing.T) string { t.Helper() diff --git a/cli/dispatcher/internal/install/command_test.go b/cli/dispatcher/internal/install/command_test.go index dc4f556c6..7f01492ce 100644 --- a/cli/dispatcher/internal/install/command_test.go +++ b/cli/dispatcher/internal/install/command_test.go @@ -714,13 +714,19 @@ func TestWindowsInstallScriptParsesOnWindows(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() + // The script path must be embedded in the command text: powershell -Command + // concatenates trailing arguments into the command instead of exposing them + // via $args, so a trailing path argument would be invoked as a command and + // execute the installer setup for real. + parseCommand := `$parseErrors = $null; $null = [System.Management.Automation.PSParser]::Tokenize((Get-Content -Raw -LiteralPath ` + + powerShellSingleQuote(scriptPath) + + `), [ref]$parseErrors); if ($parseErrors) { $parseErrors | Out-String; exit 1 }` command := exec.CommandContext( ctx, "powershell", "-NoProfile", "-Command", - `$parseErrors = $null; $null = [System.Management.Automation.PSParser]::Tokenize((Get-Content -Raw $args[0]), [ref]$parseErrors); if ($parseErrors) { $parseErrors | Out-String; exit 1 }`, - scriptPath) + parseCommand) output, err := command.CombinedOutput() if err != nil { t.Fatalf("embedded setup script does not parse: %v\n%s", err, output) diff --git a/cli/project-runner/go.mod b/cli/project-runner/go.mod index b6be3b3a2..e22b7dda4 100644 --- a/cli/project-runner/go.mod +++ b/cli/project-runner/go.mod @@ -2,11 +2,11 @@ module github.com/hatayama/unity-cli-loop/project-runner go 1.26 -require github.com/hatayama/unity-cli-loop/common v0.0.0-00010101000000-000000000000 - require ( - github.com/Microsoft/go-winio v0.6.2 // indirect - golang.org/x/sys v0.10.0 // indirect + github.com/Microsoft/go-winio v0.6.2 + github.com/hatayama/unity-cli-loop/common v0.0.0-00010101000000-000000000000 ) +require golang.org/x/sys v0.10.0 // indirect + replace github.com/hatayama/unity-cli-loop/common => ../common diff --git a/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go b/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go index a89c2abf0..6d8679193 100644 --- a/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go +++ b/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go @@ -5,7 +5,9 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" + "io" "net" "testing" "time" @@ -23,13 +25,7 @@ func TestRunControlPlayModeWithStateWaitPollsStatusAfterStaleInitialResponse(t * controlPlayModeStatePoll = originalPoll }) - listener, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - t.Fatalf("failed to listen: %v", err) - } - defer func() { - _ = listener.Close() - }() + listener := newLoopbackIpcListener(t) requests := make(chan map[string]any, 2) serverErr := make(chan error, 1) @@ -44,7 +40,7 @@ func TestRunControlPlayModeWithStateWaitPollsStatusAfterStaleInitialResponse(t * connection := unityipc.Connection{ Endpoint: unityipc.Endpoint{ - Network: "tcp", + Network: listener.Addr().Network(), Address: listener.Addr().String(), }, ProjectRoot: t.TempDir(), @@ -101,13 +97,7 @@ func TestRunControlPlayModeWithStateWaitPreservesStopChangeFields(t *testing.T) controlPlayModeStatePoll = originalPoll }) - listener, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - t.Fatalf("failed to listen: %v", err) - } - defer func() { - _ = listener.Close() - }() + listener := newLoopbackIpcListener(t) serverErr := make(chan error, 1) go serveControlPlayModeResponses( @@ -121,7 +111,7 @@ func TestRunControlPlayModeWithStateWaitPreservesStopChangeFields(t *testing.T) connection := unityipc.Connection{ Endpoint: unityipc.Endpoint{ - Network: "tcp", + Network: listener.Addr().Network(), Address: listener.Addr().String(), }, ProjectRoot: t.TempDir(), @@ -184,13 +174,7 @@ func TestRunControlPlayModeWithStateWaitFailsWhenStateNeverMatches(t *testing.T) controlPlayModeStatePoll = originalPoll }) - listener, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - t.Fatalf("failed to listen: %v", err) - } - defer func() { - _ = listener.Close() - }() + listener := newLoopbackIpcListener(t) serverErr := make(chan error, 1) go serveRepeatedControlPlayModeResponse( @@ -200,7 +184,7 @@ func TestRunControlPlayModeWithStateWaitFailsWhenStateNeverMatches(t *testing.T) connection := unityipc.Connection{ Endpoint: unityipc.Endpoint{ - Network: "tcp", + Network: listener.Addr().Network(), Address: listener.Addr().String(), }, ProjectRoot: t.TempDir(), @@ -240,13 +224,7 @@ func TestRunControlPlayModeWithStateWaitFailsImmediatelyWhenCompileErrorsBlockPl controlPlayModeStatePoll = originalPoll }) - listener, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - t.Fatalf("failed to listen: %v", err) - } - defer func() { - _ = listener.Close() - }() + listener := newLoopbackIpcListener(t) requests := make(chan map[string]any, 2) serverErr := make(chan error, 1) @@ -260,7 +238,7 @@ func TestRunControlPlayModeWithStateWaitFailsImmediatelyWhenCompileErrorsBlockPl connection := unityipc.Connection{ Endpoint: unityipc.Endpoint{ - Network: "tcp", + Network: listener.Addr().Network(), Address: listener.Addr().String(), }, ProjectRoot: t.TempDir(), @@ -321,13 +299,7 @@ func TestRunControlPlayModeWithStateWaitFailsWhenCompileErrorsAppearDuringPollin controlPlayModeStatePoll = originalPoll }) - listener, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - t.Fatalf("failed to listen: %v", err) - } - defer func() { - _ = listener.Close() - }() + listener := newLoopbackIpcListener(t) requests := make(chan map[string]any, 3) serverErr := make(chan error, 1) @@ -342,7 +314,7 @@ func TestRunControlPlayModeWithStateWaitFailsWhenCompileErrorsAppearDuringPollin connection := unityipc.Connection{ Endpoint: unityipc.Endpoint{ - Network: "tcp", + Network: listener.Addr().Network(), Address: listener.Addr().String(), }, ProjectRoot: t.TempDir(), @@ -421,6 +393,14 @@ func serveRepeatedControlPlayModeResponse( if _, err := unityipc.Read(bufio.NewReader(conn)); err != nil { _ = conn.Close() + // Why tolerated: when the client's wait deadline expires it closes a + // freshly dialed poll connection without sending a request. TCP hides + // this because the request is already buffered in the socket, but a + // named pipe surfaces it as EOF here; either way it is client-side + // cancellation, not a server failure. + if isClientAbandonedConnectionError(err) { + continue + } serverErr <- err return } @@ -481,6 +461,12 @@ func serveControlPlayModeResponses( } } +// isClientAbandonedConnectionError reports whether a fixture-server read +// failed only because the client hung up before sending a request. +func isClientAbandonedConnectionError(err error) bool { + return errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) || errors.Is(err, net.ErrClosed) +} + func readControlPlayModeRequest(t *testing.T, requests <-chan map[string]any) map[string]any { t.Helper() select { diff --git a/cli/project-runner/internal/projectrunner/ipc_listener_fixture_unix_test.go b/cli/project-runner/internal/projectrunner/ipc_listener_fixture_unix_test.go new file mode 100644 index 000000000..02ed0b0cf --- /dev/null +++ b/cli/project-runner/internal/projectrunner/ipc_listener_fixture_unix_test.go @@ -0,0 +1,24 @@ +//go:build !windows + +package projectrunner + +import ( + "net" + "testing" +) + +// newLoopbackIpcListener creates a local listener that the shared client dial +// path can reach in tests. On POSIX the client dials the endpoint network +// verbatim, so a loopback TCP listener stands in for the Unity server. +func newLoopbackIpcListener(t *testing.T) net.Listener { + t.Helper() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + t.Cleanup(func() { + _ = listener.Close() + }) + return listener +} diff --git a/cli/project-runner/internal/projectrunner/ipc_listener_fixture_windows_test.go b/cli/project-runner/internal/projectrunner/ipc_listener_fixture_windows_test.go new file mode 100644 index 000000000..cbbdd58a7 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/ipc_listener_fixture_windows_test.go @@ -0,0 +1,34 @@ +//go:build windows + +package projectrunner + +import ( + "crypto/rand" + "encoding/hex" + "net" + "testing" + + "github.com/Microsoft/go-winio" +) + +// newLoopbackIpcListener creates a local listener that the shared client dial +// path can reach in tests. Why a named pipe: the Windows client dial always +// targets a named pipe, so a TCP fixture would never receive the connection. +// A random suffix keeps concurrently running tests on distinct pipes. +func newLoopbackIpcListener(t *testing.T) net.Listener { + t.Helper() + + suffix := make([]byte, 8) + if _, err := rand.Read(suffix); err != nil { + t.Fatalf("failed to generate pipe name suffix: %v", err) + } + pipeName := `\\.\pipe\uloop-test-` + hex.EncodeToString(suffix) + listener, err := winio.ListenPipe(pipeName, nil) + if err != nil { + t.Fatalf("failed to listen on named pipe %s: %v", pipeName, err) + } + t.Cleanup(func() { + _ = listener.Close() + }) + return listener +} diff --git a/cli/release-automation/internal/automation/contract_file_at_ref_test.go b/cli/release-automation/internal/automation/contract_file_at_ref_test.go index b41f1d3d0..0835b76dd 100644 --- a/cli/release-automation/internal/automation/contract_file_at_ref_test.go +++ b/cli/release-automation/internal/automation/contract_file_at_ref_test.go @@ -5,6 +5,7 @@ import ( "errors" "os" "path/filepath" + "runtime" "strings" "testing" "time" @@ -406,6 +407,10 @@ func writeSleepingMockGit(t *testing.T, binDir string) { t.Helper() path := filepath.Join(binDir, "git") + if runtime.GOOS == "windows" { + installMockCliExecutable(t, path, mockCliExecutableConfig{Mode: "sleeping"}) + return + } writeFile(t, path, "#!/bin/sh\nsleep 10\n") if err := os.Chmod(path, 0o755); err != nil { t.Fatalf("failed to chmod mock git: %v", err) diff --git a/cli/release-automation/internal/automation/dispatcher_minimum_version_guard_test.go b/cli/release-automation/internal/automation/dispatcher_minimum_version_guard_test.go index e6f18597d..837a203ea 100644 --- a/cli/release-automation/internal/automation/dispatcher_minimum_version_guard_test.go +++ b/cli/release-automation/internal/automation/dispatcher_minimum_version_guard_test.go @@ -5,6 +5,7 @@ import ( "context" "os" "path/filepath" + "runtime" "strings" "testing" ) @@ -217,6 +218,11 @@ func buildDispatcherMinimumVersionPin(projectRunnerVersion string, minimumDispat func writeDispatcherMinimumVersionMockGit(t *testing.T, path string) { t.Helper() + if runtime.GOOS == "windows" { + installMockCliExecutable(t, path, mockCliExecutableConfig{Mode: "dispatcherMinimumVersionGit"}) + return + } + content := `#!/bin/sh set -eu diff --git a/cli/release-automation/internal/automation/mock_cli_executable_for_tests_test.go b/cli/release-automation/internal/automation/mock_cli_executable_for_tests_test.go new file mode 100644 index 000000000..4eb0a8282 --- /dev/null +++ b/cli/release-automation/internal/automation/mock_cli_executable_for_tests_test.go @@ -0,0 +1,147 @@ +package automation + +import ( + "encoding/json" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "runtime" + "sort" + "testing" +) + +// This file provides the Windows counterpart of the POSIX shell mock scripts +// installed as fake `git`/`gh` executables. Why: Windows LookPath only +// resolves files with executable extensions, so an extensionless shell script +// on PATH is silently skipped and the real git/gh would run instead; cmd batch +// shims are no alternative because cmd mangles arguments such as +// `tag^{commit}` and embedded quotes. TestMain therefore builds +// testdata/mockcli once per test run, and installMockCliExecutable copies it +// next to a JSON config that selects which shell mock to emulate. + +// mockCliExecutablePath is the shared mockcli build TestMain produces on +// Windows; empty on other platforms. +var mockCliExecutablePath string + +// mockCliExecutableConfig mirrors mockCliConfig in testdata/mockcli/main.go. +type mockCliExecutableConfig struct { + Mode string `json:"mode"` + RefResolves bool `json:"refResolves,omitempty"` + Paths map[string]mockCliPathConfig `json:"paths,omitempty"` + PathOrder []string `json:"pathOrder,omitempty"` +} + +// mockCliPathConfig mirrors mockCliPathConfig in testdata/mockcli/main.go. +type mockCliPathConfig struct { + Exists bool `json:"exists"` + ShowOK bool `json:"showOK"` + ShowContent string `json:"showContent,omitempty"` + ShowContentPath string `json:"showContentPath,omitempty"` + ShowStderr string `json:"showStderr,omitempty"` + ProbeSleeps bool `json:"probeSleeps,omitempty"` +} + +func TestMain(m *testing.M) { + os.Exit(runTestMain(m)) +} + +func runTestMain(m *testing.M) int { + if runtime.GOOS == "windows" { + buildDir, err := os.MkdirTemp("", "uloop-mockcli-") + if err != nil { + fmt.Fprintf(os.Stderr, "failed to create mockcli build directory: %v\n", err) + return 1 + } + defer func() { + _ = os.RemoveAll(buildDir) + }() + + executablePath := filepath.Join(buildDir, "mockcli.exe") + command := exec.Command("go", "build", "-o", executablePath, "./testdata/mockcli") + if output, err := command.CombinedOutput(); err != nil { + fmt.Fprintf(os.Stderr, "failed to build mockcli: %v\n%s", err, output) + return 1 + } + mockCliExecutablePath = executablePath + } + return m.Run() +} + +// installMockCliExecutable installs the prebuilt mockcli binary as +// .exe with its config at .mockconfig.json. Only used +// on Windows; scriptPath is the extensionless path the POSIX shell mock would +// occupy (e.g. .../bin/git), so PATH lookups resolve the same command name. +func installMockCliExecutable(t *testing.T, scriptPath string, config mockCliExecutableConfig) { + t.Helper() + + if mockCliExecutablePath == "" { + t.Fatal("mockcli executable was not built; TestMain only builds it on Windows") + } + + copyMockCliExecutable(t, scriptPath+".exe") + + configContent, err := json.Marshal(config) + if err != nil { + t.Fatalf("failed to encode mockcli config: %v", err) + } + writeFile(t, scriptPath+".mockconfig.json", string(configContent)) +} + +func copyMockCliExecutable(t *testing.T, destinationPath string) { + t.Helper() + + // A hard link avoids copying the binary for every test; both paths live in + // the same temp volume. Fall back to a byte copy when linking fails. + if err := os.Link(mockCliExecutablePath, destinationPath); err == nil { + return + } + source, err := os.Open(mockCliExecutablePath) + if err != nil { + t.Fatalf("failed to open mockcli executable: %v", err) + } + defer func() { + _ = source.Close() + }() + destination, err := os.OpenFile(destinationPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o755) + if err != nil { + t.Fatalf("failed to create mock executable: %v", err) + } + defer func() { + _ = destination.Close() + }() + if _, err := io.Copy(destination, source); err != nil { + t.Fatalf("failed to copy mock executable: %v", err) + } +} + +// existenceMockCliConfig converts the shared existence fixture into the +// mockcli config, preserving the sorted case-label order the generated shell +// script uses. +func existenceMockCliConfig(fixture mockGitExistenceFixture) mockCliExecutableConfig { + keys := make([]string, 0, len(fixture.paths)) + for key := range fixture.paths { + keys = append(keys, key) + } + sort.Strings(keys) + + paths := make(map[string]mockCliPathConfig, len(fixture.paths)) + for key, behavior := range fixture.paths { + paths[key] = mockCliPathConfig{ + Exists: behavior.exists, + ShowOK: behavior.showOK, + ShowContent: behavior.showContent, + ShowContentPath: behavior.showContentPath, + ShowStderr: behavior.showStderr, + ProbeSleeps: behavior.probeSleeps, + } + } + + return mockCliExecutableConfig{ + Mode: "existence", + RefResolves: fixture.refResolves, + Paths: paths, + PathOrder: keys, + } +} diff --git a/cli/release-automation/internal/automation/mock_git_for_tests_test.go b/cli/release-automation/internal/automation/mock_git_for_tests_test.go index e50ebbc03..0e6e44c0a 100644 --- a/cli/release-automation/internal/automation/mock_git_for_tests_test.go +++ b/cli/release-automation/internal/automation/mock_git_for_tests_test.go @@ -3,6 +3,7 @@ package automation import ( "os" "path/filepath" + "runtime" "sort" "strings" "testing" @@ -68,8 +69,12 @@ func setupMockGitBin(t *testing.T) (string, string) { func writeExistenceMockGit(t *testing.T, binDir string, fixture mockGitExistenceFixture) { t.Helper() - script := buildExistenceMockGitScript(fixture) path := filepath.Join(binDir, "git") + if runtime.GOOS == "windows" { + installMockCliExecutable(t, path, existenceMockCliConfig(fixture)) + return + } + script := buildExistenceMockGitScript(fixture) writeFile(t, path, script) if err := os.Chmod(path, 0o755); err != nil { t.Fatalf("failed to chmod mock git: %v", err) diff --git a/cli/release-automation/internal/automation/protocol_minimum_version_guard_test.go b/cli/release-automation/internal/automation/protocol_minimum_version_guard_test.go index e8699517c..e7b72b350 100644 --- a/cli/release-automation/internal/automation/protocol_minimum_version_guard_test.go +++ b/cli/release-automation/internal/automation/protocol_minimum_version_guard_test.go @@ -5,6 +5,7 @@ import ( "context" "os" "path/filepath" + "runtime" "strconv" "strings" "testing" @@ -600,6 +601,11 @@ func buildProtocolMinimumVersionPin(projectRunnerVersion string) string { func writeProtocolMinimumVersionMockGit(t *testing.T, path string) { t.Helper() + if runtime.GOOS == "windows" { + installMockCliExecutable(t, path, mockCliExecutableConfig{Mode: "protocolMinimumVersionGit"}) + return + } + content := `#!/bin/sh set -eu @@ -695,6 +701,11 @@ exit 1 func writeProtocolMinimumVersionMockGH(t *testing.T, path string) { t.Helper() + if runtime.GOOS == "windows" { + installMockCliExecutable(t, path, mockCliExecutableConfig{Mode: "protocolMinimumVersionGh"}) + return + } + content := `#!/bin/sh set -eu diff --git a/cli/release-automation/internal/automation/testdata/mockcli/main.go b/cli/release-automation/internal/automation/testdata/mockcli/main.go new file mode 100644 index 000000000..b80aff72f --- /dev/null +++ b/cli/release-automation/internal/automation/testdata/mockcli/main.go @@ -0,0 +1,406 @@ +// Command mockcli is the Windows stand-in for the POSIX shell mock scripts the +// automation tests install as fake `git`/`gh` executables on PATH. Why: Windows +// LookPath only resolves files with executable extensions, so an extensionless +// shell script is silently skipped and the real git/gh would run instead, and +// cmd batch files mangle arguments such as `tag^{commit}` or embedded quotes. +// The binary is built once by TestMain and copied next to a JSON config file +// (.mockconfig.json) that selects which shell mock to emulate. Each mode +// below is a line-by-line translation of the corresponding script generator in +// the automation test files. +package main + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + "time" +) + +// mockCliConfig mirrors mockCliExecutableConfig in +// internal/automation/mock_cli_executable_for_tests_test.go. +type mockCliConfig struct { + Mode string `json:"mode"` + RefResolves bool `json:"refResolves,omitempty"` + Paths map[string]mockCliPathConfig `json:"paths,omitempty"` + PathOrder []string `json:"pathOrder,omitempty"` +} + +// mockCliPathConfig mirrors mockGitPathBehavior for the existence mode. +type mockCliPathConfig struct { + Exists bool `json:"exists"` + ShowOK bool `json:"showOK"` + ShowContent string `json:"showContent,omitempty"` + ShowContentPath string `json:"showContentPath,omitempty"` + ShowStderr string `json:"showStderr,omitempty"` + ProbeSleeps bool `json:"probeSleeps,omitempty"` +} + +func main() { + config, err := loadConfig() + if err != nil { + fmt.Fprintf(os.Stderr, "mockcli: %v\n", err) + os.Exit(1) + } + + args := os.Args[1:] + switch config.Mode { + case "existence": + os.Exit(runExistenceGit(config, args)) + case "sleeping": + // Mirrors writeSleepingMockGit: block long enough that a short context + // timeout reliably kills the probe mid-run. + time.Sleep(10 * time.Second) + os.Exit(0) + case "dispatcherMinimumVersionGit": + os.Exit(runDispatcherMinimumVersionGit(args)) + case "protocolMinimumVersionGit": + os.Exit(runProtocolMinimumVersionGit(args)) + case "protocolMinimumVersionGh": + os.Exit(runProtocolMinimumVersionGh(args)) + default: + fmt.Fprintf(os.Stderr, "mockcli: unknown mode %q\n", config.Mode) + os.Exit(1) + } +} + +func loadConfig() (mockCliConfig, error) { + executablePath, err := os.Executable() + if err != nil { + return mockCliConfig{}, fmt.Errorf("failed to locate executable: %w", err) + } + configPath := strings.TrimSuffix(executablePath, filepath.Ext(executablePath)) + ".mockconfig.json" + content, err := os.ReadFile(configPath) + if err != nil { + return mockCliConfig{}, fmt.Errorf("failed to read config: %w", err) + } + config := mockCliConfig{} + if err := json.Unmarshal(content, &config); err != nil { + return mockCliConfig{}, fmt.Errorf("failed to decode config %s: %w", configPath, err) + } + return config, nil +} + +// matchWildcard reports whether value matches a shell case label where `*` +// matches any run of characters (including separators). +func matchWildcard(pattern string, value string) bool { + segments := strings.Split(pattern, "*") + quoted := make([]string, 0, len(segments)) + for _, segment := range segments { + quoted = append(quoted, regexp.QuoteMeta(segment)) + } + matched, err := regexp.MatchString("^"+strings.Join(quoted, ".*")+"$", value) + if err != nil { + return false + } + return matched +} + +func appendLogLine(logEnvName string, args []string) { + logPath := os.Getenv(logEnvName) + if logPath == "" { + return + } + file, err := os.OpenFile(logPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) + if err != nil { + fmt.Fprintf(os.Stderr, "mockcli: failed to open log: %v\n", err) + os.Exit(1) + } + defer func() { + _ = file.Close() + }() + if _, err := fmt.Fprintf(file, "%s\n", strings.Join(args, " ")); err != nil { + fmt.Fprintf(os.Stderr, "mockcli: failed to append log: %v\n", err) + os.Exit(1) + } +} + +func stripChdirFlag(args []string) []string { + if len(args) >= 2 && args[0] == "-C" { + return args[2:] + } + return args +} + +func emitFileContent(path string) int { + content, err := os.ReadFile(path) + if err != nil { + fmt.Fprintf(os.Stderr, "mockcli: %v\n", err) + return 1 + } + _, _ = os.Stdout.Write(content) + return 0 +} + +func argAt(args []string, index int) string { + if index < len(args) { + return args[index] + } + return "" +} + +// runExistenceGit mirrors buildExistenceMockGitScript in +// mock_git_for_tests_test.go. +func runExistenceGit(config mockCliConfig, args []string) int { + args = stripChdirFlag(args) + + switch argAt(args, 0) { + case "cat-file": + target := argAt(args, 2) + for _, key := range config.PathOrder { + if !matchWildcard("*:"+key, target) { + continue + } + behavior := config.Paths[key] + if behavior.ProbeSleeps { + time.Sleep(10 * time.Second) + return 0 + } + if behavior.Exists { + return 0 + } + return 1 + } + fmt.Fprintf(os.Stderr, "unexpected cat-file target: %s\n", target) + return 1 + case "rev-parse": + if argAt(args, 1) == "--verify" { + if config.RefResolves { + return 0 + } + return 1 + } + fmt.Fprintf(os.Stderr, "unexpected rev-parse: %s\n", strings.Join(args, " ")) + return 1 + case "show": + target := argAt(args, 1) + for _, key := range config.PathOrder { + if !matchWildcard("*:"+key, target) { + continue + } + behavior := config.Paths[key] + if behavior.ShowOK { + if behavior.ShowContentPath != "" { + return emitFileContent(behavior.ShowContentPath) + } + if behavior.ShowContent != "" { + fmt.Fprintf(os.Stdout, "%s", behavior.ShowContent) + } + return 0 + } + if behavior.ShowStderr != "" { + fmt.Fprintf(os.Stderr, "%s\n", behavior.ShowStderr) + } + return 1 + } + fmt.Fprintf(os.Stderr, "unexpected git show ref: %s\n", target) + return 1 + default: + fmt.Fprintf(os.Stderr, "unexpected git command: %s\n", strings.Join(args, " ")) + return 1 + } +} + +// envConditionalShowRule emits the file named by ContentEnv when that variable +// is set, falls back to the file named by FallbackEnv when provided, and +// otherwise reports MissingStderr (with {target} replaced) and exits 1. +type envConditionalShowRule struct { + pattern string + contentEnv string + fallbackEnv string + missingStderr string +} + +type envConditionalCatFileRule struct { + pattern string + envName string +} + +func runShowRules(rules []envConditionalShowRule, target string) int { + for _, rule := range rules { + if !matchWildcard(rule.pattern, target) { + continue + } + contentPath := os.Getenv(rule.contentEnv) + if contentPath == "" && rule.fallbackEnv != "" { + contentPath = os.Getenv(rule.fallbackEnv) + } + if contentPath == "" { + fmt.Fprintf(os.Stderr, "%s\n", strings.ReplaceAll(rule.missingStderr, "{target}", target)) + return 1 + } + return emitFileContent(contentPath) + } + fmt.Fprintf(os.Stderr, "unexpected git show ref: %s\n", target) + return 1 +} + +func runCatFileRules(rules []envConditionalCatFileRule, target string) int { + for _, rule := range rules { + if !matchWildcard(rule.pattern, target) { + continue + } + if os.Getenv(rule.envName) != "" { + return 0 + } + return 1 + } + fmt.Fprintf(os.Stderr, "unexpected cat-file target: %s\n", target) + return 1 +} + +// runDispatcherMinimumVersionGit mirrors writeDispatcherMinimumVersionMockGit +// in dispatcher_minimum_version_guard_test.go. +func runDispatcherMinimumVersionGit(args []string) int { + appendLogLine("GIT_LOG", args) + + if argAt(args, 0) == "rev-parse" && argAt(args, 1) == "--show-toplevel" { + fmt.Fprintf(os.Stdout, "%s\n", os.Getenv("ULOOP_REPOSITORY_ROOT")) + return 0 + } + + args = stripChdirFlag(args) + + showRules := []envConditionalShowRule{ + { + pattern: "dispatcher-v*:cli/dispatcher/dispatchercontract/dispatcher-contract.json", + contentEnv: "GIT_RELEASE_CONTRACT", + missingStderr: "fatal: path 'cli/dispatcher/dispatchercontract/dispatcher-contract.json' exists on disk, but not in '{target}'", + }, + { + pattern: "dispatcher-v*:cli/dispatcher/dispatcher-contract.json", + contentEnv: "GIT_PREVIOUS_RELEASE_CONTRACT", + missingStderr: "previous release not found", + }, + { + pattern: "dispatcher-v*:dispatcher/dispatcher-contract.json", + contentEnv: "GIT_MIDDLE_RELEASE_CONTRACT", + missingStderr: "middle release not found", + }, + { + pattern: "dispatcher-v*:cli/dispatcher-contract.json", + contentEnv: "GIT_LEGACY_RELEASE_CONTRACT", + missingStderr: "release not found", + }, + } + catFileRules := []envConditionalCatFileRule{ + {pattern: "dispatcher-v*:cli/dispatcher/dispatchercontract/dispatcher-contract.json", envName: "GIT_RELEASE_CONTRACT"}, + {pattern: "dispatcher-v*:cli/dispatcher/dispatcher-contract.json", envName: "GIT_PREVIOUS_RELEASE_CONTRACT"}, + {pattern: "dispatcher-v*:dispatcher/dispatcher-contract.json", envName: "GIT_MIDDLE_RELEASE_CONTRACT"}, + {pattern: "dispatcher-v*:cli/dispatcher-contract.json", envName: "GIT_LEGACY_RELEASE_CONTRACT"}, + } + + switch argAt(args, 0) { + case "show": + return runShowRules(showRules, argAt(args, 1)) + case "cat-file": + if argAt(args, 1) == "-e" { + return runCatFileRules(catFileRules, argAt(args, 2)) + } + case "rev-parse": + if argAt(args, 1) == "--verify" { + // Dispatcher release refs used in these tests are always considered + // resolvable; the release publishing tests set up the associated + // fixtures. + return 0 + } + } + fmt.Fprintf(os.Stderr, "unexpected git command: %s\n", strings.Join(args, " ")) + return 1 +} + +// runProtocolMinimumVersionGit mirrors writeProtocolMinimumVersionMockGit in +// protocol_minimum_version_guard_test.go. +func runProtocolMinimumVersionGit(args []string) int { + appendLogLine("GIT_LOG", args) + + if argAt(args, 0) == "rev-parse" && argAt(args, 1) == "--show-toplevel" { + fmt.Fprintf(os.Stdout, "%s\n", os.Getenv("ULOOP_REPOSITORY_ROOT")) + return 0 + } + + args = stripChdirFlag(args) + + showRules := []envConditionalShowRule{ + {pattern: "origin/v3-beta:Packages/src/project-runner-pin.json", contentEnv: "GIT_BASE_PIN"}, + {pattern: "origin/v3-beta:*", contentEnv: "GIT_BASE_CONSTANTS"}, + { + pattern: "protocol-pr-head:cli/common/clicontract/contract.json", + contentEnv: "GIT_HEAD_CONTRACT_CONTENT", + fallbackEnv: "GIT_HEAD_CONSTANTS", + }, + {pattern: "protocol-pr-head:Packages/src/project-runner-pin.json", contentEnv: "GIT_HEAD_PIN"}, + {pattern: "protocol-pr-head:*", contentEnv: "GIT_HEAD_CONSTANTS"}, + {pattern: "protocol-release:Packages/src/project-runner-pin.json", contentEnv: "GIT_HEAD_PIN"}, + {pattern: "protocol-release:*", contentEnv: "GIT_HEAD_CONSTANTS"}, + { + pattern: "uloop-project-runner-v*:cli/common/clicontract/contract.json", + contentEnv: "GIT_RELEASE_CONTENT", + missingStderr: "fatal: path 'cli/common/clicontract/contract.json' exists on disk, but not in '{target}'", + }, + { + pattern: "uloop-project-runner-v*:common/clicontract/contract.json", + contentEnv: "GIT_MIDDLE_RELEASE_CONTENT", + missingStderr: "middle release not found", + }, + { + pattern: "uloop-project-runner-v*:cli/contract.json", + contentEnv: "GIT_LEGACY_RELEASE_CONTENT", + missingStderr: "release not found", + }, + } + catFileRules := []envConditionalCatFileRule{ + {pattern: "uloop-project-runner-v*:cli/common/clicontract/contract.json", envName: "GIT_RELEASE_CONTENT"}, + {pattern: "uloop-project-runner-v*:common/clicontract/contract.json", envName: "GIT_MIDDLE_RELEASE_CONTENT"}, + {pattern: "uloop-project-runner-v*:cli/contract.json", envName: "GIT_LEGACY_RELEASE_CONTENT"}, + } + + switch argAt(args, 0) { + case "show": + return runShowRules(showRules, argAt(args, 1)) + case "cat-file": + if argAt(args, 1) == "-e" { + return runCatFileRules(catFileRules, argAt(args, 2)) + } + case "rev-parse": + if argAt(args, 1) == "--verify" { + // Release/base/head refs used in these tests are always resolvable; + // the fallback flow only reaches rev-parse when a show has already + // failed. + return 0 + } + } + fmt.Fprintf(os.Stderr, "unexpected git command: %s\n", strings.Join(args, " ")) + return 1 +} + +// runProtocolMinimumVersionGh mirrors writeProtocolMinimumVersionMockGH in +// protocol_minimum_version_guard_test.go. +func runProtocolMinimumVersionGh(args []string) int { + appendLogLine("GH_LOG", args) + + if argAt(args, 0) == "release" && argAt(args, 1) == "view" { + releaseView := os.Getenv("GH_RELEASE_VIEW") + if releaseView == "" { + releaseView = `{"isDraft":false,"assets":[{"name":"uloop-project-runner-darwin-amd64.tar.gz","size":1},{"name":"uloop-project-runner-darwin-amd64.tar.gz.sha256","size":1},{"name":"uloop-project-runner-darwin-arm64.tar.gz","size":1},{"name":"uloop-project-runner-darwin-arm64.tar.gz.sha256","size":1},{"name":"uloop-project-runner-windows-amd64.zip","size":1},{"name":"uloop-project-runner-windows-amd64.zip.sha256","size":1}]}` + } + fmt.Fprintf(os.Stdout, "%s\n", releaseView) + return 0 + } + + if argAt(args, 0) == "api" && argAt(args, 1) == "--paginate" { + if commentIDs := os.Getenv("GH_COMMENT_IDS"); commentIDs != "" { + fmt.Fprintf(os.Stdout, "%s\n", commentIDs) + } + return 0 + } + + if argAt(args, 0) == "api" && argAt(args, 1) == "--method" { + return 0 + } + + fmt.Fprintf(os.Stderr, "unexpected gh command: %s\n", strings.Join(args, " ")) + return 1 +} From 200677cbd58e5f3e0f4d746b947bd0d7bf415154 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Fri, 17 Jul 2026 20:59:30 +0900 Subject: [PATCH 2/6] fix: restore runInBackground on the main thread after input simulation (#1812) --- .../InputSimulationRunInBackgroundScope.cs | 8 ++ ...Tools.Common.InputSimulation.Editor.asmdef | 4 +- .../SimulateKeyboardUseCase.cs | 88 +++++++++++-------- .../SimulateMouseInputUseCase.cs | 82 +++++++++-------- 4 files changed, 106 insertions(+), 76 deletions(-) diff --git a/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/InputSimulationRunInBackgroundScope.cs b/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/InputSimulationRunInBackgroundScope.cs index c3a0380bc..340e6f778 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/InputSimulationRunInBackgroundScope.cs +++ b/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/InputSimulationRunInBackgroundScope.cs @@ -1,6 +1,8 @@ #nullable enable using System; +using io.github.hatayama.UnityCliLoop.ToolContracts; + namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { /// @@ -29,6 +31,12 @@ public static InputSimulationRunInBackgroundScope Enable() public void Dispose() { + // Precondition: Application.runInBackground is main-thread-only, so callers resuming + // from ConfigureAwait(false) continuations must switch back to the main thread first. + UnityEngine.Debug.Assert( + MainThreadSwitcher.IsMainThread, + "InputSimulationRunInBackgroundScope.Dispose must be called on the main thread."); + if (_isDisposed) { return; diff --git a/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/UnityCLILoop.FirstPartyTools.Common.InputSimulation.Editor.asmdef b/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/UnityCLILoop.FirstPartyTools.Common.InputSimulation.Editor.asmdef index 830a323f3..763c0aa03 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/UnityCLILoop.FirstPartyTools.Common.InputSimulation.Editor.asmdef +++ b/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/UnityCLILoop.FirstPartyTools.Common.InputSimulation.Editor.asmdef @@ -1,7 +1,9 @@ { "name": "UnityCLILoop.FirstPartyTools.Common.InputSimulation.Editor", "rootNamespace": "io.github.hatayama.UnityCliLoop.FirstPartyTools", - "references": [], + "references": [ + "GUID:fc3fd32eddbee40e39c2d76dc184957b" + ], "includePlatforms": [ "Editor" ], diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs index 3c539fe5a..f467917a1 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs @@ -104,49 +104,59 @@ public async Task ExecuteAsync( correlationId: correlationId ); - using InputSimulationRunInBackgroundScope runInBackgroundScope = InputSimulationRunInBackgroundScope.Enable(); - - EnsureOverlayExists(); + // Why not `using`: the executor awaits below use ConfigureAwait(false), so this method + // can resume on a thread-pool thread. Application.runInBackground is main-thread-only, + // so the scope must be disposed after switching back to the main thread. + InputSimulationRunInBackgroundScope runInBackgroundScope = InputSimulationRunInBackgroundScope.Enable(); + try + { + EnsureOverlayExists(); - SimulateKeyboardResponse response; + SimulateKeyboardResponse response; - switch (parameters.Action) + switch (parameters.Action) + { + case UnityCliLoopKeyboardAction.Press: + response = await KeyboardInputActionExecutor.ExecutePress( + keyboard, + key, + parameters.Duration, + ct).ConfigureAwait(false); + break; + + case UnityCliLoopKeyboardAction.KeyDown: + response = await KeyboardInputActionExecutor.ExecuteKeyDown(keyboard, key, ct).ConfigureAwait(false); + break; + + case UnityCliLoopKeyboardAction.KeyUp: + response = await KeyboardInputActionExecutor.ExecuteKeyUp(keyboard, key, ct).ConfigureAwait(false); + break; + + default: + // Only reachable when an out-of-range enum value is cast from an integer; + // surface as a Success=false response so the CLI treats it as a normal validation failure. + return new SimulateKeyboardResponse + { + Success = false, + Message = $"Unknown keyboard action: {parameters.Action}", + Action = parameters.Action.ToString() + }; + } + + VibeLogger.LogInfo( + "simulate_keyboard_complete", + $"Keyboard simulation completed: {response.Message}", + new { Action = parameters.Action.ToString(), Success = response.Success }, + correlationId: correlationId + ); + + return response; + } + finally { - case UnityCliLoopKeyboardAction.Press: - response = await KeyboardInputActionExecutor.ExecutePress( - keyboard, - key, - parameters.Duration, - ct).ConfigureAwait(false); - break; - - case UnityCliLoopKeyboardAction.KeyDown: - response = await KeyboardInputActionExecutor.ExecuteKeyDown(keyboard, key, ct).ConfigureAwait(false); - break; - - case UnityCliLoopKeyboardAction.KeyUp: - response = await KeyboardInputActionExecutor.ExecuteKeyUp(keyboard, key, ct).ConfigureAwait(false); - break; - - default: - // Only reachable when an out-of-range enum value is cast from an integer; - // surface as a Success=false response so the CLI treats it as a normal validation failure. - return new SimulateKeyboardResponse - { - Success = false, - Message = $"Unknown keyboard action: {parameters.Action}", - Action = parameters.Action.ToString() - }; + await InputSystemUpdateHelper.SwitchToMainThreadIfNeeded(CancellationToken.None); + runInBackgroundScope.Dispose(); } - - VibeLogger.LogInfo( - "simulate_keyboard_complete", - $"Keyboard simulation completed: {response.Message}", - new { Action = parameters.Action.ToString(), Success = response.Success }, - correlationId: correlationId - ); - - return response; #endif } diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs index ef3de4fe4..9bc0fa565 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs @@ -97,46 +97,56 @@ public async Task ExecuteAsync( correlationId: correlationId ); - using InputSimulationRunInBackgroundScope runInBackgroundScope = InputSimulationRunInBackgroundScope.Enable(); - - EnsureOverlayExists(); - - SimulateMouseInputResponse response; - - switch (parameters.Action) + // Why not `using`: the executor awaits below use ConfigureAwait(false), so this method + // can resume on a thread-pool thread. Application.runInBackground is main-thread-only, + // so the scope must be disposed after switching back to the main thread. + InputSimulationRunInBackgroundScope runInBackgroundScope = InputSimulationRunInBackgroundScope.Enable(); + try { - case UnityCliLoopMouseInputAction.Click: - response = await MouseInputPressActionExecutor.ExecuteClick(mouse, parameters, ct).ConfigureAwait(false); - break; - - case UnityCliLoopMouseInputAction.LongPress: - response = await MouseInputPressActionExecutor.ExecuteLongPress(mouse, parameters, ct).ConfigureAwait(false); - break; + EnsureOverlayExists(); - case UnityCliLoopMouseInputAction.MoveDelta: - response = await MouseInputMotionActionExecutor.ExecuteMoveDelta(mouse, parameters, ct).ConfigureAwait(false); - break; + SimulateMouseInputResponse response; - case UnityCliLoopMouseInputAction.Scroll: - response = await MouseInputMotionActionExecutor.ExecuteScroll(mouse, parameters, ct).ConfigureAwait(false); - break; - - case UnityCliLoopMouseInputAction.SmoothDelta: - response = await MouseInputMotionActionExecutor.ExecuteSmoothDelta(mouse, parameters, ct).ConfigureAwait(false); - break; - - default: - throw new ArgumentException($"Unknown mouse input action: {parameters.Action}"); + switch (parameters.Action) + { + case UnityCliLoopMouseInputAction.Click: + response = await MouseInputPressActionExecutor.ExecuteClick(mouse, parameters, ct).ConfigureAwait(false); + break; + + case UnityCliLoopMouseInputAction.LongPress: + response = await MouseInputPressActionExecutor.ExecuteLongPress(mouse, parameters, ct).ConfigureAwait(false); + break; + + case UnityCliLoopMouseInputAction.MoveDelta: + response = await MouseInputMotionActionExecutor.ExecuteMoveDelta(mouse, parameters, ct).ConfigureAwait(false); + break; + + case UnityCliLoopMouseInputAction.Scroll: + response = await MouseInputMotionActionExecutor.ExecuteScroll(mouse, parameters, ct).ConfigureAwait(false); + break; + + case UnityCliLoopMouseInputAction.SmoothDelta: + response = await MouseInputMotionActionExecutor.ExecuteSmoothDelta(mouse, parameters, ct).ConfigureAwait(false); + break; + + default: + throw new ArgumentException($"Unknown mouse input action: {parameters.Action}"); + } + + VibeLogger.LogInfo( + "simulate_mouse_input_complete", + $"Mouse input simulation completed: {response.Message}", + new { Action = parameters.Action.ToString(), Success = response.Success }, + correlationId: correlationId + ); + + return response; + } + finally + { + await InputSystemUpdateHelper.SwitchToMainThreadIfNeeded(CancellationToken.None); + runInBackgroundScope.Dispose(); } - - VibeLogger.LogInfo( - "simulate_mouse_input_complete", - $"Mouse input simulation completed: {response.Message}", - new { Action = parameters.Action.ToString(), Success = response.Success }, - correlationId: correlationId - ); - - return response; #endif } From 078433e7fff101c09cdd9bc5b91888460c0d2437 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Fri, 17 Jul 2026 21:05:34 +0900 Subject: [PATCH 3/6] fix: Unity process matching now survives non-ASCII project paths on Windows (#1815) --- cli/common/unityprocess/process.go | 35 ++++++++++++---- cli/common/unityprocess/process_test.go | 55 +++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 13 deletions(-) diff --git a/cli/common/unityprocess/process.go b/cli/common/unityprocess/process.go index dc023eacd..6a1d83c4d 100644 --- a/cli/common/unityprocess/process.go +++ b/cli/common/unityprocess/process.go @@ -2,6 +2,7 @@ package unityprocess import ( "context" + "encoding/base64" "fmt" "os/exec" "path/filepath" @@ -74,21 +75,32 @@ func listUnityProcessesMac(ctx context.Context) ([]UnityProcess, error) { } func listUnityProcessesWindows(ctx context.Context) ([]UnityProcess, error) { + commandContext, cancel := withCommandTimeout(ctx, ProcessListCommandTimeout) + defer cancel() + output, err := exec.CommandContext(commandContext, windowsPowerShellCommand, "-NoProfile", "-Command", windowsUnityProcessListScript()).Output() + if err != nil { + return nil, fmt.Errorf("failed to retrieve Unity process list on Windows: %w", err) + } + return parseWindowsUnityProcesses(string(output)), nil +} + +// windowsUnityProcessListScript builds the PowerShell script that lists Unity +// processes as "pid|base64(UTF-8 command line)" lines. +// why: Windows PowerShell 5.1 encodes redirected stdout with the OEM code page +// (e.g. CP932 on Japanese Windows), so non-ASCII project paths in the command +// line get corrupted when Go reads the output as UTF-8. Base64 over UTF-8 +// bytes keeps the stream ASCII-only regardless of the console code page. +func windowsUnityProcessListScript() string { scriptLines := []string{ "$ErrorActionPreference = 'Stop'", "$processes = Get-CimInstance Win32_Process -Filter \"Name = 'Unity.exe'\" | Where-Object { $_.CommandLine }", "foreach ($process in $processes) {", " $commandLine = $process.CommandLine -replace \"`r\", ' ' -replace \"`n\", ' '", - " Write-Output (\"{0}|{1}\" -f $process.ProcessId, $commandLine)", + " $encodedCommandLine = [Convert]::ToBase64String([System.Text.Encoding]::UTF8.GetBytes($commandLine))", + " Write-Output (\"{0}|{1}\" -f $process.ProcessId, $encodedCommandLine)", "}", } - commandContext, cancel := withCommandTimeout(ctx, ProcessListCommandTimeout) - defer cancel() - output, err := exec.CommandContext(commandContext, windowsPowerShellCommand, "-NoProfile", "-Command", strings.Join(scriptLines, "\n")).Output() - if err != nil { - return nil, fmt.Errorf("failed to retrieve Unity process list on Windows: %w", err) - } - return parseWindowsUnityProcesses(string(output)), nil + return strings.Join(scriptLines, "\n") } func parseMacUnityProcesses(output string) []UnityProcess { @@ -136,7 +148,12 @@ func parseWindowsUnityProcesses(output string) []UnityProcess { continue } - command := strings.TrimSpace(trimmed[delimiterIndex+1:]) + decodedCommand, err := base64.StdEncoding.DecodeString(strings.TrimSpace(trimmed[delimiterIndex+1:])) + if err != nil { + continue + } + + command := strings.TrimSpace(string(decodedCommand)) if !isUnityEditorCommand(command, windowsUnityExecutablePattern) { continue } diff --git a/cli/common/unityprocess/process_test.go b/cli/common/unityprocess/process_test.go index f29976f52..c0fb37b27 100644 --- a/cli/common/unityprocess/process_test.go +++ b/cli/common/unityprocess/process_test.go @@ -1,6 +1,7 @@ package unityprocess import ( + "encoding/base64" "path/filepath" "runtime" "strings" @@ -27,11 +28,13 @@ func TestParseMacUnityProcessesExtractsProjectPath(t *testing.T) { } } -// Verifies Windows Unity process parsing extracts project paths and skips batchmode workers. +// Verifies Windows Unity process parsing decodes Base64 command lines, extracts project paths, and skips batchmode workers. func TestParseWindowsUnityProcessesExtractsProjectPath(t *testing.T) { - output := `123|C:\Program Files\Unity\Hub\Editor\6000.0.0f1\Editor\Unity.exe -projectPath "C:\Users\\My Project" -useHub -456|C:\Program Files\Unity\Hub\Editor\6000.0.0f1\Editor\Unity.exe -batchmode -projectPath "C:\Users\\Batch" -` + encode := func(commandLine string) string { + return base64.StdEncoding.EncodeToString([]byte(commandLine)) + } + output := "123|" + encode(`C:\Program Files\Unity\Hub\Editor\6000.0.0f1\Editor\Unity.exe -projectPath "C:\Users\\My Project" -useHub`) + "\r\n" + + "456|" + encode(`C:\Program Files\Unity\Hub\Editor\6000.0.0f1\Editor\Unity.exe -batchmode -projectPath "C:\Users\\Batch"`) + "\r\n" processes := parseWindowsUnityProcesses(output) @@ -43,6 +46,50 @@ func TestParseWindowsUnityProcessesExtractsProjectPath(t *testing.T) { } } +// Verifies non-ASCII project paths survive the PowerShell boundary because command lines travel as UTF-8 Base64. +func TestParseWindowsUnityProcessesPreservesNonASCIIProjectPath(t *testing.T) { + projectPath := `C:\Users\\test[1] 検証用\proj` + commandLine := `C:\Program Files\Unity\Hub\Editor\2022.3.62f3\Editor\Unity.exe -projectPath "` + projectPath + `" -useHub` + output := "123|" + base64.StdEncoding.EncodeToString([]byte(commandLine)) + "\r\n" + + processes := parseWindowsUnityProcesses(output) + + if len(processes) != 1 { + t.Fatalf("process count mismatch: %#v", processes) + } + if processes[0].projectPath != projectPath { + t.Fatalf("project path mismatch: %q", processes[0].projectPath) + } +} + +// Verifies command fields that are not valid Base64 (e.g. legacy plain-text or OEM code page bytes) are skipped instead of mis-parsed. +func TestParseWindowsUnityProcessesSkipsNonBase64CommandLines(t *testing.T) { + // 0x8C9F 0x8FD8 0x9770 is the measured CP932 byte sequence for "検証用" + // that Windows PowerShell 5.1 emitted before the Base64 contract. + cp932KenshouYou := string([]byte{0x8C, 0x9F, 0x8F, 0xD8, 0x97, 0x70}) + output := "123|" + `C:\Editor\Unity.exe -projectPath "C:\Users\\` + cp932KenshouYou + `\proj"` + "\r\n" + + processes := parseWindowsUnityProcesses(output) + + if len(processes) != 0 { + t.Fatalf("non-Base64 command lines should be skipped: %#v", processes) + } +} + +// Verifies the Windows process list script transports command lines as UTF-8 Base64 so the OEM console code page cannot corrupt non-ASCII paths. +func TestWindowsUnityProcessListScriptEncodesCommandLineAsUTF8Base64(t *testing.T) { + script := windowsUnityProcessListScript() + + for _, expected := range []string{ + "[System.Text.Encoding]::UTF8.GetBytes($commandLine)", + "[Convert]::ToBase64String(", + } { + if !strings.Contains(script, expected) { + t.Fatalf("script missing %q: %s", expected, script) + } + } +} + // Verifies Unity -projectPath extraction supports quoted, unquoted, and equals forms. func TestExtractProjectPathSupportsEqualsAndSpaces(t *testing.T) { cases := map[string]string{ From ed38d7ae17bbba0496284b690288649ed1472cb7 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Fri, 17 Jul 2026 21:08:38 +0900 Subject: [PATCH 4/6] fix: Reject asset publishing from recovery-target dispatch runs (#1814) --- .github/workflows/native-cli-publish.yml | 11 +++++++ docs/release-recovery-runbook.md | 35 +++++++++++++++++++-- scripts/test-native-cli-publish-workflow.sh | 12 +++++++ 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/.github/workflows/native-cli-publish.yml b/.github/workflows/native-cli-publish.yml index 6d85c2abd..82197e13e 100644 --- a/.github/workflows/native-cli-publish.yml +++ b/.github/workflows/native-cli-publish.yml @@ -213,6 +213,17 @@ jobs: exit 1 fi release_sha="${TARGET_SHA}" + # Why publishing is blocked here: the attestation certificate binds + # SourceRepositoryDigest to the workflow run head (GITHUB_SHA), never + # to the checked-out build commit, so assets published for an older + # release commit can never pass the dispatcher's tag-to-certificate + # verification. Recovery dispatch may only validate builds. + if [ "${should_publish}" = "true" ] && [ "${release_sha}" != "${GITHUB_SHA}" ]; then + echo "Recovery-target dispatch cannot publish release assets: the release commit ${release_sha} differs from the run head ${GITHUB_SHA}, so the attestation certificate digest would never match the release tag." >&2 + echo "Rerun the original failed workflow run whose head commit is the release commit instead." >&2 + echo "See docs/release-recovery-runbook.md for the recovery procedure." >&2 + exit 1 + fi else if [ "${BUILD_SHA}" != "${GITHUB_SHA}" ] || [ "${TARGET_SHA}" != "${GITHUB_SHA}" ]; then echo "Build SHA and release target must match the approved event commit." >&2 diff --git a/docs/release-recovery-runbook.md b/docs/release-recovery-runbook.md index 5fa0d0191..6076a7990 100644 --- a/docs/release-recovery-runbook.md +++ b/docs/release-recovery-runbook.md @@ -20,6 +20,30 @@ and [29459266044](https://github.com/hatayama/unity-cli-loop/actions/runs/294592 The precise condition is a ref target different from the run's `GITHUB_SHA`, not merely an unreachable commit. Normal head-based releases are unaffected. +## Publishing invariant: assets may only be published by the original run + +Attestation certificates bind `SourceRepositoryDigest` to the workflow run's +head commit (`GITHUB_SHA`), never to the commit the build checked out. The +dispatcher verifies that the release tag's commit equals the certificate +digest, so a release is only downloadable when the publishing run's head IS +the release commit. + +Therefore the only valid way to publish recovery assets is to **rerun the +original failed run whose head is the release commit** (step 5 below). Never +publish through a new `recovery-target` workflow-dispatch run on a later +branch head: its certificates would carry the later head's digest while the +tag points at the historical release commit, producing a release that +permanently fails attestation verification. The workflow now rejects this +combination in the publish job's "Verify release metadata" step; recovery +dispatch runs remain usable for build validation only. + +This is not theoretical: `uloop-project-runner-v3.0.0-beta.48` was published +on 2026-07-15 through a recovery dispatch run after the owner pre-created the +tag and draft release. The tag points at the historical release commit +(`2d8d1b94`) while the attestation certificates carry the dispatch run's head +digest (`2c73c6ac`), so every cold-cache download of beta.48 fails +verification on all platforms. + ## Recovery procedure Run these commands with an owner-authenticated `gh` session. Replace every @@ -67,8 +91,11 @@ Run these commands with an owner-authenticated `gh` session. Replace every Add `--prerelease` when the release tag is a prerelease tag. -5. Rerun the failed publish job and approve the release environment if - prompted. +5. Rerun the failed publish job of the original run whose head is the release + commit, and approve the release environment if prompted. Do not dispatch a + new `recovery-target` run to publish instead: a dispatch run on a later + head cannot produce valid attestations for the release commit, and the + workflow rejects it (see "Publishing invariant" above). ```sh gh run rerun --repo / --failed @@ -118,7 +145,9 @@ Run these commands with an owner-authenticated `gh` session. Replace every `native-cli-publish.yml` is the workflow with `recovery-target` and now emits this runbook path when tag or draft-release creation returns the observed 403. -The creation and reuse logic is unchanged. +The creation and reuse logic is unchanged. Its publish job refuses to publish +assets from a recovery dispatch run whose head differs from the release +commit, because such a run cannot produce matching attestations. `dispatcher-publish.yml` has no recovery-target input and requires its release target to equal `GITHUB_SHA`, so it has no historical-commit creation path. diff --git a/scripts/test-native-cli-publish-workflow.sh b/scripts/test-native-cli-publish-workflow.sh index 0b9356baf..24b8de61d 100755 --- a/scripts/test-native-cli-publish-workflow.sh +++ b/scripts/test-native-cli-publish-workflow.sh @@ -193,6 +193,17 @@ test_recovery_target_403_errors_explain_the_manual_recovery() { assert_contains 'See docs/release-recovery-runbook.md for the recovery procedure.' } +test_recovery_dispatch_refuses_to_publish_from_a_different_head() { + # Verify recovery dispatch fails closed before publishing assets: the + # attestation certificate digest is always the run head, so publishing for an + # older release commit would create a release that never passes verification. + assert_contains 'if [ "${should_publish}" = "true" ] && [ "${release_sha}" != "${GITHUB_SHA}" ]; then' + assert_contains 'Recovery-target dispatch cannot publish release assets' + assert_contains 'Rerun the original failed workflow run whose head commit is the release commit instead.' + assert_before 'release_sha="${TARGET_SHA}"' 'Recovery-target dispatch cannot publish release assets' + assert_before 'Recovery-target dispatch cannot publish release assets' 'printf '\''RELEASE_SHA=%s\n'\'' "${release_sha}" >> "$GITHUB_ENV"' +} + test_draft_creation_accepts_only_the_known_missing_tag_responses() { assert_contains '*"HTTP 404"*) tag_sha="" ;;' assert_contains '*"HTTP 422"*"No commit found for SHA"*|*"No commit found for SHA"*"HTTP 422"*) tag_sha="" ;;' @@ -242,6 +253,7 @@ test_assets_are_attested_after_the_manifest_is_verified test_publish_rejects_manifest_and_existing_tag_mismatches test_release_tag_is_created_before_the_release test_recovery_target_403_errors_explain_the_manual_recovery +test_recovery_dispatch_refuses_to_publish_from_a_different_head test_draft_creation_accepts_only_the_known_missing_tag_responses test_publish_rechecks_the_tag_and_uses_least_privilege_post_publish_permissions test_build_verifies_assets_before_writing_the_publish_input From 26620f389974d631bfacf5f6847fe75ed8554339 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Fri, 17 Jul 2026 21:09:11 +0900 Subject: [PATCH 5/6] fix: uloop update no longer fails on Windows over the locked running exe (#1813) --- scripts/install.ps1 | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 24b4585fd..2c48ddeed 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -494,6 +494,8 @@ function Invoke-CompatibilityWindowsInstall { $TempDir = Join-Path ([System.IO.Path]::GetTempPath()) ("uloop-stage-" + [System.Guid]::NewGuid().ToString("N")) $StagedUloopPath = $null +$FinalUloopPath = Join-Path $InstallDir "uloop.exe" +$ReplacedUloopBackupPath = $null New-Item -ItemType Directory -Path $TempDir | Out-Null try { @@ -535,12 +537,28 @@ try { Expand-UloopArchive -ArchivePath $ArchivePath -DestinationPath $TempDir New-Item -ItemType Directory -Path $InstallDir -Force | Out-Null + + # Why: earlier installs rename the then-running uloop.exe aside (see below); + # those processes have exited by now, so reclaim the leftovers. A backup + # whose process is still running stays locked and is skipped silently until + # a later install can remove it. + Get-ChildItem -LiteralPath $InstallDir -Filter "uloop.exe.old-*" -File -ErrorAction SilentlyContinue | + Remove-Item -Force -ErrorAction SilentlyContinue + $StagedUloopPath = Join-Path $InstallDir ("uloop-staged-" + [System.Guid]::NewGuid().ToString("N") + ".exe") Copy-Item -Path (Join-Path $TempDir "uloop.exe") -Destination $StagedUloopPath -Force Assert-UloopVersionSucceeds -UloopPath $StagedUloopPath -Quiet $NativeInstallSupported = Test-UloopNativeInstallSupported -UloopPath $StagedUloopPath - $FinalUloopPath = Join-Path $InstallDir "uloop.exe" + # Why: Windows locks the image file of a running executable against + # overwrite and delete but still allows rename. `uloop update` runs this + # script as a child of the running uloop.exe, so overwriting the target in + # place can never succeed there. Move the existing binary aside first; the + # finally block restores it if the new binary was not placed. + if (Test-Path -LiteralPath $FinalUloopPath) { + $ReplacedUloopBackupPath = $FinalUloopPath + ".old-" + [System.Guid]::NewGuid().ToString("N") + Move-Item -LiteralPath $FinalUloopPath -Destination $ReplacedUloopBackupPath -Force + } Copy-Item -Path $StagedUloopPath -Destination $FinalUloopPath -Force Remove-Item -Path $StagedUloopPath -Force $StagedUloopPath = $null @@ -555,6 +573,11 @@ try { Assert-UloopVersionSucceeds -UloopPath $FinalUloopPath } finally { + # Why: if the install failed after the old binary was moved aside, put it + # back so a failed update never leaves the user without a working uloop. + if ($ReplacedUloopBackupPath -and (Test-Path -LiteralPath $ReplacedUloopBackupPath) -and -not (Test-Path -LiteralPath $FinalUloopPath)) { + Move-Item -LiteralPath $ReplacedUloopBackupPath -Destination $FinalUloopPath -Force + } if ($StagedUloopPath -and (Test-Path $StagedUloopPath)) { Remove-Item -Path $StagedUloopPath -Force -ErrorAction SilentlyContinue } From 4b18d53187e81d9ab3e6cfdff06afae553e007e9 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Fri, 17 Jul 2026 21:09:49 +0900 Subject: [PATCH 6/6] fix: focus-window verifies foreground state instead of trusting AppActivate on Windows (#1816) --- cli/common/unityprocess/command_error.go | 19 +++++ cli/common/unityprocess/command_error_test.go | 29 ++++++++ .../unityprocess/focus_unity_process.ps1 | 69 +++++++++++++++++-- .../focus_unity_process_with_restore.ps1 | 68 ++++++++++++++++-- cli/common/unityprocess/focus_windows.go | 6 +- cli/common/unityprocess/process_test.go | 53 +++++++++----- 6 files changed, 209 insertions(+), 35 deletions(-) diff --git a/cli/common/unityprocess/command_error.go b/cli/common/unityprocess/command_error.go index c259dfa36..9ef78874c 100644 --- a/cli/common/unityprocess/command_error.go +++ b/cli/common/unityprocess/command_error.go @@ -1,6 +1,8 @@ package unityprocess import ( + "context" + "errors" "fmt" "strings" ) @@ -15,3 +17,20 @@ func commandErrorWithStderr(err error, stderr string) error { } return fmt.Errorf("%w: %s", err, trimmedStderr) } + +// focusCommandError converts a focus script failure into an actionable error. +// Why: a timeout kills the script before it can write anything to stderr, so without this +// mapping the caller only sees a bare "exit status 1" with no hint about the stalled Editor. +func focusCommandError(contextErr error, runErr error, stderr string) error { + if runErr == nil { + return nil + } + if errors.Is(contextErr, context.DeadlineExceeded) { + return fmt.Errorf( + "focusing the Unity window timed out after %s; the Unity Editor may be busy (for example during a domain reload), retry once it is responsive: %w", + FocusCommandTimeout, + runErr, + ) + } + return commandErrorWithStderr(runErr, stderr) +} diff --git a/cli/common/unityprocess/command_error_test.go b/cli/common/unityprocess/command_error_test.go index 3f1c7c3b7..fed786e54 100644 --- a/cli/common/unityprocess/command_error_test.go +++ b/cli/common/unityprocess/command_error_test.go @@ -1,6 +1,7 @@ package unityprocess import ( + "context" "errors" "strings" "testing" @@ -25,3 +26,31 @@ func TestCommandErrorWithStderrKeepsOriginalErrorWithoutStderr(t *testing.T) { t.Fatalf("expected original error, got %v", actual) } } + +// Verifies a timed-out focus script is reported as a busy-Editor timeout instead of a bare exit status. +func TestFocusCommandErrorReportsTimeoutWhenContextDeadlineExceeded(t *testing.T) { + err := focusCommandError(context.DeadlineExceeded, errors.New("exit status 1"), "") + + if err == nil || !strings.Contains(err.Error(), "timed out") || !strings.Contains(err.Error(), "domain reload") { + t.Fatalf("expected timeout explanation, got %v", err) + } +} + +// Verifies a focus script throw keeps the stderr text without the timeout explanation. +func TestFocusCommandErrorKeepsStderrForScriptFailures(t *testing.T) { + err := focusCommandError(nil, errors.New("exit status 1"), "Windows refused to bring the Unity window\r\n") + + if err == nil || !strings.Contains(err.Error(), "Windows refused to bring the Unity window") { + t.Fatalf("expected stderr in error, got %v", err) + } + if strings.Contains(err.Error(), "timed out") { + t.Fatalf("expected no timeout explanation, got %v", err) + } +} + +// Verifies a nil run error yields no focus error. +func TestFocusCommandErrorReturnsNilWithoutRunError(t *testing.T) { + if err := focusCommandError(context.DeadlineExceeded, nil, ""); err != nil { + t.Fatalf("expected nil, got %v", err) + } +} diff --git a/cli/common/unityprocess/focus_unity_process.ps1 b/cli/common/unityprocess/focus_unity_process.ps1 index 64d904e43..89519c314 100644 --- a/cli/common/unityprocess/focus_unity_process.ps1 +++ b/cli/common/unityprocess/focus_unity_process.ps1 @@ -3,18 +3,73 @@ Add-Type -TypeDefinition @" using System; using System.Runtime.InteropServices; public static class Win32Interop { + [DllImport("user32.dll")] public static extern IntPtr GetForegroundWindow(); [DllImport("user32.dll")] public static extern bool SetForegroundWindow(IntPtr hWnd); [DllImport("user32.dll")] public static extern bool ShowWindowAsync(IntPtr hWnd, int nCmdShow); + [DllImport("user32.dll")] public static extern bool IsIconic(IntPtr hWnd); + [DllImport("user32.dll")] public static extern bool BringWindowToTop(IntPtr hWnd); + [DllImport("user32.dll")] public static extern uint GetWindowThreadProcessId(IntPtr hWnd, IntPtr processIdPointer); + [DllImport("kernel32.dll")] public static extern uint GetCurrentThreadId(); + [DllImport("user32.dll")] public static extern bool AttachThreadInput(uint idAttach, uint idAttachTo, bool fAttach); + [DllImport("user32.dll")] public static extern void keybd_event(byte bVk, byte bScan, uint dwFlags, UIntPtr dwExtraInfo); + public static uint GetWindowProcessId(IntPtr hWnd) { + IntPtr buffer = Marshal.AllocHGlobal(4); + try { + GetWindowThreadProcessId(hWnd, buffer); + return (uint)Marshal.ReadInt32(buffer); + } finally { + Marshal.FreeHGlobal(buffer); + } + } } "@ +# Why: SetForegroundWindow can report the switch before the shell finishes it, so poll briefly instead of a single read. +function Test-TargetForeground { + for ($attempt = 0; $attempt -lt 10; $attempt++) { + if ([Win32Interop]::GetWindowProcessId([Win32Interop]::GetForegroundWindow()) -eq {{PID}}) { return $true } + Start-Sleep -Milliseconds 50 + } + return $false +} try { $process = Get-Process -Id {{PID}} -ErrorAction Stop } catch { throw 'Unity process was not found: {{PID}}' } $handle = $process.MainWindowHandle if ($handle -eq 0) { throw 'Unity process has no main window handle: {{PID}}' } -$shown = [Win32Interop]::ShowWindowAsync($handle, 9) -if (-not $shown) { throw 'Failed to show Unity window' } -$focused = [Win32Interop]::SetForegroundWindow($handle) -if (-not $focused) { - $shell = New-Object -ComObject WScript.Shell - $focused = $shell.AppActivate({{PID}}) +# Why: SW_RESTORE on a non-minimized window would shrink a maximized Unity window, so restore only when minimized. +if ([Win32Interop]::IsIconic($handle)) { + $shown = [Win32Interop]::ShowWindowAsync($handle, 9) + if (-not $shown) { throw 'Failed to show Unity window' } +} +[void][Win32Interop]::SetForegroundWindow($handle) +if (-not (Test-TargetForeground)) { + # Why: the Windows foreground lock rejects SetForegroundWindow from background processes; sharing the + # foreground thread's input queue via AttachThreadInput lifts that restriction. + $currentThreadId = [Win32Interop]::GetCurrentThreadId() + $foreground = [Win32Interop]::GetForegroundWindow() + $foregroundThreadId = [Win32Interop]::GetWindowThreadProcessId($foreground, [IntPtr]::Zero) + $targetThreadId = [Win32Interop]::GetWindowThreadProcessId($handle, [IntPtr]::Zero) + $attachedForeground = $false + $attachedTarget = $false + try { + if ($foregroundThreadId -ne 0 -and $foregroundThreadId -ne $currentThreadId) { + $attachedForeground = [Win32Interop]::AttachThreadInput($currentThreadId, $foregroundThreadId, $true) + } + if ($targetThreadId -ne 0 -and $targetThreadId -ne $currentThreadId) { + $attachedTarget = [Win32Interop]::AttachThreadInput($currentThreadId, $targetThreadId, $true) + } + [void][Win32Interop]::BringWindowToTop($handle) + [void][Win32Interop]::SetForegroundWindow($handle) + } finally { + if ($attachedTarget) { [void][Win32Interop]::AttachThreadInput($currentThreadId, $targetThreadId, $false) } + if ($attachedForeground) { [void][Win32Interop]::AttachThreadInput($currentThreadId, $foregroundThreadId, $false) } + } +} +if (-not (Test-TargetForeground)) { + # Why: a transient Alt keypress makes this process the last input source, a documented workaround + # that unlocks SetForegroundWindow when AttachThreadInput alone is not enough. + [Win32Interop]::keybd_event(0x12, 0, 0, [UIntPtr]::Zero) + [void][Win32Interop]::SetForegroundWindow($handle) + [Win32Interop]::keybd_event(0x12, 0, 2, [UIntPtr]::Zero) +} +if (-not (Test-TargetForeground)) { + throw 'Windows refused to bring the Unity window (PID: {{PID}}) to the foreground (foreground lock). Click the Unity window or its taskbar icon to focus it manually.' } -if (-not $focused) { throw 'Failed to focus Unity window' } diff --git a/cli/common/unityprocess/focus_unity_process_with_restore.ps1 b/cli/common/unityprocess/focus_unity_process_with_restore.ps1 index 8c1c396e6..69175be53 100644 --- a/cli/common/unityprocess/focus_unity_process_with_restore.ps1 +++ b/cli/common/unityprocess/focus_unity_process_with_restore.ps1 @@ -6,18 +6,72 @@ public static class Win32Interop { [DllImport("user32.dll")] public static extern IntPtr GetForegroundWindow(); [DllImport("user32.dll")] public static extern bool SetForegroundWindow(IntPtr hWnd); [DllImport("user32.dll")] public static extern bool ShowWindowAsync(IntPtr hWnd, int nCmdShow); + [DllImport("user32.dll")] public static extern bool IsIconic(IntPtr hWnd); + [DllImport("user32.dll")] public static extern bool BringWindowToTop(IntPtr hWnd); + [DllImport("user32.dll")] public static extern uint GetWindowThreadProcessId(IntPtr hWnd, IntPtr processIdPointer); + [DllImport("kernel32.dll")] public static extern uint GetCurrentThreadId(); + [DllImport("user32.dll")] public static extern bool AttachThreadInput(uint idAttach, uint idAttachTo, bool fAttach); + [DllImport("user32.dll")] public static extern void keybd_event(byte bVk, byte bScan, uint dwFlags, UIntPtr dwExtraInfo); + public static uint GetWindowProcessId(IntPtr hWnd) { + IntPtr buffer = Marshal.AllocHGlobal(4); + try { + GetWindowThreadProcessId(hWnd, buffer); + return (uint)Marshal.ReadInt32(buffer); + } finally { + Marshal.FreeHGlobal(buffer); + } + } } "@ +# Why: SetForegroundWindow can report the switch before the shell finishes it, so poll briefly instead of a single read. +function Test-TargetForeground { + for ($attempt = 0; $attempt -lt 10; $attempt++) { + if ([Win32Interop]::GetWindowProcessId([Win32Interop]::GetForegroundWindow()) -eq {{PID}}) { return $true } + Start-Sleep -Milliseconds 50 + } + return $false +} $previous = [Win32Interop]::GetForegroundWindow() try { $process = Get-Process -Id {{PID}} -ErrorAction Stop } catch { throw 'Unity process was not found: {{PID}}' } $handle = $process.MainWindowHandle if ($handle -eq 0) { throw 'Unity process has no main window handle: {{PID}}' } -$shown = [Win32Interop]::ShowWindowAsync($handle, 9) -if (-not $shown) { throw 'Failed to show Unity window' } -$focused = [Win32Interop]::SetForegroundWindow($handle) -if (-not $focused) { - $shell = New-Object -ComObject WScript.Shell - $focused = $shell.AppActivate({{PID}}) +# Why: SW_RESTORE on a non-minimized window would shrink a maximized Unity window, so restore only when minimized. +if ([Win32Interop]::IsIconic($handle)) { + $shown = [Win32Interop]::ShowWindowAsync($handle, 9) + if (-not $shown) { throw 'Failed to show Unity window' } +} +[void][Win32Interop]::SetForegroundWindow($handle) +if (-not (Test-TargetForeground)) { + # Why: the Windows foreground lock rejects SetForegroundWindow from background processes; sharing the + # foreground thread's input queue via AttachThreadInput lifts that restriction. + $currentThreadId = [Win32Interop]::GetCurrentThreadId() + $foreground = [Win32Interop]::GetForegroundWindow() + $foregroundThreadId = [Win32Interop]::GetWindowThreadProcessId($foreground, [IntPtr]::Zero) + $targetThreadId = [Win32Interop]::GetWindowThreadProcessId($handle, [IntPtr]::Zero) + $attachedForeground = $false + $attachedTarget = $false + try { + if ($foregroundThreadId -ne 0 -and $foregroundThreadId -ne $currentThreadId) { + $attachedForeground = [Win32Interop]::AttachThreadInput($currentThreadId, $foregroundThreadId, $true) + } + if ($targetThreadId -ne 0 -and $targetThreadId -ne $currentThreadId) { + $attachedTarget = [Win32Interop]::AttachThreadInput($currentThreadId, $targetThreadId, $true) + } + [void][Win32Interop]::BringWindowToTop($handle) + [void][Win32Interop]::SetForegroundWindow($handle) + } finally { + if ($attachedTarget) { [void][Win32Interop]::AttachThreadInput($currentThreadId, $targetThreadId, $false) } + if ($attachedForeground) { [void][Win32Interop]::AttachThreadInput($currentThreadId, $foregroundThreadId, $false) } + } +} +if (-not (Test-TargetForeground)) { + # Why: a transient Alt keypress makes this process the last input source, a documented workaround + # that unlocks SetForegroundWindow when AttachThreadInput alone is not enough. + [Win32Interop]::keybd_event(0x12, 0, 0, [UIntPtr]::Zero) + [void][Win32Interop]::SetForegroundWindow($handle) + [Win32Interop]::keybd_event(0x12, 0, 2, [UIntPtr]::Zero) +} +if (-not (Test-TargetForeground)) { + throw 'Windows refused to bring the Unity window (PID: {{PID}}) to the foreground (foreground lock). Click the Unity window or its taskbar icon to focus it manually.' } -if (-not $focused) { throw 'Failed to focus Unity window' } Write-Output $previous.ToInt64() diff --git a/cli/common/unityprocess/focus_windows.go b/cli/common/unityprocess/focus_windows.go index 7591fc6b0..07e72c4da 100644 --- a/cli/common/unityprocess/focus_windows.go +++ b/cli/common/unityprocess/focus_windows.go @@ -16,7 +16,7 @@ func FocusUnityProcess(ctx context.Context, pid int) error { command := exec.CommandContext(commandContext, windowsPowerShellCommand, "-NoProfile", "-Command", script) command.Stderr = &stderr if err := command.Run(); err != nil { - return commandErrorWithStderr(err, stderr.String()) + return focusCommandError(commandContext.Err(), err, stderr.String()) } return nil } @@ -30,7 +30,7 @@ func FocusUnityProcessWithRestore(ctx context.Context, pid int) (RestoreFocusFun command.Stderr = &stderr output, err := command.Output() if err != nil { - return nil, commandErrorWithStderr(err, stderr.String()) + return nil, focusCommandError(commandContext.Err(), err, stderr.String()) } previousHandle := parseWindowsForegroundHandle(string(output)) if previousHandle == 0 { @@ -49,7 +49,7 @@ func restoreWindowsForegroundWindow(ctx context.Context, handle int64) error { command := exec.CommandContext(commandContext, windowsPowerShellCommand, "-NoProfile", "-Command", script) command.Stderr = &stderr if err := command.Run(); err != nil { - return commandErrorWithStderr(err, stderr.String()) + return focusCommandError(commandContext.Err(), err, stderr.String()) } return nil } diff --git a/cli/common/unityprocess/process_test.go b/cli/common/unityprocess/process_test.go index c0fb37b27..007db34c8 100644 --- a/cli/common/unityprocess/process_test.go +++ b/cli/common/unityprocess/process_test.go @@ -110,40 +110,57 @@ func TestExtractProjectPathSupportsEqualsAndSpaces(t *testing.T) { } } -// Verifies the embedded Windows focus script throws instead of silently returning on failures. -func TestBuildFocusUnityProcessWindowsScriptThrowsOnFailures(t *testing.T) { +// Verifies the embedded Windows focus script verifies the foreground result and throws instead of trusting API return values. +func TestBuildFocusUnityProcessWindowsScriptVerifiesForegroundAndThrowsOnFailures(t *testing.T) { script := buildFocusUnityProcessWindowsScript(123) + assertWindowsFocusScriptContract(t, script) +} + +// Verifies the embedded Windows focus-with-restore script captures the previous foreground window and shares the focus contract. +func TestBuildFocusUnityProcessWindowsWithRestoreScriptCapturesForegroundWindow(t *testing.T) { + script := buildFocusUnityProcessWindowsWithRestoreScript(123) + + assertWindowsFocusScriptContract(t, script) for _, expected := range []string{ - "throw 'Unity process was not found: 123'", - "throw 'Unity process has no main window handle: 123'", - "throw 'Failed to show Unity window'", - "$focused = $shell.AppActivate(123)", - "throw 'Failed to focus Unity window'", + "$previous = [Win32Interop]::GetForegroundWindow()", + "Write-Output $previous.ToInt64()", } { if !strings.Contains(script, expected) { t.Fatalf("script missing %q: %s", expected, script) } } - if strings.Contains(script, "catch { return }") || strings.Contains(script, "{ return }") { - t.Fatalf("script should not silently return: %s", script) - } } -// Verifies the embedded Windows focus-with-restore script captures the previous foreground window. -func TestBuildFocusUnityProcessWindowsWithRestoreScriptCapturesForegroundWindow(t *testing.T) { - script := buildFocusUnityProcessWindowsWithRestoreScript(123) - +// Asserts the shared Windows focus contract: escalation techniques, foreground verification, and no trust in AppActivate. +func assertWindowsFocusScriptContract(t *testing.T, script string) { + t.Helper() for _, expected := range []string{ - "GetForegroundWindow", - "$previous = [Win32Interop]::GetForegroundWindow()", - "Write-Output $previous.ToInt64()", - "$focused = $shell.AppActivate(123)", + "throw 'Unity process was not found: 123'", + "throw 'Unity process has no main window handle: 123'", + "if ([Win32Interop]::IsIconic($handle)) {", + "throw 'Failed to show Unity window'", + "function Test-TargetForeground", + "[Win32Interop]::GetWindowProcessId([Win32Interop]::GetForegroundWindow()) -eq 123", + "AttachThreadInput($currentThreadId, $foregroundThreadId, $true)", + "AttachThreadInput($currentThreadId, $targetThreadId, $true)", + "AttachThreadInput($currentThreadId, $targetThreadId, $false)", + "AttachThreadInput($currentThreadId, $foregroundThreadId, $false)", + "BringWindowToTop", + "keybd_event(0x12, 0, 0, [UIntPtr]::Zero)", + "keybd_event(0x12, 0, 2, [UIntPtr]::Zero)", + "throw 'Windows refused to bring the Unity window (PID: 123) to the foreground (foreground lock). Click the Unity window or its taskbar icon to focus it manually.'", } { if !strings.Contains(script, expected) { t.Fatalf("script missing %q: %s", expected, script) } } + if strings.Contains(script, "AppActivate") { + t.Fatalf("script must not trust AppActivate return values: %s", script) + } + if strings.Contains(script, "catch { return }") || strings.Contains(script, "{ return }") { + t.Fatalf("script should not silently return: %s", script) + } } // Verifies the embedded Windows restore script fails when the saved foreground window cannot be restored.