improvements before version bump

checks/cli.go

@@ -85,6 +85,7 @@ func runCLICommandWithLimits(
 		cmd = exec.CommandContext(ctx, "sh", "-c", finalCommand)
 	}
 
+	configureCommandCancellation(cmd)
 	cmd.Env = append(os.Environ(), "LANG=en_US.UTF-8")
 	cmd.WaitDelay = commandWaitDelay
 	cancelForOutputLimit := func() {
@@ -94,6 +95,8 @@ func runCLICommandWithLimits(
 	stderr := newBoundedBuffer(maxOutputBytesPerStream, cancelForOutputLimit)
 	cmd.Stdout = stdout
 	cmd.Stderr = stderr
+	stopSignalForwarding := forwardSignalsToCommand(cmd)
+	defer stopSignalForwarding()
 	err := cmd.Run()
 	if ee, ok := err.(*exec.ExitError); ok {
 		result.ExitCode = ee.ExitCode()

checks/command_process_other.go

@@ -0,0 +1,11 @@
+//go:build !aix && !darwin && !dragonfly && !freebsd && !linux && !netbsd && !openbsd && !solaris && !windows
+
+package checks
+
+import "os/exec"
+
+func configureCommandCancellation(cmd *exec.Cmd) {}
+
+func forwardSignalsToCommand(cmd *exec.Cmd) func() {
+	return func() {}
+}

checks/command_process_unix.go

@@ -0,0 +1,63 @@
+//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
+
+package checks
+
+import (
+	"errors"
+	"os"
+	"os/exec"
+	"os/signal"
+	"sync"
+	"syscall"
+)
+
+func configureCommandCancellation(cmd *exec.Cmd) {
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	cmd.Cancel = func() error { return killCommandProcessGroup(cmd) }
+}
+
+func forwardSignalsToCommand(cmd *exec.Cmd) func() {
+	signals := make(chan os.Signal, 1)
+	done := make(chan struct{})
+	signal.Notify(signals, os.Interrupt, syscall.SIGTERM)
+	var forwardOnce sync.Once
+	forward := func(received os.Signal) {
+		forwardOnce.Do(func() {
+			_ = killCommandProcessGroup(cmd)
+			signal.Reset(received)
+			if unixSignal, ok := received.(syscall.Signal); ok {
+				_ = syscall.Kill(os.Getpid(), unixSignal)
+			}
+		})
+	}
+
+	go func() {
+		select {
+		case received := <-signals:
+			forward(received)
+		case <-done:
+		}
+	}()
+
+	return func() {
+		signal.Stop(signals)
+		select {
+		case received := <-signals:
+			forward(received)
+		default:
+		}
+		close(done)
+	}
+}
+
+func killCommandProcessGroup(cmd *exec.Cmd) error {
+	if cmd.Process == nil {
+		return os.ErrProcessDone
+	}
+
+	err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
+	if errors.Is(err, syscall.ESRCH) {
+		return os.ErrProcessDone
+	}
+	return err
+}

checks/command_process_unix_test.go

@@ -0,0 +1,52 @@
+//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
+
+package checks
+
+import (
+	"errors"
+	"strconv"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+
+	api "github.com/bootdotdev/bootdev/client"
+)
+
+func TestRunCLICommandTimeoutKillsDescendants(t *testing.T) {
+	result := runCLICommandWithLimits(
+		api.CLIStepCLICommand{Command: "sleep 30 & echo $!; wait"},
+		map[string]string{},
+		100*time.Millisecond,
+		1024,
+	)
+	if !strings.Contains(result.Err, "command timed out") {
+		t.Fatalf("command error = %q, want timeout error", result.Err)
+	}
+
+	pid, err := strconv.Atoi(strings.TrimSpace(result.Stdout))
+	if err != nil {
+		t.Fatalf("child PID output = %q: %v", result.Stdout, err)
+	}
+	childAlive := true
+	t.Cleanup(func() {
+		if childAlive {
+			_ = syscall.Kill(pid, syscall.SIGKILL)
+		}
+	})
+
+	deadline := time.Now().Add(time.Second)
+	for time.Now().Before(deadline) {
+		err := syscall.Kill(pid, 0)
+		if errors.Is(err, syscall.ESRCH) {
+			childAlive = false
+			return
+		}
+		if err != nil {
+			t.Fatalf("check child process %d: %v", pid, err)
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+
+	t.Fatalf("child process %d survived command cancellation", pid)
+}

checks/command_process_windows.go

@@ -0,0 +1,38 @@
+//go:build windows
+
+package checks
+
+import (
+	"errors"
+	"os"
+	"os/exec"
+	"strconv"
+)
+
+func configureCommandCancellation(cmd *exec.Cmd) {
+	cmd.Cancel = func() error {
+		if cmd.Process == nil {
+			return os.ErrProcessDone
+		}
+
+		treeKill := exec.Command(
+			"taskkill.exe",
+			"/PID", strconv.Itoa(cmd.Process.Pid),
+			"/T",
+			"/F",
+		)
+		if err := treeKill.Run(); err == nil {
+			return nil
+		}
+
+		err := cmd.Process.Kill()
+		if errors.Is(err, os.ErrProcessDone) {
+			return os.ErrProcessDone
+		}
+		return err
+	}
+}
+
+func forwardSignalsToCommand(cmd *exec.Cmd) func() {
+	return func() {}
+}

cmd/localtest.go

@@ -19,7 +19,7 @@ import (
 
 func init() {
 	rootCmd.AddCommand(localTestCmd)
-	localTestCmd.Flags().BoolVarP(&verboseOutput, "verbose", "v", false, "show detailed output for every step")
+	localTestCmd.Flags().BoolVarP(&verboseOutput, "verbose", "v", false, "show detailed final output for every step")
 }
 
 var localTestCmd = &cobra.Command{

cmd/run.go

@@ -8,7 +8,7 @@ func init() {
 	rootCmd.AddCommand(runCmd)
 	runCmd.Flags().BoolVarP(&forceSubmit, "submit", "s", false, "shortcut flag to submit after running")
 	runCmd.Flags().BoolVar(&debugSubmission, "debug", false, "log submission request/response debug output")
-	runCmd.Flags().BoolVarP(&verboseOutput, "verbose", "v", false, "show detailed output for every step")
+	runCmd.Flags().BoolVarP(&verboseOutput, "verbose", "v", false, "with --submit, show detailed final output for every step")
 }
 
 // runCmd represents the run command

cmd/submit.go

@@ -26,7 +26,7 @@ var (
 func init() {
 	rootCmd.AddCommand(submitCmd)
 	submitCmd.Flags().BoolVar(&debugSubmission, "debug", false, "log submission request/response debug output")
-	submitCmd.Flags().BoolVarP(&verboseOutput, "verbose", "v", false, "show detailed output for every step")
+	submitCmd.Flags().BoolVarP(&verboseOutput, "verbose", "v", false, "show detailed final output for every step")
 }
 
 // submitCmd represents the submit command
@@ -92,14 +92,25 @@ func submissionHandler(cmd *cobra.Command, args []string) error {
 		if err != nil {
 			return err
 		}
-		checks.ApplySubmissionResults(data, submissionEvent.StructuredErrCLI, ch)
+		submissionErr := applySubmissionEvent(data, submissionEvent, ch)
 		finalise(submissionEvent)
+		if submissionErr != nil {
+			return submissionErr
+		}
 	} else {
 		finalise(api.LessonSubmissionEvent{})
 	}
 	return nil
 }
 
+func applySubmissionEvent(data api.CLIData, event api.LessonSubmissionEvent, ch chan tea.Msg) error {
+	if event.ResultSlug == api.VerificationResultSlugSystemError {
+		return errors.New("lesson verification failed due to a system error; please try again")
+	}
+	checks.ApplySubmissionResults(data, event.StructuredErrCLI, ch)
+	return nil
+}
+
 func reportDebugFileWrite(path string, err error) {
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "warning: failed to write submission debug output: %v\n", err)

cmd/submit_test.go

@@ -0,0 +1,29 @@
+package cmd
+
+import (
+	"strings"
+	"testing"
+
+	api "github.com/bootdotdev/bootdev/client"
+	tea "github.com/charmbracelet/bubbletea"
+)
+
+func TestApplySubmissionEventRejectsSystemErrorWithoutMarkingStepsPassed(t *testing.T) {
+	data := api.CLIData{Steps: []api.CLIStep{{
+		CLICommand: &api.CLIStepCLICommand{Tests: []api.CLICommandTest{{}}},
+	}}}
+	ch := make(chan tea.Msg, 1)
+
+	err := applySubmissionEvent(data, api.LessonSubmissionEvent{
+		ResultSlug: api.VerificationResultSlugSystemError,
+	}, ch)
+	if err == nil || !strings.Contains(err.Error(), "system error") {
+		t.Fatalf("applySubmissionEvent() error = %v, want system error", err)
+	}
+
+	select {
+	case msg := <-ch:
+		t.Fatalf("system error unexpectedly emitted result message: %#v", msg)
+	default:
+	}
+}

render/variables.go

@@ -13,6 +13,7 @@ import (
 type variableEntry struct {
 	name        string
 	value       string
+	found       bool
 	description string
 }
 
@@ -27,41 +28,46 @@ func renderVariableSection(title string, entries []variableEntry) string {
 	var str strings.Builder
 	fmt.Fprintf(&str, "  %s: \n", title)
 	for _, entry := range entries {
-		fmt.Fprintf(&str, "   - %s: %s (%s)\n", entry.name, formatVariableValue(entry.value), entry.description)
+		fmt.Fprintf(&str, "   - %s: %s (%s)\n", entry.name, formatVariableValue(entry.value, entry.found), entry.description)
 	}
 	return str.String()
 }
 
-func formatVariableValue(value string) string {
-	if value == "" {
+func formatVariableValue(value string, found bool) string {
+	if !found {
 		return "[not found]"
 	}
+	if value == "" {
+		return "[empty]"
+	}
 	return value
 }
 
 func savedVariablesForHTTPResult(result api.HTTPRequestResult) []variableEntry {
 	var entries []variableEntry
 	for _, responseVariable := range result.Request.ResponseVariables {
-		value := result.Variables[responseVariable.Name]
-		if value == "" {
+		value, found := result.Variables[responseVariable.Name]
+		if !found {
 			continue
 		}
 
 		description := responseVariableDescription(responseVariable)
 		entries = append(entries, variableEntry{
 			name:        responseVariable.Name,
 			value:       value,
+			found:       true,
 			description: description,
 		})
 	}
 	for _, responseHeaderVariable := range result.Request.ResponseHeaderVariables {
-		value := result.Variables[responseHeaderVariable.Name]
-		if value == "" {
+		value, found := result.Variables[responseHeaderVariable.Name]
+		if !found {
 			continue
 		}
 		entries = append(entries, variableEntry{
 			name:        responseHeaderVariable.Name,
 			value:       value,
+			found:       true,
 			description: responseHeaderVariableDescription(responseHeaderVariable),
 		})
 	}
@@ -71,7 +77,7 @@ func savedVariablesForHTTPResult(result api.HTTPRequestResult) []variableEntry {
 func missingSaveVariablesForHTTPResult(result api.HTTPRequestResult) []variableEntry {
 	var entries []variableEntry
 	for _, responseVariable := range result.Request.ResponseVariables {
-		if result.Variables[responseVariable.Name] != "" {
+		if _, found := result.Variables[responseVariable.Name]; found {
 			continue
 		}
 
@@ -82,7 +88,7 @@ func missingSaveVariablesForHTTPResult(result api.HTTPRequestResult) []variableE
 		})
 	}
 	for _, responseHeaderVariable := range result.Request.ResponseHeaderVariables {
-		if result.Variables[responseHeaderVariable.Name] != "" {
+		if _, found := result.Variables[responseHeaderVariable.Name]; found {
 			continue
 		}
 		entries = append(entries, variableEntry{
@@ -108,7 +114,7 @@ func availableVariablesForHTTPResult(result api.HTTPRequestResult) (entries []va
 			return
 		}
 		expectsVariables = true
-		value := result.Variables[name]
+		value, found := result.Variables[name]
 		key := name + "\x00" + description
 		if seen[key] {
 			return
@@ -117,6 +123,7 @@ func availableVariablesForHTTPResult(result api.HTTPRequestResult) (entries []va
 		entries = append(entries, variableEntry{
 			name:        name,
 			value:       value,
+			found:       found,
 			description: description,
 		})
 	}
@@ -167,7 +174,7 @@ func availableVariablesForCLIResult(result api.CLICommandResult) (entries []vari
 
 	add := func(name, description string) {
 		expectsVariables = true
-		value := result.Variables[name]
+		value, found := result.Variables[name]
 		key := name + "\x00" + description
 		if seen[key] {
 			return
@@ -176,6 +183,7 @@ func availableVariablesForCLIResult(result api.CLICommandResult) (entries []vari
 		entries = append(entries, variableEntry{
 			name:        name,
 			value:       value,
+			found:       found,
 			description: description,
 		})
 	}