Skip to content

Commit 2f07df6

Browse files
committed
fix: warn when an agent commit lands with no Entire session recorded
PrepareCommitMsg only logged a debug line when it found no session to attach a commit to, so the failure (agent session started outside the worktree, drifted hook config, an incompatible entire on PATH, etc.) was invisible until a user later noticed nothing had been recorded. Print a visible, rate-limited stderr warning instead, but only when a known coding-agent env marker is present (CLAUDECODE, GEMINI_CLI, COPILOT_CLI, PI_CODING_AGENT) and no session anywhere in the repo has recorded recent activity, so a plain human commit or a healthy multi-worktree agent workflow never triggers it. Fixes #1965
1 parent 3d1b73f commit 2f07df6

3 files changed

Lines changed: 295 additions & 0 deletions

File tree

cmd/entire/cli/interactive/interactive.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,30 @@ func isAgentSubprocessEnv() bool {
8585
os.Getenv("GIT_TERMINAL_PROMPT") == "0"
8686
}
8787

88+
// HasAgentEnvMarker reports whether the environment carries a marker set by a
89+
// known coding agent's own process. This answers a different question from
90+
// isAgentSubprocessEnv above: that one decides "can this process prompt on
91+
// its inherited TTY" (and so also honors GIT_TERMINAL_PROMPT=0, a caller
92+
// preference rather than an agent identity); this one decides "is a coding
93+
// agent actually driving this process", used where misidentifying a human's
94+
// own commit as agent-driven would be the worse mistake (issue #1965: warning
95+
// when a commit lands with no Entire session recording it).
96+
//
97+
// - CLAUDECODE=1: Claude Code sets this in the environment of every command
98+
// it runs.
99+
// - GEMINI_CLI=1, COPILOT_CLI=1, PI_CODING_AGENT=true: see
100+
// isAgentSubprocessEnv.
101+
//
102+
// Markers for other integrations (Cursor, Factory AI Droid, OpenCode, Codex)
103+
// are not yet verified here and are deliberately omitted rather than guessed:
104+
// omission means "not detected", not "definitely not an agent".
105+
func HasAgentEnvMarker() bool {
106+
return os.Getenv("CLAUDECODE") != "" ||
107+
os.Getenv("GEMINI_CLI") != "" ||
108+
os.Getenv("COPILOT_CLI") != "" ||
109+
os.Getenv("PI_CODING_AGENT") != ""
110+
}
111+
88112
// IsTerminalReader reports whether r is an *os.File backed by a terminal.
89113
// It is useful when an explicitly interactive command needs to distinguish a
90114
// human at stdin from an agent process that merely inherited a controlling TTY.
Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
package strategy
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"os"
7+
"path/filepath"
8+
"testing"
9+
10+
"github.com/entireio/cli/cmd/entire/cli/agent"
11+
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
// Issue #1965: when a coding agent commits in an Entire-enabled repo but no
17+
// session anywhere in the repository has recorded recent activity (e.g. the
18+
// agent was launched from a bare-clone layout's shared root, outside any
19+
// worktree, so its hooks never fired), PrepareCommitMsg used to only log a
20+
// debug line invisible to a normal user before silently skipping the commit.
21+
// These tests exercise the real hook handler (not a hand-mocked call) end to
22+
// end and assert a visible stderr warning now appears.
23+
24+
// writeEnabledSettings writes a minimal .entire/settings.json with
25+
// "enabled": true, matching what `entire enable` produces and what the
26+
// PersistentPreRun gate in hooks_git_cmd.go requires before any git-hook
27+
// command reaches the strategy layer in a real invocation.
28+
func writeEnabledSettings(t *testing.T, dir string) {
29+
t.Helper()
30+
entireDir := filepath.Join(dir, ".entire")
31+
require.NoError(t, os.MkdirAll(entireDir, 0o755))
32+
require.NoError(t, os.WriteFile(
33+
filepath.Join(entireDir, "settings.json"),
34+
[]byte(`{"enabled":true}`),
35+
0o644,
36+
))
37+
}
38+
39+
// TestPrepareCommitMsg_AgentCommitNoSession_WarnsVisibly is the core
40+
// regression test for #1965. Setup: a real temp repo (testutil.InitRepo via
41+
// setupGitRepo), Entire enabled, CLAUDECODE=1 (the real env marker Claude
42+
// Code sets on every command it runs), and deliberately NO session state
43+
// anywhere (.git/entire-sessions/ is never populated - no InitializeSession
44+
// call). PrepareCommitMsg is invoked directly - the actual function a real
45+
// prepare-commit-msg git hook invocation reaches - with a real commit message
46+
// file, and stderr is captured through the package's real injectable
47+
// stderrWriter (the same seam warnStaleEndedSessions already uses).
48+
//
49+
// On the fixed code this must produce a visible warning. Run against the
50+
// pre-fix code (git stash the strategy fix), the captured buffer is empty:
51+
// the only trace was the debug-level "prepare-commit-msg: no active
52+
// sessions" log line, invisible to a user reading their terminal.
53+
func TestPrepareCommitMsg_AgentCommitNoSession_WarnsVisibly(t *testing.T) {
54+
dir := setupGitRepo(t)
55+
t.Chdir(dir)
56+
writeEnabledSettings(t, dir)
57+
t.Setenv("CLAUDECODE", "1")
58+
59+
s := &ManualCommitStrategy{}
60+
61+
commitMsgFile := filepath.Join(t.TempDir(), "COMMIT_EDITMSG")
62+
require.NoError(t, os.WriteFile(commitMsgFile, []byte("fix: something\n"), 0o644))
63+
64+
var buf bytes.Buffer
65+
oldWriter := stderrWriter
66+
stderrWriter = &buf
67+
defer func() { stderrWriter = oldWriter }()
68+
69+
err := s.PrepareCommitMsg(context.Background(), commitMsgFile, "")
70+
require.NoError(t, err)
71+
72+
t.Logf("captured stderr: %q", buf.String())
73+
74+
assert.Contains(t, buf.String(), "no active Entire session",
75+
"a coding-agent commit with no session recorded anywhere in the repo must print a visible stderr warning (issue #1965)")
76+
assert.Contains(t, buf.String(), "entire doctor",
77+
"the warning should point the user at `entire doctor`")
78+
}
79+
80+
// TestPrepareCommitMsg_HumanCommitNoSession_NoWarn pins the other half of the
81+
// enabled-but-no-session vs no-agent-at-all distinction: with Entire enabled,
82+
// no session anywhere, but NO agent env marker set (an ordinary human running
83+
// `git commit` by hand), the warning must stay silent. This is
84+
// indistinguishable from "no agent running at all" and must never nag a
85+
// human's own commit.
86+
func TestPrepareCommitMsg_HumanCommitNoSession_NoWarn(t *testing.T) {
87+
dir := setupGitRepo(t)
88+
t.Chdir(dir)
89+
writeEnabledSettings(t, dir)
90+
// Force-clear every known agent env marker: this test asserts the
91+
// no-agent-at-all case, and the test process itself may be running
92+
// inside an actual agent session (e.g. this very fix was developed under
93+
// Claude Code, which sets CLAUDECODE=1 in its own subprocess env) - that
94+
// ambient marker must not leak into "no agent" test coverage.
95+
t.Setenv("CLAUDECODE", "")
96+
t.Setenv("GEMINI_CLI", "")
97+
t.Setenv("COPILOT_CLI", "")
98+
t.Setenv("PI_CODING_AGENT", "")
99+
100+
s := &ManualCommitStrategy{}
101+
102+
commitMsgFile := filepath.Join(t.TempDir(), "COMMIT_EDITMSG")
103+
require.NoError(t, os.WriteFile(commitMsgFile, []byte("fix: something\n"), 0o644))
104+
105+
var buf bytes.Buffer
106+
oldWriter := stderrWriter
107+
stderrWriter = &buf
108+
defer func() { stderrWriter = oldWriter }()
109+
110+
err := s.PrepareCommitMsg(context.Background(), commitMsgFile, "")
111+
require.NoError(t, err)
112+
113+
t.Logf("captured stderr: %q", buf.String())
114+
115+
assert.Empty(t, buf.String(),
116+
"a human commit with no agent env marker must never warn, even with no session recorded")
117+
}
118+
119+
// TestPrepareCommitMsg_AgentCommitWithSession_NoWarn is the mandatory
120+
// regression guard: a real session with recent activity in THIS worktree
121+
// (initialized via the real InitializeSession turn-start path, not a
122+
// hand-built state) must suppress the warning entirely - PrepareCommitMsg
123+
// takes the "sessions found" branch and never reaches the no-session warning
124+
// at all.
125+
func TestPrepareCommitMsg_AgentCommitWithSession_NoWarn(t *testing.T) {
126+
dir := setupGitRepo(t)
127+
t.Chdir(dir)
128+
writeEnabledSettings(t, dir)
129+
t.Setenv("CLAUDECODE", "1")
130+
131+
s := &ManualCommitStrategy{}
132+
133+
sessionID := "test-session-1965-present"
134+
require.NoError(t, s.InitializeSession(context.Background(), sessionID, agent.AgentTypeClaudeCode, "", "working on a fix", ""))
135+
136+
state, err := s.loadSessionState(context.Background(), sessionID)
137+
require.NoError(t, err)
138+
require.NotNil(t, state.LastInteractionTime, "InitializeSession's turn-start transition should stamp LastInteractionTime")
139+
140+
commitMsgFile := filepath.Join(t.TempDir(), "COMMIT_EDITMSG")
141+
require.NoError(t, os.WriteFile(commitMsgFile, []byte("fix: something\n"), 0o644))
142+
143+
var buf bytes.Buffer
144+
oldWriter := stderrWriter
145+
stderrWriter = &buf
146+
defer func() { stderrWriter = oldWriter }()
147+
148+
err = s.PrepareCommitMsg(context.Background(), commitMsgFile, "")
149+
require.NoError(t, err)
150+
151+
t.Logf("captured stderr: %q", buf.String())
152+
153+
assert.Empty(t, buf.String(),
154+
"an agent commit with a recently-active session present must not warn")
155+
}
156+
157+
// TestWarnIfAgentCommitHasNoSession_RateLimit pins the sentinel-file rate
158+
// limit, mirroring TestWarnStaleEndedSessions_RateLimit's pattern.
159+
func TestWarnIfAgentCommitHasNoSession_RateLimit(t *testing.T) {
160+
dir := setupGitRepo(t)
161+
t.Chdir(dir)
162+
t.Setenv("CLAUDECODE", "1")
163+
ctx := context.Background()
164+
165+
s := &ManualCommitStrategy{}
166+
167+
var buf bytes.Buffer
168+
s.warnIfAgentCommitHasNoSessionTo(ctx, &buf)
169+
assert.Contains(t, buf.String(), "no active Entire session")
170+
171+
buf.Reset()
172+
s.warnIfAgentCommitHasNoSessionTo(ctx, &buf)
173+
assert.Empty(t, buf.String(), "second call within window must be suppressed")
174+
}

cmd/entire/cli/strategy/manual_commit_hooks.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,11 @@ func (s *ManualCommitStrategy) PrepareCommitMsg(ctx context.Context, commitMsgFi
395395
slog.String("strategy", "manual-commit"),
396396
slog.String("source", source),
397397
)
398+
if err == nil {
399+
// Only warn on a proven-empty listing, not on a listing error —
400+
// we can't claim "no session" when we don't actually know.
401+
s.warnIfAgentCommitHasNoSession(ctx)
402+
}
398403
return nil
399404
}
400405
findSessionsSpan.End()
@@ -848,6 +853,98 @@ func warnStaleEndedSessionsTo(ctx context.Context, count int, w io.Writer) {
848853
)
849854
}
850855

856+
// agentCommitNoSessionWarnInterval/File rate-limit the warning below the same
857+
// way staleEndedSessionWarnInterval/File do: a sentinel file's mtime under
858+
// entire-sessions/, shared across worktrees (session state already is), so
859+
// the warning re-appears at most once per window rather than on every commit
860+
// while the underlying cause (e.g. an agent session started outside any
861+
// worktree) persists.
862+
const (
863+
agentCommitNoSessionWarnInterval = 24 * time.Hour
864+
agentCommitNoSessionWarnFile = ".warn-agent-no-session"
865+
)
866+
867+
// warnIfAgentCommitHasNoSession prints a rate-limited, visible warning when a
868+
// coding agent appears to be driving this commit but no session anywhere in
869+
// the repository (any worktree) has recorded recent activity — the "this
870+
// commit will not be linked to any session" case from issue #1965. It is
871+
// called from PrepareCommitMsg's "no active sessions" branch, which is the
872+
// exact point where the only trace of the condition used to be a debug-only
873+
// log line no normal user would ever see.
874+
//
875+
// The condition is deliberately cause-agnostic. "Agent committing, nothing
876+
// recorded" is reachable many ways — the agent session started outside this
877+
// worktree (e.g. a bare-clone layout's shared root), the git hooks are
878+
// registered but the entire binary they invoke fails to parse this repo's
879+
// settings, hook config drifted out of the agent's own settings file, or
880+
// session state was wiped mid-conversation by `entire clean` — and at the
881+
// moment of the commit this hook already has everything needed to notice the
882+
// shared symptom without having to diagnose which cause produced it.
883+
//
884+
// Firing requires all three, each independently necessary:
885+
//
886+
// 1. An agent env marker is present (interactive.HasAgentEnvMarker). A human
887+
// typing `git commit` with no agent involved never warns — this is the
888+
// line between "Entire enabled but no session recording" (worth a
889+
// warning) and "no agent running at all" (indistinguishable from an
890+
// ordinary human commit, and must stay silent).
891+
// 2. No session anywhere in the repository shows recent activity
892+
// (isRecentInteraction against LastInteractionTime). The caller already
893+
// knows sessions matched to *this* worktree/identity came up empty; this
894+
// re-checks the full listing because a live session in a sibling
895+
// worktree (task fan-out, an isolated review checkout, an agent cd-ing
896+
// between worktrees) is a deliberate, healthy shape — `entire session
897+
// adopt` already covers moving it — and warning there would mostly nag
898+
// power users rather than catch a real miss.
899+
// 3. Not rate-limited (see the sentinel-file constants above), fail-open on
900+
// any file error exactly like warnStaleEndedSessions.
901+
//
902+
// settings.IsSetUpAndEnabled is intentionally NOT re-checked here: the sole
903+
// caller reaches this strategy method through the `entire hooks git` command
904+
// tree, which already refuses to invoke any hook handler at all when Entire
905+
// is not set up and enabled for the repo (hooks_git_cmd.go's
906+
// PersistentPreRun sets gitHooksDisabled and returns before RunE). A
907+
// disabled or never-configured repo therefore never reaches this function.
908+
func (s *ManualCommitStrategy) warnIfAgentCommitHasNoSession(ctx context.Context) {
909+
s.warnIfAgentCommitHasNoSessionTo(ctx, stderrWriter)
910+
}
911+
912+
func (s *ManualCommitStrategy) warnIfAgentCommitHasNoSessionTo(ctx context.Context, w io.Writer) {
913+
if !interactive.HasAgentEnvMarker() {
914+
return
915+
}
916+
917+
states, err := s.listAllSessionStates(ctx)
918+
if err != nil {
919+
return // fail-open: couldn't prove "no session", so don't claim it
920+
}
921+
for _, state := range states {
922+
if isRecentInteraction(state.LastInteractionTime) {
923+
return // a session is live somewhere in the repo — stay quiet
924+
}
925+
}
926+
927+
root, err := gitdir.Open(ctx)
928+
if err != nil {
929+
return // fail-open
930+
}
931+
warnFile := session.SessionStateDirName + "/" + agentCommitNoSessionWarnFile
932+
if info, statErr := root.Lstat(warnFile); statErr == nil {
933+
if time.Since(info.ModTime()) < agentCommitNoSessionWarnInterval {
934+
return // rate-limited
935+
}
936+
}
937+
//nolint:errcheck // Best-effort warning — fail-open if file ops fail
938+
_ = osroot.MkdirAllNoSymlink(root, session.SessionStateDirName, 0o750)
939+
//nolint:errcheck // Best-effort sentinel file write
940+
_ = jsonutil.WriteFileAtomicIn(root, warnFile, []byte{}, 0o644)
941+
fmt.Fprint(w,
942+
"\nentire: warning: a commit was made but no active Entire session was found — "+
943+
"this commit will not be linked to any session.\n"+
944+
"Run 'entire doctor' if this is unexpected.\n\n",
945+
)
946+
}
947+
851948
// activeSessionInteractionThreshold is the maximum age of LastInteractionTime
852949
// for an ACTIVE session to be considered genuinely active. 24h is generous
853950
// because LastInteractionTime only updates at TurnStart, not per-tool-call.

0 commit comments

Comments
 (0)