fix(docker): do not give commands a PTY without a terminal

.castor/context.php

@@ -57,9 +57,15 @@ function create_default_context(): Context
         $data['project_name'] .= '-wt-' . $worktreeName;
     }
 
+    // Only when someone can answer the prompts of the commands: stdin is a terminal, outside of
+    // a CI (as for Castor). Otherwise (CI, pipes, AI agents...), a PTY would give them a terminal
+    // nobody types in, and a prompt (composer, bin/console...) would wait forever. Unlike
+    // Context::supportsInteraction(), stdout may be piped (e.g. `castor init | tee init.log`).
+    $canAnswerPrompts = false === getenv('CI') && stream_isatty(STDIN);
+
     return new Context(
         $data,
-        pty: Process::isPtySupported(),
+        pty: Process::isPtySupported() && $canAnswerPrompts,
         environment: [
             'BUILDKIT_PROGRESS' => 'plain',
         ]

.castor/docker.php

@@ -566,7 +566,9 @@ function docker_compose_run(
 
     if (0 === \count($params)) {
         $params = ['bash'];
-        $c = $c->toInteractive();
+        // A CTRL+C belongs to the shell, and must not stop the task: even when the shell exits with
+        // the code of its last command, 130 after a CTRL+C at the prompt
+        $c = $c->toInteractive()->withTrappedSignals();
     } else {
         $params = array_map(escapeshellarg(...), $params);
     }
@@ -595,7 +597,8 @@ function docker_compose_exec(
 
     if (0 === \count($params)) {
         $params = ['bash'];
-        $context = $context->toInteractive();
+        // See docker_compose_run()
+        $context = $context->toInteractive()->withTrappedSignals();
     } else {
         $params = array_map(escapeshellarg(...), $params);
     }
@@ -673,7 +676,6 @@ function push(
 
     // Additional build contexts may live outside of the compose files directory (e.g. the
     // repository root): grant bake the read access explicitly, or it asks for it interactively
-    // (and waits forever in the CI, castor running it with a pty)
     $command = ['docker', 'buildx', 'bake', '--allow=fs.read=' . variable('root_dir')];
 
     foreach ($c['docker_compose_files'] as $file) {

.castor/init.php

@@ -21,7 +21,7 @@ function init(): void
     }
 
     fs()->remove([
-        '.castor/docker-push-test.php',
+        '.castor/test',
         'README.md',
         'CHANGELOG.md',
         'CONTRIBUTING.md',

.castor/test/TerminalTestCase.php

@@ -0,0 +1,20 @@
+<?php
+
+namespace test;
+
+/**
+ * A case of "test:terminal" (see terminal.php).
+ */
+final readonly class TerminalTestCase
+{
+    public function __construct(
+        public string $situation,
+        // As typed in a terminal
+        public string $commandline,
+        // What the terminal then shows
+        public string $expected,
+        // What is typed while the command runs
+        public string $typed = '',
+    ) {
+    }
+}

.castor/docker-push-test.php → .castor/test/push.php

@@ -5,7 +5,7 @@
 // drop it from a project generated from this template, just delete this file
 // (and, if it was wired there, the corresponding step in .github/workflows/ci.yml).
 
-namespace docker;
+namespace test;
 
 use Castor\Attribute\AsTask;
 
@@ -15,9 +15,10 @@
 use function Castor\io;
 use function Castor\run;
 use function Castor\wait_for_http_status;
+use function docker\get_services;
 
-#[AsTask(description: 'Tests "docker:push" against a throwaway local registry', namespace: 'docker', name: 'test-push')]
-function test_push(): void
+#[AsTask(description: 'Tests "docker:push" against a throwaway local registry')]
+function push(): void
 {
     io()->title('Testing docker:push against a throwaway registry');
 

.castor/test/terminal.php

@@ -0,0 +1,198 @@
+<?php
+
+// This file only tests how the commands run by docker-starter behave with and without a
+// terminal (TTY, PTY, stdin). It is entirely self-contained: to drop it from a project
+// generated from this template, just delete this file (and, if it was wired there, the
+// corresponding step in .github/workflows/ci.yml).
+
+namespace test;
+
+use Castor\Attribute\AsTask;
+use Symfony\Component\Console\Output\OutputInterface;
+
+use function Castor\capture;
+use function Castor\context;
+use function Castor\io;
+use function Castor\run;
+use function docker\docker_compose_exec;
+use function docker\docker_compose_run;
+
+// What a command gets: a terminal on stdin (to answer its prompts), or else the first line read
+// on it, which must not wait forever; then whether it colors its output, as Symfony Console
+// decides it in short (StreamOutput::hasColorSupport()): stdout is a terminal of a known type
+const TERMINAL_PROBE = 'if [ -t 0 ]; then stdin=terminal; else stdin="read \"$(head -n 1)\""; fi; '
+    . 'colors=no; [ -t 1 ] && case "$TERM" in screen*|xterm*|vt100*|vt220*|putty*|rxvt*|ansi*|cygwin*|linux*|*-256*) colors=yes; esac; '
+    . 'echo "stdin: $stdin, colors: $colors"';
+
+/**
+ * Needs the stack to be started (for docker_compose_exec()), and the `script` command of
+ * util-linux to emulate a terminal.
+ */
+#[AsTask(description: 'Tests how commands behave with and without a terminal')]
+function terminal(): void
+{
+    io()->title('Testing how commands behave with and without a terminal');
+
+    foreach (get_terminal_test_cases() as $section => $cases) {
+        io()->section($section);
+
+        foreach ($cases as $case) {
+            $output = type_in_terminal($case->commandline, $case->typed);
+
+            if ($case->expected !== $output) {
+                throw new \RuntimeException(\sprintf('%s: expected "%s", got "%s".', $case->situation, $case->expected, $output));
+            }
+
+            io()->comment("{$case->situation}: OK");
+        }
+    }
+
+    io()->success('Commands behave as expected with and without a terminal.');
+}
+
+/**
+ * What "test:terminal" can not judge: how it looks, and how it feels to type in it.
+ */
+#[AsTask(description: 'Lists what to check by hand in a terminal (Linux)', namespace: 'test:terminal', name: 'manual')]
+function terminal_manual(): void
+{
+    io()->title('Checking by hand how commands behave with and without a terminal');
+
+    io()->text([
+        'Run each command in your terminal, the stack being started and the QA tools installed,',
+        'and check what follows it. Castor gives the command either your terminal as is (TTY),',
+        'or a terminal of its own, which relays your terminal (PTY).',
+    ]);
+
+    $readline = <<<'SH'
+        castor builder -- php -r 'echo "Hello ", readline("Name? "), "\n";'
+        SH;
+
+    $checks = [
+        'In your terminal, given as is (TTY)' => [
+            ['castor builder', 'A bash shell: "composer <Tab>" completes, the arrows and Ctrl+R go through the history, Ctrl+C stops a "sleep 30" but not the shell, Ctrl+D exits without any warning, even right after a Ctrl+C'],
+            ['castor builder -- top', 'Full screen, at the size of the window, which it follows when resized; "q" quits'],
+            [$readline, 'Asks your name, then greets you'],
+            ['castor pg', 'A psql prompt, with its history; "\q" quits'],
+        ],
+        'In your terminal, relayed (PTY): the tasks running a command with docker_compose_run()' => [
+            ['castor qa:cs --dry-run', 'Colors, and a progress bar redrawn in place'],
+            ['castor qa:phpstan', 'Its "[OK] No errors" block is 80 columns wide, even in a wider window: the PTY has no size (a known limit)'],
+            ['castor qa:phpstan', 'Ctrl+C stops it at once, and "docker ps" then lists no builder container'],
+        ],
+        'With pipes' => [
+            ['castor qa:cs --dry-run | cat', 'Colors still: someone can answer the prompts'],
+            ["echo Alice | {$readline}", 'Greets Alice, without asking'],
+        ],
+        'As in a CI: no terminal, not even a controlling one' => [
+            ['CI=true setsid -w castor qa:phpstan < /dev/null 2>&1 | cat', 'Ends, without colors nor progress bar redrawn'],
+            ["CI=true setsid -w {$readline} < /dev/null 2>&1 | cat", 'Does not wait for an answer: "Hello " right away'],
+        ],
+        'As an AI agent: the same, without CI (or ask yours to run them)' => [
+            ['setsid -w castor qa:phpstan < /dev/null 2>&1 | cat', 'Ends, without colors nor progress bar redrawn'],
+            ["setsid -w {$readline} < /dev/null 2>&1 | cat", 'Does not wait for an answer: "Hello " right away'],
+        ],
+    ];
+
+    foreach ($checks as $section => $sectionChecks) {
+        io()->section($section);
+
+        foreach ($sectionChecks as [$commandline, $check]) {
+            io()->writeln('$ ' . $commandline, OutputInterface::OUTPUT_RAW);
+            io()->writeln('  ' . $check, OutputInterface::OUTPUT_RAW);
+            io()->newLine();
+        }
+    }
+}
+
+#[AsTask(description: 'Runs the probe of "test:terminal"', namespace: 'test:terminal', name: 'probe')]
+function terminal_probe(string $via): void
+{
+    $command = ['sh', '-c', TERMINAL_PROBE];
+
+    match ($via) {
+        'run' => run($command),
+        'docker_compose_run' => docker_compose_run($command),
+        'docker_compose_run_with_pty' => docker_compose_run($command, c: context()->withPty()),
+        'docker_compose_exec' => docker_compose_exec($command, service: 'frontend'),
+        default => throw new \InvalidArgumentException(\sprintf('Unknown way to run the probe: "%s".', $via)),
+    };
+}
+
+/**
+ * @return array<string, list<TerminalTestCase>> Indexed by section
+ */
+function get_terminal_test_cases(): array
+{
+    $terminal = 'stdin: terminal, colors: yes';
+    $nothingRead = 'stdin: read "", colors: no';
+    $helloRead = 'stdin: read "hello", colors: no';
+
+    $cases = [];
+
+    // Without a PTY, Castor gives its stdin to no command: only builder() forwards it (see below)
+    foreach (['run', 'docker_compose_run', 'docker_compose_exec'] as $via) {
+        $probe = "castor test:terminal:probe {$via}";
+
+        $cases["{$via}()"] = [
+            new TerminalTestCase('In a terminal', $probe, $terminal),
+            // Someone can still answer the prompts (e.g. `castor init | tee init.log`)
+            new TerminalTestCase('In a terminal, stdout piped', "{$probe} | cat", $terminal),
+            // Nobody types in the terminal of a CI
+            new TerminalTestCase('In a terminal, in a CI', "CI=true {$probe}", $nothingRead),
+            new TerminalTestCase('Without a terminal (AI agent)', "{$probe} < /dev/null | cat", $nothingRead),
+            new TerminalTestCase('Without a terminal, in a CI', "CI=true {$probe} < /dev/null | cat", $nothingRead),
+            new TerminalTestCase('Without a terminal, stdin piped', "echo hello | {$probe} | cat", $nothingRead),
+        ];
+    }
+
+    $cases['A PTY asked explicitly'] = [
+        new TerminalTestCase('Without a terminal', 'castor test:terminal:probe docker_compose_run_with_pty < /dev/null | cat', $terminal),
+    ];
+
+    // It mirrors the terminal, the pipes and the redirections of Castor, as when the command is
+    // run by hand
+    $builder = 'castor builder -- sh -c ' . escapeshellarg(TERMINAL_PROBE);
+
+    $cases['castor builder'] = [
+        new TerminalTestCase('In a terminal', $builder, $terminal),
+        // The terminal echoes what is typed
+        new TerminalTestCase('In a terminal, stdout piped', "{$builder} | cat", "hello\n{$helloRead}", typed: "hello\n"),
+        new TerminalTestCase('In a terminal, stdin piped', "echo hello | {$builder}", $helloRead),
+        new TerminalTestCase('Without a terminal', "{$builder} < /dev/null | cat", $nothingRead),
+        new TerminalTestCase('Without a terminal, stdin piped', "echo hello | {$builder} | cat", $helloRead),
+    ];
+
+    return $cases;
+}
+
+/**
+ * Types the command line in a terminal emulated by script, and returns what it shows.
+ */
+function type_in_terminal(string $commandline, string $typed): string
+{
+    // Then nobody types anymore, but the terminal stays open: a command reading it waits, as
+    // in a real one, instead of getting an end of file
+    $input = (static function () use ($typed): \Generator {
+        yield $typed;
+
+        while (true) {
+            yield '';
+        }
+    })();
+
+    $output = capture(['script', '--quiet', '--return', '--command', $commandline, '/dev/null'], context: context()
+        ->withInput($input)
+        // A command waiting for an answer must fail the test, not block it
+        ->withTimeout(60)
+        ->withEnvironment([
+            // The command lines set it themselves (it is set in GitHub Actions)
+            'CI' => false,
+            // As a terminal emulator does (it is not set in GitHub Actions)
+            'TERM' => 'xterm-256color',
+        ])
+    );
+
+    // The terminals end the lines with "\r\n" (even "\r\r\n", through two of them)
+    return str_replace("\r", '', $output);
+}

.github/workflows/ci.yml

@@ -126,23 +126,10 @@ jobs:
 
             # >>> docker-starter only
             - name: "Test docker:push"
-              run: "castor docker:test-push"
+              run: "castor test:push"
 
-            - name: "Test the TTY of builder commands"
-              run: |
-                  set -e
-                  set -o pipefail
-
-                  # In a terminal (emulated by script), the command gets a TTY (for colors).
-                  # CI is unset, as Castor never considers it as interactive
-                  env -u CI script -qec "castor builder -- php -r 'exit(stream_isatty(STDOUT) ? 0 : 1);'" /dev/null
-                  # Even in a terminal, a pipe on stdout or stdin reaches the container
-                  env -u CI script -qec "castor builder -- php -r 'echo stream_isatty(STDOUT) ? \"tty\" : \"pipe\";' | cat" /dev/null | grep -w pipe
-                  env -u CI script -qec "echo hello | castor builder -- cat" /dev/null | grep -w hello
-
-                  # Without a terminal
-                  castor builder -- php -r 'exit(stream_isatty(STDOUT) ? 1 : 0);' | cat
-                  echo hello | castor builder -- cat | grep -x hello
+            - name: "Test commands with and without a terminal"
+              run: "castor test:terminal"
 
             - name: "Test HTTP server"
               run: |

castor.php

@@ -18,7 +18,7 @@
 
 defined('CASTOR_USE_CHDIR') || define('CASTOR_USE_CHDIR', true);
 
-guard_min_version('1.8.0');
+guard_min_version('1.9.0');
 
 import(__DIR__ . '/.castor');