Add add-wizard integration coverage for manifest bootstrap ordering

docs/adr/47517-pty-backed-integration-test-harness-for-add-wizard-bootstrap.md

@@ -0,0 +1,43 @@
+# ADR-47517: PTY-Backed Integration Test Harness for Add-Wizard Manifest Bootstrap Ordering
+
+**Date**: 2026-07-23
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+### Context
+
+PR #47462 changed `add-wizard` to surface `aw.yml` config actions before the generic engine setup flow, ensuring pre-install bootstrap steps (such as repo-variable prompts) are presented to users before engine selection. This ordering invariant was verified only by unit tests, which cannot simulate a real interactive terminal. Validating the terminal-boundary behavior — that prompts appear in the correct order on a real PTY — requires integration tests that exercise the full `add-wizard` binary under a live interactive session. The existing tuistory-based tests require `npx`, live `gh` authentication, and network access, making them unsuitable for isolated environments.
+
+### Decision
+
+We will introduce a PTY-backed integration test harness using `github.com/creack/pty` to drive the `add-wizard` binary in a real pseudo-terminal, combined with a fake `gh` CLI stub that satisfies the binary's GitHub API calls without external network access. Tests set up a temporary git repository containing a local package fixture with a known `aw.yml` manifest and assert on the order in which prompts appear in the PTY output stream.
+
+### Alternatives Considered
+
+#### Alternative 1: Extend the Existing Tuistory-Based Tests
+
+The project already uses `tuistory` to run add-wizard sessions against a real interactive terminal. Extending those tests to cover the manifest bootstrap case would reuse existing infrastructure. However, tuistory tests require live `gh` authentication and `npx` availability; they cannot run in environments without external network access or a real GitHub login, making them unsuitable as the sole regression guard for this ordering invariant. This alternative was not chosen because the new test scenario must be isolatable from external state.
+
+#### Alternative 2: Assert Ordering via Unit Tests with a Fake Terminal Writer
+
+Unit tests could mock the `io.Writer` passed to the wizard and assert on the sequence of strings written. This avoids a real PTY and any external tooling. However, terminal prompt rendering in interactive CLI frameworks is sensitive to whether the output is an actual terminal; a fake writer changes the code path (skipping interactive prompts entirely in CI-like environments) and therefore cannot confirm that the visual ordering of prompts at the real terminal boundary is correct. This alternative was not chosen because it would not catch regressions in the actual interactive flow.
+
+### Consequences
+
+#### Positive
+- Provides a regression guard that exercises the full binary at the terminal boundary, catching ordering bugs that unit tests cannot detect.
+- Tests are self-contained: a fake `gh` stub and a temporary git repository eliminate all external dependencies, so the test suite runs in offline or restricted environments.
+
+#### Negative
+- Introduces `github.com/creack/pty` as a direct test dependency; PTY allocation is Unix-specific, so the new test cannot run on Windows.
+- The PTY harness adds non-trivial test infrastructure (mutex-protected buffer, done channel, interrupt helper) that future contributors must understand and maintain.
+
+#### Neutral
+- The fake `gh` CLI stub hard-codes specific GitHub API responses; tests that exercise code paths relying on responses outside the stub's scope will emit an error and exit non-zero, requiring stub expansion as the wizard evolves.
+- The test requires the compiled `add-wizard` binary to be present (via `globalBinaryPath`), linking it to the build step rather than running against library-level interfaces.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/cli/add_wizard_tuistory_integration_test.go

@@ -3,14 +3,18 @@
 package cli
 
 import (
+	"bytes"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
+	"github.com/creack/pty"
 	"github.com/github/gh-aw/pkg/fileutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -22,6 +26,13 @@ type addWizardTuistorySetup struct {
 	workflowPath string
 }
 
+type addWizardManifestTuistorySetup struct {
+	tempDir     string
+	binaryPath  string
+	packagePath string
+	fakeGHDir   string
+}
+
 func setupAddWizardTuistoryTest(t *testing.T) *addWizardTuistorySetup {
 	t.Helper()
 
@@ -79,18 +90,243 @@ func runTuistory(t *testing.T, args ...string) (string, error) {
 	return string(output), err
 }
 
+func runGitCommand(t *testing.T, dir string, args ...string) {
+	t.Helper()
+
+	cmd := exec.Command("git", args...)
+	cmd.Dir = dir
+	output, err := cmd.CombinedOutput()
+	require.NoError(t, err, "git %s failed: %s", strings.Join(args, " "), string(output))
+}
+
+func setupAddWizardManifestTuistoryTest(t *testing.T) *addWizardManifestTuistorySetup {
+	t.Helper()
+
+	tempDir, err := os.MkdirTemp("", "gh-aw-add-wizard-manifest-*")
+	require.NoError(t, err, "Failed to create temp directory")
+
+	runGitCommand(t, tempDir, "init")
+	runGitCommand(t, tempDir, "config", "user.name", "Test User")
+	runGitCommand(t, tempDir, "config", "user.email", "test@example.com")
+
+	packagePath := filepath.Join(tempDir, "local-package")
+	workflowsDir := filepath.Join(packagePath, "workflows")
+	require.NoError(t, os.MkdirAll(workflowsDir, 0755), "Failed to create local package directories")
+
+	manifestContent := `manifest-version: "1"
+name: Local Add Wizard Package
+files:
+  - workflows/bootstrap.md
+config:
+  - type: repo-variable
+    name: OPTIONAL_BOOTSTRAP_VAR
+    prompt: Enter optional bootstrap variable
+    description: Leave blank to skip this optional setup value.
+    optional: true
+  - type: handoff
+    message: Post-install handoff should not appear before engine selection.
+`
+	require.NoError(t, os.WriteFile(filepath.Join(packagePath, "aw.yml"), []byte(manifestContent), 0644), "Failed to write local package manifest")
+	require.NoError(t, os.WriteFile(filepath.Join(packagePath, "README.md"), []byte("# Local Add Wizard Package\n"), 0644), "Failed to write local package README")
+
+	workflowContent := `---
+name: Local Package Bootstrap Integration
+on:
+  workflow_dispatch:
+engine: copilot
+---
+
+# Local Package Bootstrap Integration
+
+This workflow is used by add-wizard manifest integration tests.
+`
+	require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "bootstrap.md"), []byte(workflowContent), 0644), "Failed to write local package workflow")
+
+	runGitCommand(t, tempDir, "add", "local-package")
+	runGitCommand(t, tempDir, "commit", "-m", "Add local package fixture")
+
+	fakeGHDir, err := os.MkdirTemp("", "gh-aw-fake-gh-*")
+	require.NoError(t, err, "Failed to create fake gh directory")
+
+	fakeGHScript := `#!/usr/bin/env bash
+set -euo pipefail
+
+if [[ "$#" -ge 2 && "$1" == "auth" && "$2" == "status" ]]; then
+  echo "github.com"
+  exit 0
+fi
+
+if [[ "$#" -ge 2 && "$1" == "repo" && "$2" == "view" ]]; then
+  echo "octo/example"
+  exit 0
+fi
+
+if [[ "$#" -ge 2 && "$1" == "api" ]]; then
+  path=""
+  for arg in "$@"; do
+    case "$arg" in
+      /repos/*|/orgs/*|/user)
+        path="$arg"
+        break
+        ;;
+    esac
+  done
+
+  case "$path" in
+    /repos/octo/example)
+      echo "public"
+      exit 0
+      ;;
+    /repos/octo/example/actions/permissions)
+      echo '{"enabled":true,"allowed_actions":"all"}'
+      exit 0
+      ;;
+    /repos/octo/example/collaborators/tester/permission)
+      echo '{"permission":"write"}'
+      exit 0
+      ;;
+    /repos/octo/example/actions/variables?per_page=100|/repos/octo/example/actions/secrets?per_page=100|/repos/octo/example/actions/secrets|/orgs/octo/actions/secrets)
+      exit 0
+      ;;
+    /user)
+      echo "tester"
+      exit 0
+      ;;
+  esac
+fi
+
+echo "unexpected gh invocation: $*" >&2
+exit 1
+`
+	fakeGHPath := filepath.Join(fakeGHDir, "gh")
+	require.NoError(t, os.WriteFile(fakeGHPath, []byte(fakeGHScript), 0755), "Failed to write fake gh script")
+
+	return &addWizardManifestTuistorySetup{
+		tempDir:     tempDir,
+		binaryPath:  globalBinaryPath,
+		packagePath: packagePath,
+		fakeGHDir:   fakeGHDir,
+	}
+}
+
 func waitForTuistoryText(t *testing.T, sessionName string, text string, timeoutMs int) {
 	t.Helper()
 	output, err := runTuistory(t, "-s", sessionName, "wait", text, "--timeout", fmt.Sprintf("%d", timeoutMs))
+	if err != nil && strings.Contains(output, "requires an interactive terminal") {
+		t.Skipf("tuistory session is not interactive in this environment: %s", output)
+	}
 	require.NoError(t, err, "Expected tuistory to find %q. Output: %s", text, output)
 }
 
+type lockedBuffer struct {
+	mu sync.Mutex
+	b  bytes.Buffer
+}
+
+func (b *lockedBuffer) Write(p []byte) (int, error) {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	return b.b.Write(p)
+}
+
+func (b *lockedBuffer) String() string {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	return b.b.String()
+}
+
+type interactivePTYSession struct {
+	cmd    *exec.Cmd
+	ptmx   *os.File
+	output lockedBuffer
+	done   chan error
+}
+
+func startInteractivePTYSession(t *testing.T, cmd *exec.Cmd) *interactivePTYSession {
+	t.Helper()
+
+	ptmx, err := pty.StartWithSize(cmd, &pty.Winsize{Cols: 140, Rows: 40})
+	require.NoError(t, err, "failed to start PTY")
+
+	session := &interactivePTYSession{
+		cmd:  cmd,
+		ptmx: ptmx,
+		done: make(chan error, 1),
+	}
+
+	go func() {
+		_, _ = io.Copy(&session.output, ptmx)
+	}()
+	go func() {
+		session.done <- cmd.Wait()
+	}()
+
+	return session
+}
+
+func (s *interactivePTYSession) readAll() string {
+	return s.output.String()
+}
+
+func (s *interactivePTYSession) waitForText(text string, timeout time.Duration) error {
+	deadline := time.Now().Add(timeout)
+	for time.Now().Before(deadline) {
+		if strings.Contains(s.readAll(), text) {
+			return nil
+		}
+
+		select {
+		case err := <-s.done:
+			if strings.Contains(s.readAll(), text) {
+				return nil
+			}
+			if err == nil {
+				return fmt.Errorf("process exited before output contained %q\nOutput:\n%s", text, s.readAll())
+			}
+			return fmt.Errorf("process exited before output contained %q: %w\nOutput:\n%s", text, err, s.readAll())
+		default:
+		}
+
+		time.Sleep(100 * time.Millisecond)
+	}
+
+	return fmt.Errorf("timed out waiting for %q\nOutput:\n%s", text, s.readAll())
+}
+
+func (s *interactivePTYSession) writeString(t *testing.T, text string) {
+	t.Helper()
+	_, err := io.WriteString(s.ptmx, text)
+	require.NoError(t, err, "failed to write to PTY")
+}
+
+func (s *interactivePTYSession) interrupt(t *testing.T) {
+	t.Helper()
+	s.writeString(t, "\x03")
+}
+
+func (s *interactivePTYSession) close(t *testing.T) {
+	t.Helper()
+	_ = s.ptmx.Close()
+	select {
+	case <-s.done:
+	case <-time.After(2 * time.Second):
+		_ = s.cmd.Process.Kill()
+	}
+}
+
 func TestTuistoryAddWizardIntegration(t *testing.T) {
 	const launchTimeoutMs = 30000 // 30 seconds
 
 	if _, err := exec.LookPath("npx"); err != nil {
 		t.Skip("npx not available, skipping tuistory add-wizard integration test")
 	}
+	if _, err := exec.LookPath("gh"); err != nil {
+		t.Skip("gh not available, skipping tuistory add-wizard integration test")
+	}
+	authCheck := exec.Command("gh", "auth", "status")
+	if output, err := authCheck.CombinedOutput(); err != nil {
+		t.Skipf("gh auth is not usable in this environment: %v (%s)", err, string(output))
+	}
 
 	versionOutput, err := runTuistory(t, "--version")
 	if err != nil {
@@ -112,6 +348,8 @@ func TestTuistoryAddWizardIntegration(t *testing.T) {
 		"--cols", "140",
 		"--rows", "40",
 		"--env", "CI=",
+		"--env", "CONTINUOUS_INTEGRATION=",
+		"--env", "GITHUB_ACTIONS=",
 		"--env", "GO_TEST_MODE=",
 		"--timeout", fmt.Sprintf("%d", launchTimeoutMs),
 	}
@@ -152,3 +390,55 @@ func TestTuistoryAddWizardIntegration(t *testing.T) {
 	_, statErr := os.Stat(addedWorkflowPath)
 	assert.ErrorIs(t, statErr, os.ErrNotExist, "Workflow file should not be created when add-wizard is cancelled")
 }
+
+func TestTuistoryAddWizardManifestBootstrapRunsBeforeEngineSelection(t *testing.T) {
+	setup := setupAddWizardManifestTuistoryTest(t)
+	defer func() {
+		_ = os.RemoveAll(setup.tempDir)
+		_ = os.RemoveAll(setup.fakeGHDir)
+	}()
+
+	const earlyPrompt = "Enter optional bootstrap variable"
+	const enginePrompt = "Which coding agent would you like to use?"
+	const lateMessage = "Post-install handoff should not appear before engine selection."
+
+	cmd := exec.Command(setup.binaryPath, "add-wizard", "./local-package", "--no-secret")
+	cmd.Dir = setup.tempDir
+	cmd.Env = append(os.Environ(),
+		"CI=",
+		"CONTINUOUS_INTEGRATION=",
+		"GITHUB_ACTIONS=",
+		"GO_TEST_MODE=",
+		"NO_COLOR=1",
+		"PAGER=cat",
+		"GH_PAGER=cat",
+		fmt.Sprintf("PATH=%s%c%s", setup.fakeGHDir, os.PathListSeparator, os.Getenv("PATH")),
+	)
+
+	session := startInteractivePTYSession(t, cmd)
+	defer session.close(t)
+
+	require.NoError(t, session.waitForText(earlyPrompt, 30*time.Second), "Expected pre-install bootstrap prompt")
+
+	beforeEngineOutput := session.readAll()
+	assert.Contains(t, beforeEngineOutput, earlyPrompt, "Expected pre-install bootstrap prompt to be shown")
+	assert.NotContains(t, beforeEngineOutput, enginePrompt, "Engine selection should not start before pre-install bootstrap setup")
+	assert.NotContains(t, beforeEngineOutput, lateMessage, "Post-install bootstrap steps should not run before engine selection")
+
+	session.writeString(t, "\r")
+
+	require.NoError(t, session.waitForText(enginePrompt, 30*time.Second), "Expected engine selection prompt")
+
+	afterEngineOutput := session.readAll()
+	assert.Contains(t, afterEngineOutput, earlyPrompt, "Expected pre-install bootstrap prompt in final session output")
+	assert.Contains(t, afterEngineOutput, enginePrompt, "Expected engine selection prompt after pre-install bootstrap setup")
+	assert.NotContains(t, afterEngineOutput, lateMessage, "Post-install bootstrap steps should not run before installation")
+
+	earlyPromptIndex := strings.Index(afterEngineOutput, earlyPrompt)
+	enginePromptIndex := strings.Index(afterEngineOutput, enginePrompt)
+	require.NotEqual(t, -1, earlyPromptIndex, "Expected to find pre-install bootstrap prompt in session output")
+	require.NotEqual(t, -1, enginePromptIndex, "Expected to find engine selection prompt in session output")
+	assert.Less(t, earlyPromptIndex, enginePromptIndex, "Pre-install bootstrap prompt should appear before engine selection")
+
+	session.interrupt(t)
+}