Skip to content

fix: correct service-account env var and version-check guidance - #88

Merged
kylegani merged 3 commits into
mainfrom
kyle/phase0-cli-guidance
Aug 12, 2026
Merged

fix: correct service-account env var and version-check guidance#88
kylegani merged 3 commits into
mainfrom
kyle/phase0-cli-guidance

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 1 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 1: guidance fixes only. Splits the guidance corrections out of the original single-commit PR (#87) so they can merge independently of the deploy confirmation hardening.

Changes

  • internal/commands/login.go: the login error named CEREBRIUM_SERVICE_ACCOUNT, an env var nothing reads. It now names the real CEREBRIUM_SERVICE_ACCOUNT_TOKEN (read at pkg/config/config.go:267,376) and mentions the --service-account-token flag.
  • SECURITY.md: same env var correction.
  • internal/version/check.go: the update banner suggested the nonexistent cerebrium config --version-check=false; it now suggests the real cerebrium config set skip-version-check true (verified against the config set implementation, including the env-prefix handling).

Why

Agents and users follow error text verbatim. Both messages pointed at invocations that do not exist, so the suggested remediation silently failed.

How to test this layer

  1. go build ./... && go test ./internal/commands/...
  2. Trigger the login error without credentials and confirm it names CEREBRIUM_SERVICE_ACCOUNT_TOKEN and --service-account-token.
  3. Force the update banner (old version) and run the suggested cerebrium config set skip-version-check true verbatim; confirm it is accepted.

Stack position: 1 of 2 (kyle/phase0-cli-guidance -> kyle/phase0-agent-safe-cli). Layer 2 is the deploy confirmation hardening in #87, based on this branch.

🤖 Generated with Claude Code

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kylegani
kylegani marked this pull request as ready for review August 4, 2026 12:50
jonoirwinrsa and others added 2 commits August 7, 2026 14:39
Follow-ups in the same lines this branch already touches:

- login.go: the non-TTY error named --service-account-token inside
  login's own error, where the flag is never read (runLogin returns
  before it). Point at the command the user actually wants to run.
- SECURITY.md: auth/service_account.go does not exist; precedence is
  in internal/api/client.go. "Not persisted to disk" was wrong for
  tokens stored via save-auth-config, which writes them to
  ~/.cerebrium/config.yaml.
- SECURITY.md: 0644 is world-readable, not "user read/write only".
  Mode is unchanged; only the description was wrong.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sion guidance (#87)

## 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. #88 guidance fixes (env var name, version-check command)
2. #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](https://claude.com/claude-code)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@kylegani
kylegani added this pull request to the merge queue Aug 12, 2026
Merged via the queue into main with commit 73dc70b Aug 12, 2026
4 checks passed
@kylegani
kylegani deleted the kyle/phase0-cli-guidance branch August 12, 2026 11:17
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.

4 participants