test: raise codex package coverage to ≥80%

internal/agent/codex/discover_extra_test.go

@@ -0,0 +1,83 @@
+package codex
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+// TestDayDirsFor_MidnightAddsPrevDay covers the clock-skew branch:
+// when t falls in the first 5 minutes of a UTC day, dayDirsFor should
+// also include the previous day's directory so a rollover-skewed file
+// is still found.
+func TestDayDirsFor_MidnightAddsPrevDay(t *testing.T) {
+	root := "/codex/root"
+	// 00:02 UTC on 2026-04-27 — well within the 5-minute window.
+	t0 := time.Date(2026, 4, 27, 0, 2, 0, 0, time.UTC)
+	dirs := dayDirsFor(root, t0)
+	if len(dirs) != 2 {
+		t.Fatalf("expected 2 dirs (today + previous), got %d: %v", len(dirs), dirs)
+	}
+	want0 := filepath.Join(root, "2026", "04", "27")
+	want1 := filepath.Join(root, "2026", "04", "26")
+	if dirs[0] != want0 {
+		t.Errorf("dirs[0] = %q, want %q", dirs[0], want0)
+	}
+	if dirs[1] != want1 {
+		t.Errorf("dirs[1] = %q, want %q", dirs[1], want1)
+	}
+}
+
+// TestDayDirsFor_OutsideMidnightWindow covers the negative branch of
+// the same conditional: at noon UTC only today's day-dir is returned.
+func TestDayDirsFor_OutsideMidnightWindow(t *testing.T) {
+	root := "/codex/root"
+	t0 := time.Date(2026, 4, 27, 12, 0, 0, 0, time.UTC)
+	dirs := dayDirsFor(root, t0)
+	if len(dirs) != 1 {
+		t.Fatalf("expected 1 dir, got %d: %v", len(dirs), dirs)
+	}
+}
+
+// TestDayDirsFor_HourZeroPastFiveMinutes verifies the boundary: at
+// 00:05 UTC the previous-day include is gone (the conditional is
+// strict `< 5`).
+func TestDayDirsFor_HourZeroPastFiveMinutes(t *testing.T) {
+	root := "/codex/root"
+	t0 := time.Date(2026, 4, 27, 0, 5, 0, 0, time.UTC)
+	dirs := dayDirsFor(root, t0)
+	if len(dirs) != 1 {
+		t.Fatalf("expected 1 dir at 00:05, got %d: %v", len(dirs), dirs)
+	}
+}
+
+// TestNewestMatchingRollout_DirectoryInDir covers the inner-loop
+// `e.IsDir() continue` branch: a sub-directory in the day-dir is
+// ignored even if its name superficially matches.
+func TestNewestMatchingRollout_DirectoryInDir(t *testing.T) {
+	spawn := time.Now()
+	day := fakeCodexHome(t, spawn)
+	// Drop a directory shaped like a rollout file — must be skipped.
+	rolloutDir := filepath.Join(day, "rollout-2026-05-14T12-00-00-deadbeef.jsonl")
+	if err := os.MkdirAll(rolloutDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	id, _, ok := newestMatchingRollout(day, spawn)
+	if ok {
+		t.Fatalf("expected no match (only a dir present), got id=%q", id)
+	}
+}
+
+// TestNewestMatchingRollout_NonJsonlSkipped covers the
+// "name doesn't end with .jsonl" branch in newestMatchingRollout.
+func TestNewestMatchingRollout_NonJsonlSkipped(t *testing.T) {
+	spawn := time.Now()
+	day := fakeCodexHome(t, spawn)
+	writeRollout(t, day, "rollout-2026-05-14T12-00-00-abc.txt", spawn.Add(50*time.Millisecond))
+	id, _, ok := newestMatchingRollout(day, spawn)
+	if ok {
+		t.Fatalf("expected non-jsonl to be skipped, got id=%q", id)
+	}
+}
+

internal/agent/codex/process_test.go

@@ -0,0 +1,103 @@
+package codex
+
+import (
+	"os"
+	"strconv"
+	"testing"
+)
+
+// process.go is a Linux-only /proc scanner. The tests here mirror the
+// shape of internal/procscan/procscan_test.go but target the codex copy
+// (which predates the shared procscan package and is still wired into
+// some callers).
+
+func TestIsCodexAlive_EmptyPID(t *testing.T) {
+	if _, err := IsCodexAlive(""); err == nil {
+		t.Fatal("expected error on empty pid, got nil")
+	}
+}
+
+func TestIsCodexAlive_InvalidPID(t *testing.T) {
+	// Same caveat as procscan: Sscanf accepts "12x" as 12, so the
+	// branch we want is non-leading-digit + non-positive.
+	for _, pid := range []string{"abc", "-1", "0", " "} {
+		t.Run(pid, func(t *testing.T) {
+			if _, err := IsCodexAlive(pid); err == nil {
+				t.Fatalf("expected error for pid=%q", pid)
+			}
+		})
+	}
+}
+
+func TestIsCodexAlive_NonexistentPID(t *testing.T) {
+	alive, err := IsCodexAlive("2147483647")
+	if err != nil {
+		t.Fatalf("expected nil err for absent pid: %v", err)
+	}
+	if alive {
+		t.Fatal("expected alive=false for absent pid")
+	}
+}
+
+func TestIsCodexAlive_Self(t *testing.T) {
+	pid := strconv.Itoa(os.Getpid())
+	alive, err := IsCodexAlive(pid)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if !alive {
+		t.Fatalf("expected alive=true for self pid %s", pid)
+	}
+}
+
+func TestFindCodexChild_EmptyPID(t *testing.T) {
+	if got := FindCodexChild(""); got != "" {
+		t.Errorf("FindCodexChild(\"\") = %q, want \"\"", got)
+	}
+}
+
+// TestFindCodexChild_NoMatch exercises the full /proc scan returning ""
+// when no process matches the criteria. PPID "0" with no real codex
+// child guarantees nothing matches in CI.
+func TestFindCodexChild_NoMatch(t *testing.T) {
+	if got := FindCodexChild("0"); got != "" {
+		t.Errorf("FindCodexChild(0) = %q, want \"\"", got)
+	}
+}
+
+func TestIsNumeric_Codex(t *testing.T) {
+	cases := []struct {
+		in   string
+		want bool
+	}{
+		{"", false},
+		{"0", true},
+		{"12345", true},
+		{"12a", false},
+		{"-1", false},
+		{"一", false},
+	}
+	for _, tc := range cases {
+		if got := isNumeric(tc.in); got != tc.want {
+			t.Errorf("isNumeric(%q) = %v, want %v", tc.in, got, tc.want)
+		}
+	}
+}
+
+func TestReadProcStatus_Self(t *testing.T) {
+	pid := strconv.Itoa(os.Getpid())
+	ppid, procName, ok := readProcStatus("/proc/" + pid + "/status")
+	if !ok {
+		t.Fatal("readProcStatus on self returned ok=false")
+	}
+	if ppid == "" || procName == "" {
+		t.Errorf("expected non-empty fields, got ppid=%q procName=%q", ppid, procName)
+	}
+}
+
+func TestReadProcStatus_MissingFile(t *testing.T) {
+	_, _, ok := readProcStatus("/proc/2147483647/status")
+	if ok {
+		t.Error("expected ok=false for missing path")
+	}
+}

internal/config/config_test.go

@@ -333,6 +333,92 @@ func TestMigration_V1ToV2_Idempotent(t *testing.T) {
 	}
 }
 
+// TestHookTimeout_ZeroIsDefault and TestHookTimeout_ExplicitRespected
+// cover both branches of the HookTimeout duration resolver. Zero (and
+// negative) values fall back to the package default; positive seconds
+// are returned verbatim.
+func TestHookTimeout_ZeroIsDefault(t *testing.T) {
+	for _, n := range []int{0, -3} {
+		c := Config{HookTimeoutSec: n}
+		want := time.Duration(DefaultHookTimeoutSec) * time.Second
+		if got := c.HookTimeout(); got != want {
+			t.Errorf("HookTimeout(%d) = %v, want %v", n, got, want)
+		}
+	}
+}
+
+func TestHookTimeout_ExplicitRespected(t *testing.T) {
+	c := Config{HookTimeoutSec: 42}
+	if got := c.HookTimeout(); got != 42*time.Second {
+		t.Errorf("HookTimeout(42) = %v, want 42s", got)
+	}
+}
+
+// TestRewriteRequiredPathClaude_MissingKey covers the early-return
+// branch when obj has no required_in_path key at all (or an empty raw
+// value). A v0/v1 config that never customized the list looks exactly
+// like this.
+func TestRewriteRequiredPathClaude_MissingKey(t *testing.T) {
+	in := map[string]json.RawMessage{} // no required_in_path
+	if err := rewriteRequiredPathClaude(in); err != nil {
+		t.Fatalf("expected no-op nil, got %v", err)
+	}
+	if _, present := in["required_in_path"]; present {
+		t.Error("step should not invent the key")
+	}
+}
+
+// TestRewriteRequiredPathClaude_MalformedArray covers the
+// "json.Unmarshal into []json.RawMessage failed" branch: the malformed
+// value is left untouched (jsonstrict will surface the error on the
+// typed Load that follows). The step must not error.
+func TestRewriteRequiredPathClaude_MalformedArray(t *testing.T) {
+	in := map[string]json.RawMessage{
+		"required_in_path": json.RawMessage(`"not-an-array"`),
+	}
+	original := in["required_in_path"]
+	if err := rewriteRequiredPathClaude(in); err != nil {
+		t.Fatalf("expected no-op on malformed value, got %v", err)
+	}
+	if string(in["required_in_path"]) != string(original) {
+		t.Errorf("malformed value mutated: got %s", in["required_in_path"])
+	}
+}
+
+// TestRewriteRequiredPathClaude_NonStringEntries covers the inner-loop
+// branch where an individual entry is not a JSON string (defensive). It
+// is passed through verbatim.
+func TestRewriteRequiredPathClaude_NonStringEntries(t *testing.T) {
+	in := map[string]json.RawMessage{
+		"required_in_path": json.RawMessage(`["claude",123,{"x":1}]`),
+	}
+	if err := rewriteRequiredPathClaude(in); err != nil {
+		t.Fatalf("step error: %v", err)
+	}
+	var list []json.RawMessage
+	if err := json.Unmarshal(in["required_in_path"], &list); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if string(list[0]) != `"codex"` {
+		t.Errorf("first entry (claude) not rewritten: %s", list[0])
+	}
+	if string(list[1]) != `123` {
+		t.Errorf("non-string entry mutated: %s", list[1])
+	}
+}
+
+// TestRewriteRequiredPathClaude_EmptyRawValue covers the explicit
+// "raw exists but len==0" branch (defensive — Marshal would never
+// emit this, but a hand-edited config might).
+func TestRewriteRequiredPathClaude_EmptyRawValue(t *testing.T) {
+	in := map[string]json.RawMessage{
+		"required_in_path": json.RawMessage(``),
+	}
+	if err := rewriteRequiredPathClaude(in); err != nil {
+		t.Fatalf("expected no-op, got %v", err)
+	}
+}
+
 // TestMigration_V1ToV2_FullPlanRewritesLegacyConfig verifies end-to-
 // end behavior via the migrate runner: a v1 config.json with
 // `claude` in required_in_path lands at v2 with `codex`, no backup.

internal/health/agent_check_test.go

@@ -0,0 +1,104 @@
+package health
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// CheckWorkdir is the pure-stdlib half of agent_check.go — its
+// CheckAgentProcess sibling shells out to a tmux server and walks
+// /proc, both of which are sandbox-hostile. CheckWorkdir is fully
+// unit-testable with t.TempDir.
+
+func TestCheckWorkdir_Empty(t *testing.T) {
+	r := CheckWorkdir("")
+	if r.Passed() {
+		t.Fatal("expected fail on empty workdir")
+	}
+	if r.Name != "workdir" {
+		t.Errorf("Name = %q, want workdir", r.Name)
+	}
+	if !strings.Contains(r.Message, "not set") {
+		t.Errorf("Message should mention not-set, got %q", r.Message)
+	}
+	if r.Fix == "" {
+		t.Error("expected non-empty Fix hint for empty workdir")
+	}
+}
+
+func TestCheckWorkdir_Missing(t *testing.T) {
+	missing := filepath.Join(t.TempDir(), "does-not-exist")
+	r := CheckWorkdir(missing)
+	if r.Passed() {
+		t.Fatal("expected fail on missing dir")
+	}
+	if !strings.Contains(r.Message, "does not exist") {
+		t.Errorf("Message should mention does-not-exist, got %q", r.Message)
+	}
+	if !strings.Contains(r.Fix, "mkdir") {
+		t.Errorf("Fix should mention mkdir, got %q", r.Fix)
+	}
+}
+
+func TestCheckWorkdir_FileNotDir(t *testing.T) {
+	dir := t.TempDir()
+	file := filepath.Join(dir, "afile")
+	if err := os.WriteFile(file, []byte("x"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	r := CheckWorkdir(file)
+	if r.Passed() {
+		t.Fatal("expected fail on file-as-workdir")
+	}
+	if !strings.Contains(r.Message, "not a directory") {
+		t.Errorf("Message should mention not-a-directory, got %q", r.Message)
+	}
+}
+
+func TestCheckWorkdir_Happy(t *testing.T) {
+	dir := t.TempDir()
+	r := CheckWorkdir(dir)
+	if !r.Passed() {
+		t.Fatalf("expected pass on real dir, got %+v", r)
+	}
+	if r.Status != StatusPass {
+		t.Errorf("Status = %q, want %q", r.Status, StatusPass)
+	}
+	if !strings.Contains(r.Message, "exists and is a directory") {
+		t.Errorf("Message should mention success, got %q", r.Message)
+	}
+}
+
+// TestCheckWorkdir_StatError exercises the "stat returned an error
+// that is not IsNotExist" branch. On Linux, attempting to stat a path
+// under an unreadable directory yields EACCES rather than ENOENT.
+//
+// Skips on root (root can read everything).
+func TestCheckWorkdir_StatError(t *testing.T) {
+	if os.Geteuid() == 0 {
+		t.Skip("running as root; cannot exercise permission-denied branch")
+	}
+	parent := filepath.Join(t.TempDir(), "locked")
+	if err := os.Mkdir(parent, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	target := filepath.Join(parent, "child")
+	if err := os.Mkdir(target, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	// Strip parent's execute bit so stat on child fails with EACCES.
+	if err := os.Chmod(parent, 0o000); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.Chmod(parent, 0o700) })
+
+	r := CheckWorkdir(target)
+	if r.Passed() {
+		t.Fatal("expected fail when stat returns non-NotExist error")
+	}
+	if !strings.Contains(r.Message, "error checking") {
+		t.Errorf("Message should mention stat error, got %q", r.Message)
+	}
+}