Skip to content

fix(deploy): fail fast on non-TTY confirmation; correct login and version guidance - #87

Merged
kylegani merged 1 commit into
kyle/phase0-cli-guidancefrom
kyle/phase0-agent-safe-cli
Aug 12, 2026
Merged

fix(deploy): fail fast on non-TTY confirmation; correct login and version guidance#87
kylegani merged 1 commit into
kyle/phase0-cli-guidancefrom
kyle/phase0-agent-safe-cli

Conversation

@kylegani

@kylegani kylegani commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Context

Part of Phase 0 of the Agentic Cerebrium initiative: making Cerebrium reliably drivable by coding agents. Agents and CI drive this CLI headlessly (piped stdin, no TTY). Today the deploy confirmation silently auto-confirms on stdin EOF and hangs forever on an open pipe, and two guidance messages point at an env var and a command that do not exist. Bad guidance is amplified with agents: they follow error messages literally.

This PR is layer 2 of a 2-PR chain (merge in order; the upper PR auto-retargets to main when the lower merges):

  1. fix: correct service-account env var and version-check guidance #88 guidance fixes (env var name, version-check command)
  2. fix(deploy): fail fast on non-TTY confirmation; correct login and version guidance #87 deploy confirmation hardening

(Chained bases rather than a native GitHub stack: the stacked-PR public preview is not enabled on this repository.)

Phase 0 CLI track, layer 2: deploy confirmation hardening. Based on kyle/phase0-cli-guidance (#88), which carries the login/version guidance fixes that were previously part of this PR.

Changes

  • Before: fmt.Scanln meant EOF on stdin silently auto-confirmed a deploy, and an open non-TTY pipe blocked forever. An agent or CI piping into cerebrium deploy could ship to prod by accident.
  • Now: StdinIsTTY added to DisplayConfig (mattn/go-isatty, already a dependency, same pattern as the existing stdout detection). When stdin is not a TTY and confirmation was not disabled, deploy fails fast before any side effects: unable to prompt for confirmation: stdin is not a TTY. Re-run with -y/--yes to skip confirmation (clig.dev / Azure CLI convention).
  • The guard is keyed off stdin TTY-ness only, NOT SimpleOutput: --no-color on a real terminal still prompts.
  • fmt.Scanln replaced by readConfirmationResponse(io.Reader): EOF or read error is a decline, never consent. Plain Enter still defaults to yes on a live TTY.
  • --yes added as a visible alias of --disable-confirmation (-y shorthand preserved), resolved through a testable confirmationDisabled(flags) helper.
  • No agent-env detection (CLAUDECODE etc.): deliberately deferred to Phase 1; detection must never auto-confirm.

How to test this layer

  1. Unit tests: go test ./internal/commands/... ./internal/ui/commands/...
    • Test_validateConfirmationPrompt: TTY/non-TTY x confirmation required/disabled matrix, asserts the error names -y/--yes.
    • Test_confirmationDisabled: 5 flag combos including -y and --yes.
    • TestReadConfirmationResponse: 10 cases, EOF and read-error decline paths included.
      Full suite go test ./... passes.
  2. Manual, non-TTY (the dangerous paths, both previously auto-confirmed or hung):
    • go build -o /tmp/cerebrium-test ./cmd/cerebrium && echo | /tmp/cerebrium-test deploy fails fast, exit 1, error names -y/--yes.
    • /tmp/cerebrium-test deploy < /dev/null same fail-fast (previously auto-confirmed).
    • /tmp/cerebrium-test deploy -y and --yes and --disable-confirmation all skip the prompt.
  3. Manual, TTY: cerebrium deploy in a real terminal still prompts, including with --no-color; Ctrl-D at the prompt aborts (previously confirmed); deploy --help shows both --yes and --disable-confirmation.

Notes

  • golangci-lint v2.5.0 reports 4 findings, all verified pre-existing on origin/main via stash round-trip; zero new.
  • go mod tidy promoted spf13/pflag to a direct dependency (now imported by the deploy command).
  • Open question recorded in plan.md: whether other interactive (bubbletea) prompts should get the same non-TTY guard; only deploy used raw Scanln.

Stack position: 2 of 2 (kyle/phase0-cli-guidance -> kyle/phase0-agent-safe-cli). Merge #88 first, then retarget this PR to main (or let GitHub retarget on branch deletion).

🤖 Generated with Claude Code

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kylegani
kylegani force-pushed the kyle/phase0-agent-safe-cli branch from cb5f401 to 28db3d1 Compare August 4, 2026 09:53
@kylegani
kylegani changed the base branch from main to kyle/phase0-cli-guidance August 4, 2026 09:54
@kylegani
kylegani marked this pull request as ready for review August 4, 2026 12:49
@kylegani
kylegani merged commit f4c4247 into kyle/phase0-cli-guidance Aug 12, 2026
4 checks passed
@kylegani
kylegani deleted the kyle/phase0-agent-safe-cli branch August 12, 2026 11:16
kylegani added a commit that referenced this pull request Aug 24, 2026
**Stack position: 2 of 2.** Stacked on #102 — this diff is against it,
not `main`.

## Problem

Every request already carries `X-Source: cli` and `X-CLI-Version`, but
nothing says *what invoked the CLI*. An agent-driven deploy and an
engineer at a terminal are indistinguishable, so deploy-side agent
metrics can only split on the calling principal: currently around 32% of
deploying principals are automation against around 70% of build volume,
and that gap is one undifferentiated blob of service accounts belonging
to CI, to coding agents, and to internal tooling.

## Change

`internal/clientenv` classifies the invoking environment into one of
three shapes, emitted as `X-Client-Env`:

- `agent:<name>` for a detected agent
- `ci` for a CI system (`CI`, plus 13 vendor variables)
- `interactive` otherwise

Wired onto the JSON API client, the multipart `RunApp` path and both
device-auth endpoints. Computed once per process.

The agent matrix mirrors
[vercel/detect-agent](https://github.com/vercel/detect-agent)
`agents.json` (schema version 1): 20 agents, its identifiers verbatim
(including its mixed `claude_code` and `cursor-cli` styles) and its
array order, since order decides which of two simultaneous markers wins.
`TestAgentDetectorsMatchRegistryOrder` pins both, so a drifting sync
fails instead of silently reclassifying traffic. The registry promotes
`AI_AGENT` as the cross-vendor self-declaration variable, which we
honour.

Two deliberate narrowings, both flagged in the code:

- **`replit` is gated on the absence of a TTY.** `REPL_ID` is set for
every process on Replit including a human in the editor, so bare
`REPL_ID` evidences the host, not an agent. This is the registry's own
stated reasoning for `kiro` ("set by both the IDE terminal and the CLI
agent, so gate on `no_tty` to avoid misdetecting a human at the
integrated terminal") applied to the variable with the same problem.
Following the registry literally here would have put humans in the agent
bucket, which is the exact overcount this header exists to avoid.
- **`AI_AGENT` is evaluated first**, not as a fallback, so an operator
wrapping a known agent can name their own harness and have that win.

Detection never influences behaviour: no output default, no prompt
suppression, nothing branches on it. That is why the non-TTY deploy
guard from #87 keys off stdin rather than agent detection. `Detect`
takes its environment, filesystem and TTY probes as parameters, so the
matrix is tested without touching the real environment.

`AI_AGENT` values are caller-supplied, so they are lowercased,
restricted to `[a-z0-9._-]` and capped at 64 characters. A value that
sanitizes to nothing becomes `agent:unknown` rather than degrading to
`interactive`, keeping a declared-but-unparseable agent in the agent
bucket.

## Not in scope

The header is only *stored* once dashboard-backend #4132 lands; until
then it is accepted and discarded. `X-` prefixes are discouraged for new
headers by RFC 6648, but this joins an existing `X-Source` /
`X-CLI-Version` / `X-Client-OS` family and consistency won.

## Test

`go test ./internal/clientenv/... ./internal/api/...
./internal/auth/...` green: 58 detection cases (one per registry
variable, both TTY branches for the two host-gated agents, the three
ordering rules, the `AI_AGENT` sanitizer and the CI matrix), plus
header-presence tests on the API client and both OAuth endpoints.

`gofmt` clean. `golangci-lint run ./internal/clientenv/...` reports 0
issues; on the other touched packages the single finding (`nilerr` in
`internal/auth/docker.go`) is pre-existing, verified by re-running
against a stash. Repo-wide `make lint` and `go build ./...` fail on
vendored `cortex/server/node_modules` AWS CDK templates, unrelated and
also failing on `main`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <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.

2 participants