diff --git a/cmd/harnesscli/go_code_script_test.go b/cmd/harnesscli/go_code_script_test.go index 04cc38a7..6d11a934 100644 --- a/cmd/harnesscli/go_code_script_test.go +++ b/cmd/harnesscli/go_code_script_test.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "errors" "fmt" "os" @@ -202,3 +203,85 @@ func TestGoCodeScriptStartsHarnessdOnLoopback(t *testing.T) { got, want, out) } } + +// TestGoCodeScriptEmitsNoAnsiWhenNotATty guards the color contract of issue +// #1413: styling is an enhancement for interactive terminals only. +// +// exec.Command gives the script no controlling terminal, so this is the same +// condition as `go-code runs | cat`. It cannot be red before the feature exists +// — it is a regression guard against a later change that colors unconditionally +// and corrupts piped or captured output. +func TestGoCodeScriptEmitsNoAnsiWhenNotATty(t *testing.T) { + scriptPath, err := filepath.Abs(filepath.Join("..", "..", "scripts", "go-code.sh")) + if err != nil { + t.Fatalf("resolve go-code script path: %v", err) + } + + binDir := t.TempDir() + writeExecutable(t, filepath.Join(binDir, "curl"), "#!/usr/bin/env bash\nexit 0\n") + writeExecutable(t, filepath.Join(binDir, "harnessd"), "#!/usr/bin/env bash\nexit 0\n") + writeExecutable(t, filepath.Join(binDir, "harnesscli"), "#!/usr/bin/env bash\nexit 0\n") + + for _, tc := range []struct { + name string + env []string + }{ + {name: "no tty", env: nil}, + {name: "NO_COLOR set", env: []string{"NO_COLOR=1"}}, + {name: "dumb terminal", env: []string{"TERM=dumb"}}, + } { + t.Run(tc.name, func(t *testing.T) { + cmd := exec.Command("bash", scriptPath, "runs") + cmd.Env = append(os.Environ(), + "PATH="+binDir+string(os.PathListSeparator)+os.Getenv("PATH"), + "HARNESS_ADDR=:19383", + ) + cmd.Env = append(cmd.Env, tc.env...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("go-code runs failed: %v\n%s", err, out) + } + if bytes.Contains(out, []byte("\x1b[")) { + t.Fatalf("output contains ANSI escape sequences with no terminal attached:\n%q", out) + } + }) + } +} + +// TestGoCodeScriptSurfacesHarnessdLogOnStartupFailure pins the other half of +// issue #1413: the wrapper captures harnessd's output to a log file so a clean +// start is not buried in boot noise (and so no daemon line can scribble into the +// TUI after handoff) — but a failed start must still show the operator why the +// daemon died. Capturing without surfacing would trade noise for silence. +func TestGoCodeScriptSurfacesHarnessdLogOnStartupFailure(t *testing.T) { + scriptPath, err := filepath.Abs(filepath.Join("..", "..", "scripts", "go-code.sh")) + if err != nil { + t.Fatalf("resolve go-code script path: %v", err) + } + + binDir := t.TempDir() + // curl: the health check never succeeds, so the wrapper must give up. + writeExecutable(t, filepath.Join(binDir, "curl"), "#!/usr/bin/env bash\nexit 1\n") + // harnessd: emit a boot line and a fatal line, then die — the shape of the + // real bind-guard and workspace-lock failures. + writeExecutable(t, filepath.Join(binDir, "harnessd"), "#!/usr/bin/env bash\necho 'loaded model catalog with 15 providers'\necho 'fatal: refusing to start: sentinel failure reason'\nexit 1\n") + writeExecutable(t, filepath.Join(binDir, "harnesscli"), "#!/usr/bin/env bash\nexit 0\n") + + cmd := exec.Command("bash", scriptPath, "runs") + cmd.Env = append(os.Environ(), + "PATH="+binDir+string(os.PathListSeparator)+os.Getenv("PATH"), + "HARNESS_ADDR=:19384", + ) + out, err := cmd.CombinedOutput() + if err == nil { + t.Fatalf("expected go-code to fail when harnessd never becomes healthy\n%s", out) + } + + if !bytes.Contains(out, []byte("fatal: refusing to start: sentinel failure reason")) { + t.Fatalf("startup failure did not surface harnessd's own reason for dying;\n"+ + "the daemon log was captured but never shown, which hides the cause:\n%s", out) + } + if !bytes.Contains(out, []byte("harnessd.")) || !bytes.Contains(out, []byte(".log")) { + t.Fatalf("startup failure did not report the harnessd log path:\n%s", out) + } +} diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index c8193b99..02f528cd 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -1,5 +1,42 @@ # Engineering Log +## 2026-09-08 — Issue #1413 readable go-code startup output + +- Symptom: `go-code` printed 13 lines of undifferentiated output on a normal + startup, mixing three different voices with no visual separation — the + wrapper's own `[go-code] ...` status lines, harnessd's own boot log (since + the daemon inherited the wrapper's stdout), and (on failure) a fatal error — + so the one actionable line was buried in the noise. Because the daemon + inherited the terminal, a stray daemon log line could also render into the + TUI after handoff, not just during startup. +- Cause: `scripts/go-code.sh`'s three one-line output helpers (`info`, `warn`, + `die`) gave every line the same undifferentiated `[go-code] ...` prefix with + no severity distinction, and `start_server()` let harnessd inherit the + wrapper's stdout/stderr instead of capturing them. +- Fix: color detection now runs once at startup into `COLOR_STDOUT` / + `COLOR_STDERR` (disabled under `NO_COLOR`, `TERM=dumb`, or when the relevant + stream isn't a terminal), feeding a `style` helper; `info` is cyan-prefixed, + `warn` yellow, `die` red, with the literal words `WARN:`/`ERROR:` always kept + in the text so severity never depends on color alone. A wrapper-started + harnessd now writes to `${TMPDIR:-/tmp}/harnessd..log` (created with + `umask 077`) instead of the terminal. On success the wrapper prints + `server ready at ` plus a `log: ` line, and the daemon boot log no + longer appears at all. On failure, `die` calls `show_harnessd_log`, which + prints a `harnessd said:` block with the last 20 log lines indented four + spaces (lines matching `fatal:`, `panic:`, or `refusing to start` in bold + red, everything else dimmed) followed by `full log: `. +- Gotcha (the durable lesson here): the first implementation tested `[[ -t 1 ]]` + lazily, inside the `style` helper itself. But `style` is invoked from + command substitution (`$(style ...)`), where `$( )` only redirects stdout — + so inside that substitution, fd 1 is a pipe, not the terminal, and stdout + was never colored even on a real tty, while stderr colored correctly. Color + detection has to happen once at startup, before any command substitution + runs. This was caught by looking at real rendered pty output, not by the + test suite — the tests (`TestGoCodeScriptEmitsNoAnsiWhenNotATty`, + `TestGoCodeScriptSurfacesHarnessdLogOnStartupFailure`, both in + `cmd/harnesscli/go_code_script_test.go`) check the no-color and log-surfacing + paths but don't exercise a real tty, so they would have passed either way. + ## 2026-09-08 — Issue #1411 go-code wrapper bound harnessd beyond loopback - Symptom: plain `go-code` (no flags) died on a clean machine with no API key diff --git a/scripts/go-code.sh b/scripts/go-code.sh index f4e3fb43..23db07ff 100755 --- a/scripts/go-code.sh +++ b/scripts/go-code.sh @@ -62,9 +62,59 @@ Description: EOF } -info() { printf '[go-code] %s\n' "$*"; } -warn() { printf '[go-code] WARN: %s\n' "$*" >&2; } -die() { printf '[go-code] ERROR: %s\n' "$*" >&2; exit 1; } +# Color is an enhancement, never the only signal: the WARN:/ERROR: words stay in +# the text, so severity survives a monochrome terminal, a pipe, a captured log, +# and colorblind readers. Only the 8 standard ANSI colors are used, so each +# terminal applies its own theme and nothing turns invisible on a light +# background. Issue #1413. +# +# Support is detected once, here, and must not be tested lazily inside style(): +# style() is called from command substitution, where stdout is a pipe rather +# than the terminal, so an `-t 1` check there is always false and stdout would +# never be colored. stdout and stderr are tracked separately because either can +# be redirected on its own. +COLOR_STDOUT=0 +COLOR_STDERR=0 +if [[ -z "${NO_COLOR:-}" && "${TERM:-}" != "dumb" ]]; then + [[ -t 1 ]] && COLOR_STDOUT=1 + [[ -t 2 ]] && COLOR_STDERR=1 +fi + +# style — style text only when that stream is a terminal. +style() { + local enabled="$COLOR_STDOUT" + [[ "$1" == "2" ]] && enabled="$COLOR_STDERR" + if [[ "$enabled" == "1" ]]; then + printf '\033[%sm%s\033[0m' "$2" "$3" + else + printf '%s' "$3" + fi +} + +info() { printf '%s %s\n' "$(style 1 '36' '[go-code]')" "$*"; } +warn() { printf '%s %s %s\n' "$(style 2 '33' '[go-code]')" "$(style 2 '1;33' 'WARN:')" "$*" >&2; } +die() { printf '%s %s %s\n' "$(style 2 '31' '[go-code]')" "$(style 2 '1;31' 'ERROR:')" "$*" >&2; show_harnessd_log; exit 1; } + +# show_harnessd_log prints the captured daemon log when a wrapper-started +# harnessd failed. The daemon's stdout is redirected to a file so a healthy +# start is not buried in boot noise and no log line can scribble into the TUI +# after handoff — which means a failure has to bring that output back, or the +# operator is left with no reason at all. Lines are colored per logical line, so +# a long fatal message stays emphasized across the terminal's soft wrap. +show_harnessd_log() { + [[ -n "${HARNESSD_LOG:-}" && -s "${HARNESSD_LOG:-}" ]] || return 0 + printf '\n %s\n' "$(style 2 '1' 'harnessd said:')" >&2 + local line + while IFS= read -r line; do + case "$line" in + *fatal:*|*panic:*|*"refusing to start"*) + printf ' %s\n' "$(style 2 '1;31' "$line")" >&2 ;; + *) + printf ' %s\n' "$(style 2 '2' "$line")" >&2 ;; + esac + done < <(tail -n 20 "$HARNESSD_LOG") + printf '\n %s %s\n\n' "$(style 2 '2' 'full log:')" "$HARNESSD_LOG" >&2 +} require_command() { local cmd="$1" @@ -184,7 +234,7 @@ start_server() { local port="${1}" local base_url="${2}" - info "no server at ${base_url}, starting harnessd on port ${port}" + info "starting harnessd on port ${port}" local harnessd_bin harnessd_bin="$(command -v harnessd)" @@ -202,7 +252,10 @@ start_server() { # harnessd's bind guard (cmd/harnessd/bind_guard.go, issue #1328) refuses to # start there without auth. HARNESS_ADDR supplies the port; the host is ours. # Issue #1411. - HARNESS_ADDR="127.0.0.1:${port}" "$harnessd_bin" & + local tmpdir="${TMPDIR:-/tmp}" + HARNESSD_LOG="${tmpdir%/}/harnessd.${$}.log" + ( umask 077; : > "$HARNESSD_LOG" ) + HARNESS_ADDR="127.0.0.1:${port}" "$harnessd_bin" >"$HARNESSD_LOG" 2>&1 & local pid=$! PID_FILE="${TMPDIR:-/tmp}/harnessd.${$}.pid" echo "$pid" > "$PID_FILE" @@ -218,10 +271,11 @@ start_server() { die "server did not become healthy within 10 s" fi if ! kill -0 "$pid" 2>/dev/null; then - die "harnessd (pid ${pid}) exited before becoming healthy on port ${port}. See the harnessd log above for the reason it stopped. If it reports the port is already in use, free it or run on another port with HARNESS_ADDR=:PORT (e.g. HARNESS_ADDR=:9090 go-code)." + die "harnessd (pid ${pid}) exited before becoming healthy on port ${port}. If it reports the port is already in use, free it or run on another port with HARNESS_ADDR=:PORT (e.g. HARNESS_ADDR=:9090 go-code)." fi done - info "server is ready" + info "$(style 1 '32' 'server ready') at ${base_url}" + info "log: ${HARNESSD_LOG}" } # --- project-root detection -------------------------------------------------- diff --git a/website/docs/cli/go-code-wrapper.md b/website/docs/cli/go-code-wrapper.md index 2c752afb..eddbc29c 100644 --- a/website/docs/cli/go-code-wrapper.md +++ b/website/docs/cli/go-code-wrapper.md @@ -94,9 +94,10 @@ Starts `harnessd` in the background, prints the server URL, and exits immediatel Example output: ``` -[go-code] no server at http://127.0.0.1:8080, starting harnessd on port 8080 +[go-code] starting harnessd on port 8080 [go-code] waiting for server to become healthy (pid 12345)... -[go-code] server is ready +[go-code] server ready at http://127.0.0.1:8080 +[go-code] log: /tmp/harnessd.12345.log [go-code] project root: /your/project/root [go-code] server running at http://127.0.0.1:8080 (pid 12345) http://127.0.0.1:8080 @@ -182,6 +183,14 @@ The address can also be set in your project or user config file (`~/.harness/con If you need a daemon reachable from another machine, don't go through `go-code` — run `harnessd` directly with an explicit `HARNESS_ADDR` (a real host, not just a port) and either a configured API key store or `HARNESS_AUTH_DISABLED=true`. An unauthenticated daemon that listens beyond loopback refuses to start otherwise. +### Startup output and the daemon log + +When the wrapper starts `harnessd` itself, the daemon's stdout and stderr no longer print to your terminal. They are redirected to a log file at `${TMPDIR:-/tmp}/harnessd..log` (created with `umask 077`, so only you can read it), and the wrapper prints that path on a `log:` line after a successful start. This matters most in `--server` mode, where the daemon outlives the wrapper process. This keeps a healthy startup to a handful of lines instead of interleaving the daemon's own boot log into the terminal, and it stops a stray daemon log line from landing in the TUI after handoff. + +If `harnessd` fails to become healthy, `go-code` prints the last 20 lines of that log under a `harnessd said:` heading before exiting, so the failure reason isn't just a path you have to go open yourself. Lines matching `fatal:`, `panic:`, or `refusing to start` are highlighted; the rest are dimmed. + +The wrapper colors its own `[go-code]` prefix and the `WARN:`/`ERROR:` markers (cyan, yellow, red) when writing to a terminal. Color is never the only signal — the words `WARN:` and `ERROR:` always stay in the text. Color is disabled, and output is plain text, whenever `NO_COLOR` is set, `TERM=dumb`, or the given output stream (stdout or stderr) isn't a terminal — for example when you pipe `go-code` into another command. stdout and stderr are checked independently, since one can be redirected without the other. + ### Project root detection `go-code` automatically resolves the workspace root before launching TUI or prompt mode. It walks parent directories from `$PWD`, looking for: