Skip to content

fix(strategy): surface ambiguous-worktree warning to the user with a runnable remedy. - #1859

Open
ColeDriver wants to merge 1 commit into
mainfrom
fix/ambiguous-worktree-warning-visibility
Open

fix(strategy): surface ambiguous-worktree warning to the user with a runnable remedy.#1859
ColeDriver wants to merge 1 commit into
mainfrom
fix/ambiguous-worktree-warning-visibility

Conversation

@ColeDriver

@ColeDriver ColeDriver commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

What & why

#1856 added warnAmbiguousWorktreeSessions for the #1852 case where live sessions span multiple worktrees, matching refuses to guess, and 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.

Change

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 <session-id> --from ../a --yes' to link one explicitly.

The command names a concrete session ID rather than the bare --from <path> form, because bare --from relies on adoption's auto-detect, which only considers sessions within adoptRecentWindow (12h) and fails outright when a worktree has multiple sessions; same-repo adoption also requires --yes. The named session is the most-recently-active candidate adoption will accept (mirrors isAdoptableSourceSession), so the printed command actually runs.

Scope

Additive only — the warning trigger and the existing log line are unchanged; this just makes the already-detected case visible and actionable. If reviewers want the stderr line throttled (show-once / rate-limited), happy to add that.

Tests

  • New table-driven unit tests (t.Parallel()): formatAmbiguousWorktreeNotice (with/without an adoptable session, empty input) and mostRecentlyAdoptableSession (newest-adoptable, skips ended/ended-at/fully-condensed, nil when none).
  • fix(strategy): warn when ambiguous worktree sessions block commit linking #1856's existing ambiguous-worktree tests still pass; full strategy suite green; maintidx + golangci-lint v2.11.3 and gofmt clean.

Refs #1852 · follow-up to #1856

🤖 Generated with Claude Code

@ColeDriver
ColeDriver requested a review from a team as a code owner July 26, 2026 05:38
…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 <session-id> --from ../a --yes' to link one explicitly.

The command names a concrete session ID rather than the bare
`--from <path>` 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 <noreply@anthropic.com>
@ColeDriver
ColeDriver force-pushed the fix/ambiguous-worktree-warning-visibility branch from b7df895 to 4ff48cf Compare July 26, 2026 05:47
@gtrrz-victor

gtrrz-victor commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

⚠️ The notice is swallowed by the hook — it never reaches the user

I tested this end-to-end against a dev:publish build of this branch and the warning does not surface on a real git commit, so the PR's stated goal ("the person running git commit still sees nothing" → make it visible) isn't met as written.

Root cause

The notice is written to os.Stderr (via stderrWriter), but every commit-side hook the CLI installs invokes entire with 2>/dev/null (cmd/entire/cli/strategy/hooks.go:178-181):

entire hooks git prepare-commit-msg "$1" "$2" 2>/dev/null || true
entire hooks git post-commit           2>/dev/null || true
entire hooks git post-rewrite "$1"     2>/dev/null || true

Git's hook script discards stderr, so fmt.Fprint(stderrWriter, …) output is thrown away before it reaches the terminal.

Reproduction (proven both ways)

Setup: repo with two sibling worktrees A and B, a live (non-ended) session recorded in each, then commit from a third worktree that has no session of its own → ambiguous fallback → warn path.

  • Real git commit (shipped hook, with 2>/dev/null) → no warning printed, commit succeeds silently.

  • Same commit with 2>/dev/null removed from the installed prepare-commit-msg hook → warning prints exactly as designed:

    entire: this commit was not linked to a checkpoint (live agent sessions span multiple worktrees: …/A, …/B).
      to link one explicitly, run: entire session adopt sess-a --from '…/A' --yes
    
  • Running the hook binary directly without the redirect (entire hooks git prepare-commit-msg <msg> "") also prints it. So the formatting/remedy logic is correct — the delivery channel is the problem.

Note also: the trigger on a normal commit is prepare-commit-msg (manual_commit_hooks.go:407), not post-commitPostCommit only calls findSessionsForWorktree after it finds an Entire-Checkpoint trailer (:910-915 early-returns otherwise).

On the "same channel" justification

The PR notes it uses "the same user-facing hook channel warnIfAttributionDiverged and warnStaleEndedSessions use." Those fire from PrepareCommitMsg:420 and PostCommit:1048 — both 2>/dev/null hooks — so they're swallowed too. The precedent doesn't actually reach the user either; it's the same latent gap.

🤖 Reviewed with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants