From 4ff48cf3e753727c25dbde464721b034aab4b4ed Mon Sep 17 00:00:00 2001 From: Cole Date: Sat, 25 Jul 2026 22:37:28 -0700 Subject: [PATCH] fix(strategy): surface ambiguous-worktree warning to the user with a runnable remedy #1856 added warnAmbiguousWorktreeSessions for the #1852 case where live sessions span multiple worktrees and matching refuses to guess, so the commit is left unlinked. But it only logs to the internal .entire/logs file and prints no remedy, so the person running `git commit` still sees nothing -- the "silent" part of #1852 is unchanged for them. This keeps the structured WARN (diagnostics) and additionally writes a user-facing notice to stderrWriter -- the same channel warnIfAttributionDiverged and warnStaleEndedSessions use -- with a directly runnable remedy: entire: this commit was not linked to a checkpoint (live agent sessions span multiple worktrees: ../a, ../b). run 'entire session adopt --from ../a --yes' to link one explicitly. The command names a concrete session ID rather than the bare `--from ` form: bare --from relies on adoption's auto-detect, which only considers sessions within adoptRecentWindow (12h) and fails outright when a worktree has multiple sessions; and same-repo adoption requires --yes. The named session is the most-recently-active candidate that adoption will accept (mirrors isAdoptableSourceSession), so the suggested command actually runs. Additive only: the warning trigger and the existing log line are unchanged. Refs #1852 Follow-up to #1856 Co-Authored-By: Claude Opus 4.8 --- .../manual_commit_ambiguous_notice_test.go | 93 +++++++++++++++++++ .../cli/strategy/manual_commit_session.go | 62 +++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 cmd/entire/cli/strategy/manual_commit_ambiguous_notice_test.go diff --git a/cmd/entire/cli/strategy/manual_commit_ambiguous_notice_test.go b/cmd/entire/cli/strategy/manual_commit_ambiguous_notice_test.go new file mode 100644 index 0000000000..38bd9b317b --- /dev/null +++ b/cmd/entire/cli/strategy/manual_commit_ambiguous_notice_test.go @@ -0,0 +1,93 @@ +package strategy + +import ( + "testing" + "time" + + "github.com/entireio/cli/cmd/entire/cli/session" + "github.com/stretchr/testify/require" +) + +// TestFormatAmbiguousWorktreeNotice verifies the user-facing stderr message: it +// always reports the ambiguity, and when an adoptable session is available it +// appends a directly-runnable adopt command (concrete session ID so it doesn't +// depend on adoption's 12h auto-detect, plus --yes for same-repo adoption). +func TestFormatAmbiguousWorktreeNotice(t *testing.T) { + t.Parallel() + + t.Run("with adoptable session includes runnable command", func(t *testing.T) { + t.Parallel() + primary := &SessionState{SessionID: "sess-1", WorktreePath: "/repo/a"} + got := formatAmbiguousWorktreeNotice([]string{"/repo/a", "/repo/b"}, primary) + require.Contains(t, got, "not linked to a checkpoint") + require.Contains(t, got, "/repo/a, /repo/b") + require.Contains(t, got, "entire session adopt sess-1 --from '/repo/a' --yes") + }) + + t.Run("worktree path with spaces is shell-quoted", func(t *testing.T) { + t.Parallel() + primary := &SessionState{SessionID: "sess-1", WorktreePath: "/Users/cole/my repo"} + got := formatAmbiguousWorktreeNotice([]string{"/Users/cole/my repo"}, primary) + require.Contains(t, got, "--from '/Users/cole/my repo' --yes") + }) + + t.Run("without a session still reports ambiguity but no command", func(t *testing.T) { + t.Parallel() + got := formatAmbiguousWorktreeNotice([]string{"/repo/a", "/repo/b"}, nil) + require.Contains(t, got, "/repo/a, /repo/b") + require.NotContains(t, got, "entire session adopt") + }) + + t.Run("no worktrees yields no message", func(t *testing.T) { + t.Parallel() + require.Empty(t, formatAmbiguousWorktreeNotice(nil, &SessionState{SessionID: "x", WorktreePath: "/repo/a"})) + }) +} + +// TestShellSingleQuote covers pasteability of paths with spaces and embedded +// single quotes. +func TestShellSingleQuote(t *testing.T) { + t.Parallel() + + require.Equal(t, "'/repo/a'", shellSingleQuote("/repo/a")) + require.Equal(t, "'/repo/my dir'", shellSingleQuote("/repo/my dir")) + require.Equal(t, `'/repo/it'\''s'`, shellSingleQuote("/repo/it's")) +} + +// TestMostRecentlyAdoptableSession verifies the source named in the remedy is +// the newest candidate adoption would actually accept. +func TestMostRecentlyAdoptableSession(t *testing.T) { + t.Parallel() + + older := time.Now().Add(-30 * time.Minute) + newer := time.Now().Add(-1 * time.Minute) + ended := time.Now() + + t.Run("picks newest adoptable", func(t *testing.T) { + t.Parallel() + got := mostRecentlyAdoptableSession([]*SessionState{ + {SessionID: "old", WorktreePath: "/a", LastInteractionTime: &older}, + {SessionID: "new", WorktreePath: "/b", LastInteractionTime: &newer}, + }) + require.NotNil(t, got) + require.Equal(t, "new", got.SessionID) + }) + + t.Run("skips ended, ended-at, and fully-condensed sessions", func(t *testing.T) { + t.Parallel() + got := mostRecentlyAdoptableSession([]*SessionState{ + {SessionID: "ended", WorktreePath: "/a", Phase: session.PhaseEnded, LastInteractionTime: &newer}, + {SessionID: "condensed", WorktreePath: "/b", FullyCondensed: true, LastInteractionTime: &newer}, + {SessionID: "endedat", WorktreePath: "/c", EndedAt: &ended, LastInteractionTime: &newer}, + {SessionID: "ok", WorktreePath: "/d", LastInteractionTime: &older}, + }) + require.NotNil(t, got) + require.Equal(t, "ok", got.SessionID) + }) + + t.Run("nil when none adoptable", func(t *testing.T) { + t.Parallel() + require.Nil(t, mostRecentlyAdoptableSession([]*SessionState{{SessionID: "ended", Phase: session.PhaseEnded}})) + require.Nil(t, mostRecentlyAdoptableSession(nil)) + }) +} diff --git a/cmd/entire/cli/strategy/manual_commit_session.go b/cmd/entire/cli/strategy/manual_commit_session.go index ced086b848..8e8fbaaa7e 100644 --- a/cmd/entire/cli/strategy/manual_commit_session.go +++ b/cmd/entire/cli/strategy/manual_commit_session.go @@ -257,6 +257,68 @@ func warnAmbiguousWorktreeSessions(ctx context.Context, worktreePath string, can slog.Int("candidate_sessions", len(candidates)), slog.Any("candidate_worktrees", worktrees), ) + + // logging.Warn only reaches the internal .entire/logs file, so on its own it + // leaves the #1852 case silent to the person running `git commit`. Surface a + // notice on stderrWriter too — the same user-facing hook channel + // warnIfAttributionDiverged and warnStaleEndedSessions use — with a runnable + // remedy. The command names a concrete session ID (the bare `--from ` + // form relies on adoption's 12h auto-detect, which errors here) and passes + // --yes (required for same-repo adoption). + fmt.Fprint(stderrWriter, formatAmbiguousWorktreeNotice(worktrees, mostRecentlyAdoptableSession(candidates))) +} + +// mostRecentlyAdoptableSession returns the newest-by-last-seen candidate that +// `entire session adopt` would accept (not ended, not fully condensed), or nil +// if none qualify. Mirrors isAdoptableSourceSession in session_adopt.go so the +// remedy we print is one adoption will actually run. +func mostRecentlyAdoptableSession(candidates []*SessionState) *SessionState { + var best *SessionState + for _, state := range candidates { + if state == nil || state.Phase == session.PhaseEnded || state.EndedAt != nil || state.FullyCondensed { + continue + } + if best == nil || ambiguousSessionLastSeen(state).After(ambiguousSessionLastSeen(best)) { + best = state + } + } + return best +} + +func ambiguousSessionLastSeen(state *SessionState) time.Time { + if state.LastInteractionTime != nil { + return *state.LastInteractionTime + } + return state.StartedAt +} + +// formatAmbiguousWorktreeNotice builds the user-facing stderr message. It always +// reports the ambiguity; when an adoptable session exists it appends a directly +// runnable adopt command. Returns "" when there are no worktrees to report. +func formatAmbiguousWorktreeNotice(worktrees []string, primary *SessionState) string { + if len(worktrees) == 0 { + return "" + } + notice := fmt.Sprintf( + "entire: this commit was not linked to a checkpoint (live agent sessions span multiple worktrees: %s).\n", + strings.Join(worktrees, ", "), + ) + if primary != nil && primary.SessionID != "" && primary.WorktreePath != "" { + // Shell-quote the worktree path: it can contain spaces or shell + // metacharacters, so an unquoted --from value would word-split or + // misbehave when the user copy-pastes this command. + notice += fmt.Sprintf( + " to link one explicitly, run: entire session adopt %s --from %s --yes\n", + primary.SessionID, shellSingleQuote(primary.WorktreePath), + ) + } + return notice +} + +// shellSingleQuote wraps s in single quotes so it pastes into a POSIX shell as a +// single literal argument, escaping any embedded single quotes. +func shellSingleQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" } // sessionsFromSingleWorktree returns the candidates only when they were all