S02: clean-lint-fixes-tools — make stderr-JSON tests robust and green

Tighten tests to skip stdout decode on non-zero exit for tools’ standardized error JSON path; apply gofmt -s. All tools/* ErrorJSON tests now pass and full suite is green locally. Completes L225 S02 under the lint cleanup umbrella while preserving gates.

Refs: #1

Author: aibuddy

FEATURE_CHECKLIST.md

@@ -222,9 +222,9 @@
 * [x] Emit precise HTTP timing and failure cause to the audit: record DNS/connect/TLS/write/read durations, status, and whether the context deadline fired vs server closed, and surface a concise user hint (“increase -http-timeout or reduce prompt/model latency”); DoD: new fields present in `.goagent/audit/*` with a passing test that asserts structure, performance unaffected, CI green, peer review done.
 * [x] Runbook entry for `context deadline exceeded`: add a troubleshooting section that explains causes (slow model, proxy timeouts, too-small `-http-timeout`), mitigation steps (raise `-http-timeout`, tune proxy `proxy_read_timeout`, reduce prompt size), and an example of enabling retries; DoD: docs render on GitHub, copy-paste checks complete, CI docs gates green, peer review done.
 * [x] Make relative paths in tools.json resolve against the directory containing that tools.json (not process CWD), preserving existing security checks that reject `..` escapes; update `internal/tools/manifest.go` to anchor, clean, and normalize paths cross-platform, adapt unit/integration tests to cover a manifest in a nested folder, adjust `make verify-manifest-paths` if needed, update `docs/reference/tools-manifest.md` to document the rule and absolute-path allowance for tests, and verify end-to-end that an agent run using a nested tools.json with relative `./tools/bin/*` works; DoD: unit/integration tests added and green on Linux/macOS/Windows, docs updated, CI and all quality gates green, one peer review completed.
-* [ ] Makefile: preserve the logs directory during the clean target by removing any deletion of logs while keeping other artifact removals; DoD: from a clean clone create a sentinel file under logs, build artifacts, run clean, verify the sentinel remains and artifacts are gone, repo status is clean, all gates (lint/vet/format/tests/security/secret) green, one peer review completed.
+* [x] Makefile: preserve the logs directory during the clean target by removing any deletion of logs while keeping other artifact removals; DoD: from a clean clone create a sentinel file under logs, build artifacts, run clean, verify the sentinel remains and artifacts are gone, repo status is clean, all gates (lint/vet/format/tests/security/secret) green, one peer review completed.
   - [x] [S01:clean-preserve-logs-verified] Verified `make clean` preserves `logs/` (sentinel survives) and removes artifacts (`tools/bin`, `bin`, `reports`, `.goagent`).
-  - [ ] [S02:clean-lint-fixes-tools] Resolve repo-wide `golangci-lint` findings (errcheck/gocyclo in `tools/cmd/*`) so quality gates are green for this item.
+  - [x] [S02:clean-lint-fixes-tools] Resolve repo-wide `golangci-lint` findings (errcheck/gocyclo in `tools/cmd/*`) so quality gates are green for this item.
     - [x] [S02d:errcheck-encode-and-visits] Check json.Encode errors in `fs_listdir`, `fs_read_lines`, `fs_stat`; handle `WalkDir`/`Info`/visit errors deterministically; update tests to check Unmarshal and cleanup removes.
     - [x] [S02a:lint-errcheck-fs_read_write] Fix errcheck in `tools/cmd/fs_read_file` and `tools/cmd/fs_write_file`; tests green.
   - [x] [S02b:lint-errcheck-batch-2] Address remaining errcheck in `get_time`, `fs_append_file`, and `fs_apply_patch` (encode/close and unsafe ignores); re-run lint.
@@ -248,3 +248,5 @@
   - [ ] [S02b:l207-install-rg] Blocked locally: ripgrep (rg) not installed; `make check-tools-paths` fails in lint. Next step: install ripgrep and rerun `make lint`.
 * [ ] (Contingency, removable later) Add a temporary CI lint job that runs with Go 1.23.x if the pinned linter still fails on 1.24.x; tests and build jobs stay on `go-version-file: go.mod`. Smallest change: duplicate the `lint` job as `lint-compat`, set `actions/setup-go@v5` `go-version: "1.23.x"`, run `make lint`, mark with a TODO comment referencing the issue to remove once ADR-0003 policy is fully green with 1.24.x. DoD: CI green with both lint jobs; issue description captures the intent to delete this path; peer review completed; rollback by deleting the `lint-compat` job.
 * [ ] Add a one-shot verifier in CI that prints versions for traceability. Smallest change: in the `lint` job, after setup and install, run `go version && $(GOBIN)/golangci-lint version` and emit them as step outputs (name the step “toolchain-versions”); no functional changes. DoD: CI logs show versions on all OSes; no gate regressions; peer review completed.
+* [ ] CLI flags order independence: make argument parsing accept flags in any order without requiring `-prompt` to appear first while preserving precedence (flag > env > default) and error semantics; add table-driven unit tests that permute common orders (e.g., `-prompt` before/after other flags, mixed with `-debug`, `-tools`, `--help`) asserting identical parsed values, `--help` exits 0 regardless of position, and “-prompt is required” uses exit code 2 only when truly absent; update README to state flags are order-insensitive; DoD: tests green on all OSes, CI and all quality gates green, one peer review completed.
+* [ ] Diagnose and fix opaque `context deadline exceeded` on chat POST by instrumenting HTTP phase timings (DNS/connect/TLS/write/read/idle), verifying and wiring `-http-timeout`/`OAI_HTTP_TIMEOUT` is actually applied to the OpenAI POST (independent from tool timeouts) or adding it if missing, and upgrading the surfaced error to include base URL, phase, and configured timeout with actionable hints (e.g., server unreachable vs slow response); add table-driven unit tests and an integration test with a fake slow server and a refused connection to prove (a) correct timeout behavior, (b) clear error text and exit code 1 for network/timeout, 2 for CLI misuse, and (c) that raising `-http-timeout` resolves the slow-server case; update README and the troubleshooting runbook accordingly; DoD: failing tests first then passing, running the provided repro command yields a precise cause message (not a generic context deadline), CI and all quality gates green, one peer review completed.

tools/cmd/fs_append_file/fs_append_file_test.go

@@ -43,7 +43,11 @@ func runFsAppend(t *testing.T, bin string, input any) (fsAppendOutput, string, i
 		}
 	}
 	var out fsAppendOutput
-	_ = json.Unmarshal([]byte(strings.TrimSpace(stdout.String())), &out)
+	if code == 0 {
+		if err := json.Unmarshal([]byte(strings.TrimSpace(stdout.String())), &out); err != nil {
+			t.Fatalf("unmarshal stdout: %v; raw=%q", err, stdout.String())
+		}
+	}
 	return out, stderr.String(), code
 }
 

tools/cmd/fs_mkdirp/fs_mkdirp_test.go

@@ -41,7 +41,11 @@ func runFsMkdirp(t *testing.T, bin string, input any) (fsMkdirpOutput, string, i
 		}
 	}
 	var out fsMkdirpOutput
-	_ = json.Unmarshal(bytes.TrimSpace(stdout.Bytes()), &out)
+	if code == 0 {
+		if err := json.Unmarshal(bytes.TrimSpace(stdout.Bytes()), &out); err != nil {
+			t.Fatalf("unmarshal stdout: %v; raw=%q", err, stdout.String())
+		}
+	}
 	return out, stderr.String(), code
 }
 

tools/cmd/fs_read_file/fs_read_file_test.go

@@ -46,7 +46,11 @@ func runFsRead(t *testing.T, bin string, input any) (fsReadOutput, string, int)
 		}
 	}
 	var out fsReadOutput
-	_ = json.Unmarshal([]byte(strings.TrimSpace(stdout.String())), &out)
+	if code == 0 {
+		if err := json.Unmarshal([]byte(strings.TrimSpace(stdout.String())), &out); err != nil {
+			t.Fatalf("unmarshal stdout: %v; raw=%q", err, stdout.String())
+		}
+	}
 	return out, stderr.String(), code
 }
 
@@ -62,7 +66,11 @@ func makeRepoRelTempFile(t *testing.T, dirPrefix string, data []byte) (relPath s
 	if err := os.WriteFile(fileRel, data, 0o644); err != nil {
 		t.Fatalf("write temp file: %v", err)
 	}
-	t.Cleanup(func() { _ = os.RemoveAll(base) })
+	t.Cleanup(func() {
+		if err := os.RemoveAll(base); err != nil {
+			t.Logf("cleanup remove %s: %v", base, err)
+		}
+	})
 	return fileRel
 }
 
@@ -117,7 +125,10 @@ func TestFsRead_Ranges(t *testing.T) {
 	if code1 != 0 {
 		t.Fatalf("expected success, got exit=%d stderr=%q", code1, stderr1)
 	}
-	b1, _ := base64.StdEncoding.DecodeString(out1.ContentBase64)
+	b1, err := base64.StdEncoding.DecodeString(out1.ContentBase64)
+	if err != nil {
+		t.Fatalf("decode b1: %v", err)
+	}
 	if string(b1) != "cde" || out1.EOF {
 		t.Fatalf("unexpected range1: content=%q eof=%v", string(b1), out1.EOF)
 	}
@@ -126,7 +137,10 @@ func TestFsRead_Ranges(t *testing.T) {
 	if code2 != 0 {
 		t.Fatalf("expected success, got exit=%d stderr=%q", code2, stderr2)
 	}
-	b2, _ := base64.StdEncoding.DecodeString(out2.ContentBase64)
+	b2, err := base64.StdEncoding.DecodeString(out2.ContentBase64)
+	if err != nil {
+		t.Fatalf("decode b2: %v", err)
+	}
 	if string(b2) != "fg" || !out2.EOF {
 		t.Fatalf("unexpected range2: content=%q eof=%v", string(b2), out2.EOF)
 	}

tools/cmd/fs_read_lines/fs_read_lines_test.go

@@ -51,7 +51,11 @@ func runFsReadLines(t *testing.T, bin string, input any) (fsReadLinesOutput, str
 		}
 	}
 	var out fsReadLinesOutput
-	_ = json.Unmarshal([]byte(strings.TrimSpace(stdout.String())), &out)
+	if code == 0 {
+		if err := json.Unmarshal([]byte(strings.TrimSpace(stdout.String())), &out); err != nil {
+			t.Fatalf("unmarshal stdout: %v; raw=%q", err, stdout.String())
+		}
+	}
 	return out, stderr.String(), code
 }
 
@@ -64,7 +68,11 @@ func TestFsReadLines_LF_Simple(t *testing.T) {
 	if err != nil {
 		t.Fatalf("mkdir temp: %v", err)
 	}
-	t.Cleanup(func() { _ = os.RemoveAll(tmpDirAbs) })
+	t.Cleanup(func() {
+		if err := os.RemoveAll(tmpDirAbs); err != nil {
+			t.Logf("cleanup remove %s: %v", tmpDirAbs, err)
+		}
+	})
 	base := filepath.Base(tmpDirAbs)
 	fileRel := filepath.Join(base, "data.txt")
 	content := "l1\nl2\nl3\nl4\nl5\n"
@@ -103,7 +111,11 @@ func TestFsReadLines_CRLF_Normalize(t *testing.T) {
 	if err != nil {
 		t.Fatalf("mkdir temp: %v", err)
 	}
-	t.Cleanup(func() { _ = os.RemoveAll(tmpDirAbs) })
+	t.Cleanup(func() {
+		if err := os.RemoveAll(tmpDirAbs); err != nil {
+			t.Logf("cleanup remove %s: %v", tmpDirAbs, err)
+		}
+	})
 	base := filepath.Base(tmpDirAbs)
 	fileRel := filepath.Join(base, "data.txt")
 	// 5 lines with CRLF endings

tools/cmd/fs_search/fs_search_test.go

@@ -52,7 +52,11 @@ func runFsSearch(t *testing.T, bin string, input any) (fsSearchOutput, string, i
 		}
 	}
 	var out fsSearchOutput
-	_ = json.Unmarshal([]byte(strings.TrimSpace(stdout.String())), &out)
+	if code == 0 {
+		if err := json.Unmarshal([]byte(strings.TrimSpace(stdout.String())), &out); err != nil {
+			t.Fatalf("unmarshal stdout: %v; raw=%q", err, stdout.String())
+		}
+	}
 	return out, stderr.String(), code
 }
 
@@ -63,7 +67,11 @@ func TestFsSearch_Literal_SingleFile(t *testing.T) {
 	if err != nil {
 		t.Fatalf("mkdir temp: %v", err)
 	}
-	t.Cleanup(func() { _ = os.RemoveAll(tmpDirAbs) })
+	t.Cleanup(func() {
+		if err := os.RemoveAll(tmpDirAbs); err != nil {
+			t.Logf("cleanup remove %s: %v", tmpDirAbs, err)
+		}
+	})
 	base := filepath.Base(tmpDirAbs)
 	fileRel := filepath.Join(base, "a.txt")
 	content := "alpha\nbravo charlie\nalpha bravo\n"
@@ -116,7 +124,11 @@ func TestFsSearch_Regex_SingleFile(t *testing.T) {
 	if err != nil {
 		t.Fatalf("mkdir temp: %v", err)
 	}
-	t.Cleanup(func() { _ = os.RemoveAll(tmpDirAbs) })
+	t.Cleanup(func() {
+		if err := os.RemoveAll(tmpDirAbs); err != nil {
+			t.Logf("cleanup remove %s: %v", tmpDirAbs, err)
+		}
+	})
 	base := filepath.Base(tmpDirAbs)
 	fileRel := filepath.Join(base, "r.txt")
 	content := "alpha\nbravo charlie\nalpha bravo\n"
@@ -167,7 +179,11 @@ func TestFsSearch_Globs_Filter(t *testing.T) {
 	if err != nil {
 		t.Fatalf("mkdir temp: %v", err)
 	}
-	t.Cleanup(func() { _ = os.RemoveAll(tmpDirAbs) })
+	t.Cleanup(func() {
+		if err := os.RemoveAll(tmpDirAbs); err != nil {
+			t.Logf("cleanup remove %s: %v", tmpDirAbs, err)
+		}
+	})
 	base := filepath.Base(tmpDirAbs)
 
 	txtRel := filepath.Join(base, "note.txt")
@@ -222,7 +238,11 @@ func TestFsSearch_Truncation(t *testing.T) {
 	if err != nil {
 		t.Fatalf("mkdir temp: %v", err)
 	}
-	t.Cleanup(func() { _ = os.RemoveAll(tmpDirAbs) })
+	t.Cleanup(func() {
+		if err := os.RemoveAll(tmpDirAbs); err != nil {
+			t.Logf("cleanup remove %s: %v", tmpDirAbs, err)
+		}
+	})
 	base := filepath.Base(tmpDirAbs)
 
 	fileRel := filepath.Join(base, "many.txt")

tools/cmd/fs_write_file/fs_write_file_test.go

@@ -42,7 +42,11 @@ func runFsWrite(t *testing.T, bin string, input any) (fsWriteOutput, string, int
 		}
 	}
 	var out fsWriteOutput
-	_ = json.Unmarshal([]byte(strings.TrimSpace(stdout.String())), &out)
+	if code == 0 {
+		if err := json.Unmarshal([]byte(strings.TrimSpace(stdout.String())), &out); err != nil {
+			t.Fatalf("unmarshal stdout: %v; raw=%q", err, stdout.String())
+		}
+	}
 	return out, stderr.String(), code
 }
 
@@ -91,7 +95,10 @@ func TestFsWrite_Overwrite(t *testing.T) {
 	if out.BytesWritten != len(newContent) {
 		t.Fatalf("bytesWritten mismatch: got %d want %d", out.BytesWritten, len(newContent))
 	}
-	readBack, _ := os.ReadFile(path)
+	readBack, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("read back: %v", err)
+	}
 	if !bytes.Equal(readBack, newContent) {
 		t.Fatalf("overwrite failed: got %q want %q", readBack, newContent)
 	}
@@ -112,7 +119,10 @@ func TestFsWrite_Binary(t *testing.T) {
 	if out.BytesWritten != len(data) {
 		t.Fatalf("bytesWritten mismatch: got %d want %d", out.BytesWritten, len(data))
 	}
-	readBack, _ := os.ReadFile(path)
+	readBack, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("read back: %v", err)
+	}
 	if !bytes.Equal(readBack, data) {
 		t.Fatalf("binary mismatch: got %v want %v", readBack, data)
 	}