Skip to content

Backport #916 to v26-branch: Don't require a TTY for non-interactive composer server commands#925

Open
mikelittle wants to merge 1 commit into
v26-branchfrom
backport-916-to-v26
Open

Backport #916 to v26-branch: Don't require a TTY for non-interactive composer server commands#925
mikelittle wants to merge 1 commit into
v26-branchfrom
backport-916-to-v26

Conversation

@mikelittle
Copy link
Copy Markdown
Contributor

Manual backport of #916 to v26-branch. The auto-backport bot couldn't apply this because v26-branch has a case null: arm in db() that was restructured to default: on master.

Why backport

On GitHub Actions there is no controlling TTY, so the hardcoded docker exec -it … -db in composer server db exec errors with the input device is not a TTY before the SQL runs. The product-dev nightly CI uses this path indirectly via composer dev-tools codecept's test-DB bootstrap (DROP DATABASE … CREATE DATABASE test;…), so the test/test2 DBs never get created and codecept then fails with [1044] Access denied for user 'wordpress'@'%' to database 'test' for every module suite. Travis didn't hit this because its runner supplied enough of a pseudo-TTY for -it to work.

What changed

Same shape as #916 against master, adapted for v26's slightly older db() body:

  • composer server db exec / s3 import-uploads / s3 exec drop -t and only pass -i so they run without a controlling terminal.
  • composer server db (interactive REPL on this branch is case null:), shell, and the logs service picker keep -it and gate on a new require_tty() that prints a clear error and a non-interactive alternative when no terminal is attached, instead of hanging or crashing on Docker's opaque error.
  • Adds is_tty() / require_tty() helpers.

The db case on this branch differs from master:

  • master collapsed case null (REPL) and default (unknown subcommand) into a single default arm with the non-interactive -e/--execute/-B/--batch detection inside.
  • v26 still has separate case null and default arms, and the REPL path doesn't take extra options, so the -e/-B detection isn't needed here — the REPL is always interactive on this branch. Kept that simpler shape; just guarded case null with require_tty().

Test plan

  • Trigger product-dev nightly against v26 after merge and confirm Altis 26 jobs no longer hit "Access denied for user 'wordpress'@'%' to database 'test'".
  • Manual: composer server db exec -- "SELECT 1; " works without a TTY (e.g. piped through <&-).
  • Manual: composer server db with a TTY still opens the interactive REPL.

🤖 Generated with Claude Code

`composer server db exec`, `s3 import-uploads`, and `s3 exec` always
hardcoded `docker exec/run -it`, which fails with "cannot attach stdin
to a TTY-enabled container" when the caller has no controlling
terminal — CI, cron, scripts, AI agents, ssh without `-t`,
docker-in-docker, and `composer dev-tools codecept`'s test-DB setup,
which delegates SQL execution to `composer server db exec`.

Drop `-t` from the non-interactive paths so they run anywhere stdin
is available. The interactive paths (`shell`, `db` REPL, `destroy`
confirmation, `logs` service picker) keep `-it` and now gate on
`require_tty()`, which prints a clear error and a non-interactive
alternative instead of hanging on a prompt or crashing on Docker's
opaque error.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant