diff --git a/tools/thothctl/cmd/thothctl/main_test.go b/tools/thothctl/cmd/thothctl/main_test.go index 205ec6d1..4c255ee5 100644 --- a/tools/thothctl/cmd/thothctl/main_test.go +++ b/tools/thothctl/cmd/thothctl/main_test.go @@ -3,6 +3,7 @@ package main import ( "bytes" "context" + "encoding/json" "errors" "fmt" "io" @@ -135,37 +136,127 @@ func TestRunWorkspacePublicDispatchExitMatrix(t *testing.T) { func TestRunWorkspaceBoundsFinalJSONEncoding(t *testing.T) { fixture := newCLIFixture(t, "") fixture.setEnvironment(t) - result := fmt.Sprintf(`{"schemaVersion":1,"status":"succeeded","code":"ok","workspaceId":"psd","workspaceRevision":"%s","descriptorBlob":"%s","operation":"inspect","completedStages":[],"warnings":[%q]}`, strings.Repeat("0", 40), strings.Repeat("a", 40), strings.Repeat(`"`, 524000)) - t.Setenv("THOTHCTL_FAKE_WORKSPACE_RESULT", result) - var stdout, stderr bytes.Buffer - if code := run(context.Background(), []string{"--installation", fixture.installationPath, "workspace", "inspect", "--workspace", "psd", "--json"}, &stdout, &stderr); code != 1 { - t.Fatalf("exit = %d, want operational failure 1", code) - } - if stdout.Len() > 1<<20 { - t.Fatalf("stdout length = %d, exceeds 1 MiB", stdout.Len()) + for _, tc := range []struct { + name string + extraBytes int + wantCode int + }{ + {name: "exact final limit", wantCode: 0}, + {name: "one encoded byte over", extraBytes: 1, wantCode: 1}, + } { + t.Run(tc.name, func(t *testing.T) { + payload, encodedLength := boundedWorkspaceResultPayload(t, (1<<20)+tc.extraBytes) + if len(payload) >= 1<<20 { + t.Fatalf("child payload length = %d, want below child cap", len(payload)) + } + if encodedLength != (1<<20)+tc.extraBytes { + t.Fatalf("final encoded length = %d, want %d", encodedLength, (1<<20)+tc.extraBytes) + } + writeWorkspaceResultFile(t, fixture, payload) + var stdout, stderr bytes.Buffer + code := run(context.Background(), []string{"--installation", fixture.installationPath, "workspace", "inspect", "--workspace", "psd", "--json"}, &stdout, &stderr) + if code != tc.wantCode { + t.Fatalf("exit = %d, want %d (stdout=%d stderr=%q)", code, tc.wantCode, stdout.Len(), stderr.String()) + } + if tc.extraBytes == 0 { + if stdout.Len() != 1<<20 || stderr.Len() != 0 { + t.Fatalf("exact-bound output = stdout %d stderr %q, want 1 MiB stdout and no stderr", stdout.Len(), stderr.String()) + } + } else if stdout.Len() != 0 || stderr.String() != "thothctl: workspace result exceeds output limit\n" { + t.Fatalf("over-bound output = stdout %d stderr %q", stdout.Len(), stderr.String()) + } + }) } } func TestRunWorkspaceBoundsFinalHumanEncoding(t *testing.T) { fixture := newCLIFixture(t, "") fixture.setEnvironment(t) - result := fmt.Sprintf(`{"schemaVersion":1,"status":"succeeded","code":"ok","workspaceId":"psd","workspaceRevision":"%s","descriptorBlob":"%s","operation":"inspect","completedStages":[%q]}`, strings.Repeat("0", 40), strings.Repeat("a", 40), strings.Repeat("x", (1<<20)-100)) - t.Setenv("THOTHCTL_FAKE_WORKSPACE_RESULT", result) + payload := encodeWorkspaceResultForTest(workspaceops.Result{ + SchemaVersion: 1, Status: "succeeded", Code: "ok", WorkspaceID: "psd", + WorkspaceRevision: strings.Repeat("0", 40), DescriptorBlob: strings.Repeat("a", 40), + Operation: "inspect", CompletedStages: []string{}, + }) + writeWorkspaceResultFile(t, fixture, payload) var stdout, stderr bytes.Buffer - _ = run(context.Background(), []string{"--installation", fixture.installationPath, "workspace", "inspect", "--workspace", "psd"}, &stdout, &stderr) - if stdout.Len() > 1<<20 { - t.Fatalf("stdout length = %d, exceeds 1 MiB", stdout.Len()) + code := run(context.Background(), []string{"--installation", fixture.installationPath, "workspace", "inspect", "--workspace", "psd"}, &stdout, &stderr) + if code != 0 || stderr.Len() != 0 { + t.Fatalf("successful child result was not rendered: exit=%d stderr=%q", code, stderr.String()) + } + if !strings.HasPrefix(stdout.String(), "Workspace psd: succeeded (ok)\n") { + t.Fatalf("human output = %q, want successful workspace rendering", stdout.String()) + } + + // A human rendering can only be larger than the child JSON when it is supplied directly; + // exercise the same bounded public writer without making the child hit its 1 MiB cap. + large := workspaceops.Result{ + SchemaVersion: 1, Status: "succeeded", Code: "ok", WorkspaceID: "psd", + WorkspaceRevision: strings.Repeat("0", 40), DescriptorBlob: strings.Repeat("a", 40), + Operation: "inspect", CompletedStages: []string{strings.Repeat("x", 1<<20)}, + } + var bounded bytes.Buffer + renderWorkspaceHuman(&boundedWriter{dst: &bounded, maximum: 1 << 20}, large) + if bounded.Len() != 1<<20 { + t.Fatalf("bounded human output length = %d, want exactly 1 MiB", bounded.Len()) } } func TestRunBoundsParseErrorStderr(t *testing.T) { fixture := newCLIFixture(t, "") fixture.setEnvironment(t) + unknownAction := strings.Repeat("x", 70<<10) var stdout, stderr bytes.Buffer - _ = run(context.Background(), []string{"--installation", fixture.installationPath, "workspace", "inspect", "--workspace", "psd", "--unknown", strings.Repeat("x", 70<<10)}, &stdout, &stderr) - if stderr.Len() > 64<<10 { - t.Fatalf("stderr length = %d, exceeds 64 KiB", stderr.Len()) + code := run(context.Background(), []string{"--installation", fixture.installationPath, "workspace", unknownAction, "--workspace", "psd"}, &stdout, &stderr) + if code != 2 { + t.Fatalf("exit = %d, want usage exit 2", code) } + if stdout.Len() != 0 || stderr.Len() != 64<<10 { + t.Fatalf("bounded parse error = stdout %d stderr %d, want 0 and exactly 64 KiB", stdout.Len(), stderr.Len()) + } + if !strings.HasPrefix(stderr.String(), "thothctl: unknown workspace command: workspace ") || !strings.HasSuffix(stderr.String(), "x") { + t.Fatalf("parse error was not the truncated unknown action: prefix/suffix mismatch") + } +} + +func encodeWorkspaceResultForTest(result workspaceops.Result) []byte { + var encoded bytes.Buffer + encoder := json.NewEncoder(&encoded) + encoder.SetEscapeHTML(false) + if err := encoder.Encode(result); err != nil { + panic(err) + } + return encoded.Bytes() +} + +func boundedWorkspaceResultPayload(t *testing.T, finalLength int) ([]byte, int) { + t.Helper() + result := workspaceops.Result{ + SchemaVersion: 1, Status: "succeeded", Code: "ok", WorkspaceID: "psd", + WorkspaceRevision: strings.Repeat("0", 40), DescriptorBlob: strings.Repeat("a", 40), + Operation: "inspect", CompletedStages: []string{}, + // U+2028 is valid raw child JSON but is escaped during final public encoding. + Warnings: []string{strings.Repeat("\u2028", 1000)}, + } + encoded := encodeWorkspaceResultForTest(result) + if len(encoded) >= finalLength { + t.Fatalf("base encoded result length = %d, cannot reach target %d", len(encoded), finalLength) + } + result.Warnings[0] += strings.Repeat("x", finalLength-len(encoded)) + encoded = encodeWorkspaceResultForTest(result) + if len(encoded) != finalLength { + t.Fatalf("final encoded result length = %d, want %d", len(encoded), finalLength) + } + child := bytes.ReplaceAll(encoded, []byte(`\u2028`), []byte("\u2028")) + return child, len(encoded) +} + +func writeWorkspaceResultFile(t *testing.T, fixture cliFixture, payload []byte) { + t.Helper() + path := filepath.Join(fixture.root, "workspace-result.json") + if err := os.WriteFile(path, payload, 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("THOTHCTL_FAKE_WORKSPACE_RESULT_FILE", path) } func TestRunWorkspaceOperationalFailureExitsOne(t *testing.T) { @@ -840,7 +931,11 @@ case " $* " in printf '%s\n' '{"volumes":{"settings":{}},"services":{"core":{"image":"thothii-core:local","environment":{"THT_LLM_URL":"https://llm.example.invalid"}}}}' fi ;; *" run --rm --no-deps --no-TTY workspace-maintenance "*) - printf '%s\n' "${THOTHCTL_FAKE_WORKSPACE_RESULT:-}" + workspace_result="${THOTHCTL_FAKE_WORKSPACE_RESULT:-}" + if [ -n "${THOTHCTL_FAKE_WORKSPACE_RESULT_FILE:-}" ]; then + workspace_result=$(/bin/cat "$THOTHCTL_FAKE_WORKSPACE_RESULT_FILE") + fi + printf '%s\n' "$workspace_result" exit "${THOTHCTL_FAKE_WORKSPACE_EXIT:-0}" ;; *" run --rm --no-deps --no-TTY session-migrate "*) if [ "${THOTHCTL_FAKE_MIGRATION_EXIT:-0}" -ne 0 ]; then @@ -898,6 +993,7 @@ func (f cliFixture) setEnvContents(t *testing.T, env string) { t.Setenv("THOTHCTL_FAKE_MIGRATION_FAILURE", "") t.Setenv("THOTHCTL_FAKE_MIGRATION_EXIT", "0") t.Setenv("THOTHCTL_FAKE_WORKSPACE_RESULT", "") + t.Setenv("THOTHCTL_FAKE_WORKSPACE_RESULT_FILE", "") t.Setenv("THOTHCTL_FAKE_WORKSPACE_EXIT", "0") } diff --git a/tools/thothctl/internal/compose/process_windows_test.go b/tools/thothctl/internal/compose/process_windows_test.go index 68c3fd63..0825e7b7 100644 --- a/tools/thothctl/internal/compose/process_windows_test.go +++ b/tools/thothctl/internal/compose/process_windows_test.go @@ -7,7 +7,8 @@ import ( "errors" "os" "os/exec" - "path/filepath" + "strconv" + "strings" "testing" "time" @@ -32,72 +33,166 @@ func TestOwnedJobPolicyKillsDescendantsOnClose(t *testing.T) { } // TestWindowsProcessHelper is both a test helper and the child process used by the runtime -// ownership regression below. The child writes a marker only if it outlives its parent job. +// ownership regression below. The grandchild records its PID before the parent can exit. The +// parent then either exits, blocks for cancellation, or overflows stdout. func TestWindowsProcessHelper(t *testing.T) { mode := os.Getenv("THOTHCTL_WINDOWS_HELPER") if mode == "" { return } - marker := os.Getenv("THOTHCTL_WINDOWS_MARKER") - if mode == "child" { - time.Sleep(2 * time.Second) - _ = os.WriteFile(marker, []byte("survived"), 0o600) - return + pidFile := os.Getenv("THOTHCTL_WINDOWS_PID_FILE") + releaseFile := os.Getenv("THOTHCTL_WINDOWS_RELEASE_FILE") + if mode == "grandchild" { + if err := os.WriteFile(pidFile, []byte(strconv.Itoa(os.Getpid())), 0o600); err != nil { + t.Fatal(err) + } + for { + time.Sleep(time.Hour) + } } child := exec.Command(os.Args[0], "-test.run=TestWindowsProcessHelper", "--") - child.Env = append(os.Environ(), "THOTHCTL_WINDOWS_HELPER=child", "THOTHCTL_WINDOWS_MARKER="+marker) + child.Env = append(os.Environ(), + "THOTHCTL_WINDOWS_HELPER=grandchild", + "THOTHCTL_WINDOWS_PID_FILE="+pidFile, + ) if err := child.Start(); err != nil { t.Fatal(err) } + waitForWindowsPIDFile(t, pidFile) + waitForWindowsReleaseFile(t, releaseFile) if mode == "exit" { return } if mode == "cancel" { for { - time.Sleep(time.Second) + time.Sleep(time.Hour) } } for { _, _ = os.Stdout.Write([]byte("overflow\n")) + } +} + +func waitForWindowsPIDFile(t *testing.T, path string) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + contents, err := os.ReadFile(path) + if err == nil { + pid, parseErr := strconv.Atoi(strings.TrimSpace(string(contents))) + if parseErr == nil && pid > 0 { + return + } + } time.Sleep(time.Millisecond) } + t.Fatalf("grandchild PID was not recorded in %s", path) +} + +func waitForWindowsReleaseFile(t *testing.T, path string) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if _, err := os.Stat(path); err == nil { + return + } + time.Sleep(time.Millisecond) + } + t.Fatalf("test did not release Windows helper via %s", path) +} + +func openWindowsProcessForTest(t *testing.T, pid int) windows.Handle { + t.Helper() + const access = windows.SYNCHRONIZE | windows.PROCESS_QUERY_LIMITED_INFORMATION + handle, err := windows.OpenProcess(access, false, uint32(pid)) + if err != nil { + t.Fatalf("OpenProcess(%d): %v", pid, err) + } + return handle +} + +func assertWindowsProcessTerminated(t *testing.T, handle windows.Handle, pid int) { + t.Helper() + const stillActive = 259 + status, err := windows.WaitForSingleObject(handle, uint32((5*time.Second)/time.Millisecond)) + if err != nil { + t.Fatalf("WaitForSingleObject(%d): %v", pid, err) + } + if status != windows.WAIT_OBJECT_0 { + t.Fatalf("WaitForSingleObject(%d) = %#x, want signaled terminated process", pid, status) + } + var exitCode uint32 + if err := windows.GetExitCodeProcess(handle, &exitCode); err != nil { + t.Fatalf("GetExitCodeProcess(%d): %v", pid, err) + } + if exitCode == stillActive { + t.Fatalf("GetExitCodeProcess(%d) still reports STILL_ACTIVE", pid) + } } func TestOwnedProcessTerminatesDescendantOnOverflowCancelAndParentExit(t *testing.T) { for _, mode := range []string{"overflow", "cancel", "exit"} { t.Run(mode, func(t *testing.T) { - marker := filepath.Join(t.TempDir(), "descendant-marker") + pidFile, err := os.CreateTemp("", "thothctl-windows-grandchild-*.pid") + if err != nil { + t.Fatal(err) + } + pidPath := pidFile.Name() + _ = pidFile.Close() + _ = os.Remove(pidPath) + defer os.Remove(pidPath) + releaseFile, err := os.CreateTemp("", "thothctl-windows-release-*") + if err != nil { + t.Fatal(err) + } + releasePath := releaseFile.Name() + _ = releaseFile.Close() + _ = os.Remove(releasePath) + defer os.Remove(releasePath) t.Setenv("THOTHCTL_WINDOWS_HELPER", mode) - t.Setenv("THOTHCTL_WINDOWS_MARKER", marker) + t.Setenv("THOTHCTL_WINDOWS_PID_FILE", pidPath) + t.Setenv("THOTHCTL_WINDOWS_RELEASE_FILE", releasePath) ctx := context.Background() var cancel context.CancelFunc if mode == "cancel" { ctx, cancel = context.WithCancel(ctx) + defer cancel() } result := make(chan error, 1) go func() { - _, err := runBoundedWindows(ctx, os.Args[0], []string{"-test.run=TestWindowsProcessHelper", "--"}, nil, CaptureLimits{StdoutBytes: 1024, StderrBytes: 1024}) - result <- err + _, runErr := runBoundedWindows(ctx, os.Args[0], []string{"-test.run=TestWindowsProcessHelper", "--"}, nil, CaptureLimits{StdoutBytes: 1024, StderrBytes: 1024}) + result <- runErr }() + + waitForWindowsPIDFile(t, pidPath) + contents, err := os.ReadFile(pidPath) + if err != nil { + t.Fatal(err) + } + pid, err := strconv.Atoi(strings.TrimSpace(string(contents))) + if err != nil { + t.Fatal(err) + } + handle := openWindowsProcessForTest(t, pid) + defer windows.CloseHandle(handle) + if err := os.WriteFile(releasePath, []byte("release"), 0o600); err != nil { + t.Fatal(err) + } if mode == "cancel" { - time.Sleep(100 * time.Millisecond) cancel() } select { - case err := <-result: - if mode == "overflow" && !errors.Is(err, ErrOutputLimit) { - t.Fatalf("overflow error = %v, want ErrOutputLimit", err) + case runErr := <-result: + if mode == "overflow" && !errors.Is(runErr, ErrOutputLimit) { + t.Fatalf("overflow error = %v, want ErrOutputLimit", runErr) } - if mode == "cancel" && !errors.Is(err, context.Canceled) { - t.Fatalf("cancel error = %v, want context.Canceled", err) + if mode == "cancel" && !errors.Is(runErr, context.Canceled) { + t.Fatalf("cancel error = %v, want context.Canceled", runErr) } case <-time.After(5 * time.Second): t.Fatal("owned process did not terminate") } - time.Sleep(150 * time.Millisecond) - if _, err := os.Stat(marker); !errors.Is(err, os.ErrNotExist) { - t.Fatalf("descendant marker error = %v; descendant survived %s termination", err, mode) - } + assertWindowsProcessTerminated(t, handle, pid) }) } }