Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions cmd/entire/cli/checkpoint/checkpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,14 @@ type WriteCommittedOptions struct {
// session.Kind.IsReview) because checkpoint can't import session
// — the session package imports checkpoint, creating a cycle.
HasReview bool

// AttachReason names which decision branch in PostCommit caused this
// session to be attached to the checkpoint (e.g. "active_recent_interaction",
// "file_overlap"). Empty for callers that don't track this (e.g. doctor
// backfills) or for legacy writers. Stored as a plain string here because
// the checkpoint package cannot import session (cycle); the typed enum
// lives in session.AttachReason.
AttachReason string
}

// UpdateCommittedOptions contains options for updating an existing committed checkpoint.
Expand Down Expand Up @@ -517,6 +525,14 @@ type CommittedMetadata struct {
// for spawn, first user prompt for attach). Only set when Kind is a
// review kind.
ReviewPrompt string `json:"review_prompt,omitempty"`

// AttachReason names which PostCommit decision branch caused this session
// to be attached to the checkpoint — e.g. "active_recent_interaction"
// or "file_overlap". Useful for diagnosing why a particular session
// shows up under a checkpoint, especially when multiple sessions share
// one. Empty for checkpoints written by older CLI versions or by code
// paths that don't track this (e.g. doctor-driven backfill).
AttachReason string `json:"attach_reason,omitempty"`
}

// GetTranscriptStart returns the transcript line offset at which this checkpoint's data begins.
Expand Down
71 changes: 71 additions & 0 deletions cmd/entire/cli/checkpoint/checkpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4331,6 +4331,77 @@ func TestCommittedMetadata_ReviewFields(t *testing.T) {
}
}

// TestWriteCommitted_PersistsAttachReason verifies that the AttachReason
// passed via WriteCommittedOptions actually lands on the persisted
// metadata.json and reads back through ReadSessionContent — the diagnostic
// is only useful if it survives the full Write → Read path.
func TestWriteCommitted_PersistsAttachReason(t *testing.T) {
t.Parallel()
repo, _ := setupBranchTestRepo(t)
store := NewGitStore(repo)
checkpointID := id.MustCheckpointID("aa11bb22cc33")

err := store.WriteCommitted(context.Background(), WriteCommittedOptions{
CheckpointID: checkpointID,
SessionID: "session-attach",
Strategy: "manual-commit",
Transcript: redact.AlreadyRedacted([]byte(`{"message": "attached"}`)),
FilesTouched: []string{"a.go"},
CheckpointsCount: 1,
AuthorName: "Test Author",
AuthorEmail: "test@example.com",
AttachReason: "file_overlap",
})
if err != nil {
t.Fatalf("WriteCommitted() error = %v", err)
}

content, err := store.ReadSessionContent(context.Background(), checkpointID, 0)
if err != nil {
t.Fatalf("ReadSessionContent() error = %v", err)
}
if content.Metadata.AttachReason != "file_overlap" {
t.Errorf("AttachReason = %q, want %q", content.Metadata.AttachReason, "file_overlap")
}
}

// TestCommittedMetadata_AttachReason pins the JSON wire format for the
// AttachReason field on CommittedMetadata. The whole point of persisting
// the field is that consumers can read it back from metadata.json, so a
// silent rename of the struct field or its JSON tag would break the
// diagnostic — even a self-consistent round-trip wouldn't catch that.
// Also asserts omitempty: older checkpoints without the field must not
// gain a misleading "attach_reason":"" key.
func TestCommittedMetadata_AttachReason(t *testing.T) {
t.Parallel()

// Populated case: key marshals as "attach_reason":"manual_attach".
bSet, err := json.Marshal(CommittedMetadata{AttachReason: "manual_attach"})
if err != nil {
t.Fatalf("marshal set: %v", err)
}
var rawSet map[string]any
if err := json.Unmarshal(bSet, &rawSet); err != nil {
t.Fatalf("unmarshal set: %v", err)
}
if got, ok := rawSet["attach_reason"].(string); !ok || got != "manual_attach" {
t.Errorf(`expected "attach_reason":"manual_attach", got %v (raw: %s)`, rawSet["attach_reason"], string(bSet))
}

// Empty case: omitempty must drop the key for legacy/zero-value metadata.
bZero, err := json.Marshal(CommittedMetadata{})
if err != nil {
t.Fatalf("marshal zero: %v", err)
}
var rawZero map[string]any
if err := json.Unmarshal(bZero, &rawZero); err != nil {
t.Fatalf("unmarshal zero: %v", err)
}
if _, present := rawZero["attach_reason"]; present {
t.Errorf(`expected "attach_reason" to be omitted when empty, got %v (raw: %s)`, rawZero["attach_reason"], string(bZero))
}
}

// TestCheckpointSummary_HasReview pins the JSON wire format for the HasReview
// umbrella flag on CheckpointSummary. Callers such as the re-run guard in
// `entire review` and `entire status` depend on the on-disk shape, so we
Expand Down
1 change: 1 addition & 0 deletions cmd/entire/cli/checkpoint/committed.go
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,7 @@ func (s *GitStore) writeSessionToSubdirectory(ctx context.Context, opts WriteCom
Kind: opts.Kind,
ReviewSkills: opts.ReviewSkills,
ReviewPrompt: opts.ReviewPrompt,
AttachReason: opts.AttachReason,
}

metadataJSON, err := jsonutil.MarshalIndentWithNewline(sessionMetadata, "", " ")
Expand Down
1 change: 1 addition & 0 deletions cmd/entire/cli/checkpoint/v2_committed.go
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,7 @@ func (s *V2GitStore) writeMainSessionToSubdirectory(opts WriteCommittedOptions,
Kind: opts.Kind,
ReviewSkills: opts.ReviewSkills,
ReviewPrompt: opts.ReviewPrompt,
AttachReason: opts.AttachReason,
}

metadataJSON, err := jsonutil.MarshalIndentWithNewline(sessionMetadata, "", " ")
Expand Down
49 changes: 49 additions & 0 deletions cmd/entire/cli/session/attach_reason.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package session

// AttachReason describes why a session was (or was not) attached to a
// checkpoint during PostCommit condensation. Attach reasons are persisted
// on CommittedMetadata; skip reasons are log-only.
type AttachReason string

const (
// AttachReasonActiveRecentInteraction: ACTIVE session within the
// activeSessionInteractionThreshold. PrepareCommitMsg already established
// commit relatedness via the trailer, so no overlap check is required.
AttachReasonActiveRecentInteraction AttachReason = "active_recent_interaction"

// AttachReasonFileOverlap: stale ACTIVE or IDLE/ENDED, attached because
// tracked files overlap the committed file set and contents match the
// shadow branch.
AttachReasonFileOverlap AttachReason = "file_overlap"

// AttachReasonManual: session was imported via `entire attach` (or
// `entire review attach`). Bypasses heuristic gates — user intent wins.
AttachReasonManual AttachReason = "manual_attach"

AttachSkipNoNewContent AttachReason = "skip_no_new_content"

// AttachSkipReadOnlyActive: avoids attaching read-only agents (e.g. a
// summarize tool) to commits owned by a different session.
AttachSkipReadOnlyActive AttachReason = "skip_read_only_active"

AttachSkipNoTrackedFiles AttachReason = "skip_no_tracked_files"
AttachSkipNoCommittedOverlap AttachReason = "skip_no_committed_overlap"
AttachSkipContentMismatch AttachReason = "skip_content_mismatch"
)

// IsAttach reports whether this reason resulted in the session being
// attached to the checkpoint. New AttachReason values must be added to one
// of the cases below — the exhaustive linter will flag forgotten additions.
func (r AttachReason) IsAttach() bool {
switch r {
case AttachReasonActiveRecentInteraction, AttachReasonFileOverlap, AttachReasonManual:
return true
case AttachSkipNoNewContent,
AttachSkipReadOnlyActive,
AttachSkipNoTrackedFiles,
AttachSkipNoCommittedOverlap,
AttachSkipContentMismatch:
return false
}
return false
}
16 changes: 9 additions & 7 deletions cmd/entire/cli/strategy/manual_commit_condensation.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,14 @@ func (s *ManualCommitStrategy) getCheckpointLog(ctx context.Context, checkpointI

// condenseOpts provides pre-resolved git objects to avoid redundant reads.
type condenseOpts struct {
shadowRef *plumbing.Reference // Pre-resolved shadow branch ref (nil = resolve from repo)
headTree *object.Tree // Pre-resolved HEAD tree (passed through to calculateSessionAttributions)
parentTree *object.Tree // Pre-resolved parent tree (nil for initial commits, for consistent non-agent line counting)
repoDir string // Repository worktree path for git CLI commands
parentCommitHash string // HEAD's first parent hash for per-commit non-agent file detection
headCommitHash string // HEAD commit hash (passed through for attribution)
allAgentFiles map[string]struct{} // Union of all sessions' FilesTouched for cross-session exclusion (nil = single-session)
shadowRef *plumbing.Reference // Pre-resolved shadow branch ref (nil = resolve from repo)
headTree *object.Tree // Pre-resolved HEAD tree (passed through to calculateSessionAttributions)
parentTree *object.Tree // Pre-resolved parent tree (nil for initial commits, for consistent non-agent line counting)
repoDir string // Repository worktree path for git CLI commands
parentCommitHash string // HEAD's first parent hash for per-commit non-agent file detection
headCommitHash string // HEAD commit hash (passed through for attribution)
allAgentFiles map[string]struct{} // Union of all sessions' FilesTouched for cross-session exclusion (nil = single-session)
attachReason session.AttachReason // Why this session was attached to the checkpoint (persisted on CommittedMetadata; empty = not set)
}

var redactSessionJSONLBytes = redact.JSONLBytes
Expand Down Expand Up @@ -250,6 +251,7 @@ func (s *ManualCommitStrategy) CondenseSession(ctx context.Context, repo *git.Re
ReviewSkills: state.ReviewSkills,
ReviewPrompt: state.ReviewPrompt,
HasReview: state.Kind.IsReview(),
AttachReason: string(o.attachReason),
}

compactResult := buildExternalCompactTranscript(ctx, ag, state)
Expand Down
92 changes: 54 additions & 38 deletions cmd/entire/cli/strategy/manual_commit_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -689,16 +689,8 @@ func (h *postCommitActionHandler) parentCommitHash() string {
}

func (h *postCommitActionHandler) HandleCondense(state *session.State) error {
logCtx := logging.WithComponent(h.ctx, "checkpoint")
shouldCondense := h.shouldCondenseWithOverlapCheck(state.Phase.IsActive(), state.LastInteractionTime)

logging.Debug(logCtx, "post-commit: HandleCondense decision",
slog.String("session_id", state.SessionID),
slog.String("phase", string(state.Phase)),
slog.Bool("has_new", h.hasNew),
slog.Bool("should_condense", shouldCondense),
slog.String("shadow_branch", h.shadowBranchName),
)
shouldCondense, reason := h.shouldCondenseWithOverlapCheck(state)
h.logAttachDecision(state, "HandleCondense", shouldCondense, reason, len(state.FilesTouched))

if shouldCondense {
h.condensed = h.s.condenseAndUpdateState(h.ctx, h.repo, h.checkpointID, state, h.head, h.shadowBranchName, h.shadowBranchesToDelete, h.committedFileSet, condenseOpts{
Expand All @@ -709,6 +701,7 @@ func (h *postCommitActionHandler) HandleCondense(state *session.State) error {
parentCommitHash: h.parentCommitHash(),
headCommitHash: h.newHead,
allAgentFiles: h.allAgentFiles,
attachReason: reason,
})
} else {
h.s.updateBaseCommitIfChanged(h.ctx, state, h.newHead)
Expand All @@ -717,17 +710,19 @@ func (h *postCommitActionHandler) HandleCondense(state *session.State) error {
}

func (h *postCommitActionHandler) HandleCondenseIfFilesTouched(state *session.State) error {
logCtx := logging.WithComponent(h.ctx, "checkpoint")
shouldCondense := len(state.FilesTouched) > 0 && h.shouldCondenseWithOverlapCheck(state.Phase.IsActive(), state.LastInteractionTime)

logging.Debug(logCtx, "post-commit: HandleCondenseIfFilesTouched decision",
slog.String("session_id", state.SessionID),
slog.String("phase", string(state.Phase)),
slog.Bool("has_new", h.hasNew),
slog.Int("files_touched", len(state.FilesTouched)),
slog.Bool("should_condense", shouldCondense),
slog.String("shadow_branch", h.shadowBranchName),
// Manually-attached sessions skip the files-touched gate — review attach
// in particular may legitimately have no files touched, and the user's
// import is the explicit signal we honor in place of file evidence.
var (
shouldCondense bool
reason session.AttachReason
)
if state.AttachedManually || len(state.FilesTouched) > 0 {
shouldCondense, reason = h.shouldCondenseWithOverlapCheck(state)
} else {
reason = session.AttachSkipNoTrackedFiles
}
h.logAttachDecision(state, "HandleCondenseIfFilesTouched", shouldCondense, reason, len(state.FilesTouched))
Comment thread
Soph marked this conversation as resolved.

if shouldCondense {
h.condensed = h.s.condenseAndUpdateState(h.ctx, h.repo, h.checkpointID, state, h.head, h.shadowBranchName, h.shadowBranchesToDelete, h.committedFileSet, condenseOpts{
Expand All @@ -738,21 +733,38 @@ func (h *postCommitActionHandler) HandleCondenseIfFilesTouched(state *session.St
parentCommitHash: h.parentCommitHash(),
headCommitHash: h.newHead,
allAgentFiles: h.allAgentFiles,
attachReason: reason,
})
} else {
h.s.updateBaseCommitIfChanged(h.ctx, state, h.newHead)
}
return nil
}

// shouldCondenseWithOverlapCheck returns true if the session should be condensed
// into this commit. Active sessions with recent interaction condense unless they
// have no tracked files and another session claims the committed files (read-only
// gate). Stale ACTIVE and IDLE/ENDED sessions require file overlap evidence
// between tracked files and committed files.
func (h *postCommitActionHandler) shouldCondenseWithOverlapCheck(isActive bool, lastInteraction *time.Time) bool {
func (h *postCommitActionHandler) logAttachDecision(state *session.State, trigger string, attached bool, reason session.AttachReason, filesTouched int) {
logCtx := logging.WithComponent(h.ctx, "checkpoint")
logging.Debug(logCtx, "post-commit: attach decision",
slog.String("session_id", state.SessionID),
slog.String("phase", string(state.Phase)),
slog.String("trigger", trigger),
slog.Bool("attached", attached),
slog.String("attach_reason", string(reason)),
slog.Bool("has_new", h.hasNew),
slog.Int("files_touched", filesTouched),
slog.String("shadow_branch", h.shadowBranchName),
)
}

// shouldCondenseWithOverlapCheck returns whether the session should be
// condensed into this commit, along with the AttachReason that named the
// decision. Manually attached sessions bypass the heuristic branches —
// the user's import is a stronger signal than file-content evidence.
func (h *postCommitActionHandler) shouldCondenseWithOverlapCheck(state *session.State) (bool, session.AttachReason) {
if !h.hasNew {
return false
return false, session.AttachSkipNoNewContent
}
if state.AttachedManually {
return true, session.AttachReasonManual
}
// ACTIVE sessions with recent interaction: skip the overlap check.
// PrepareCommitMsg already validated this commit is session-related
Expand All @@ -769,17 +781,14 @@ func (h *postCommitActionHandler) shouldCondenseWithOverlapCheck(isActive bool,
// We check LastInteractionTime to avoid condensing stale ACTIVE sessions
// (agent killed without Stop hook) into every subsequent commit. A stale
// session has no recent interaction and falls through to the overlap check.
if isActive && isRecentInteraction(lastInteraction) {
if state.Phase.IsActive() && isRecentInteraction(state.LastInteractionTime) {
if h.sessionsWithCommittedFiles > 0 && len(h.filesTouchedBefore) == 0 {
logging.Debug(h.ctx, "post-commit: skipping read-only ACTIVE session (no tracked files, other sessions claim committed files)",
slog.Int("sessions_with_committed_files", h.sessionsWithCommittedFiles),
)
return false
return false, session.AttachSkipReadOnlyActive
}
return true
return true, session.AttachReasonActiveRecentInteraction
}
if len(h.filesTouchedBefore) == 0 {
return false // No files tracked = no overlap evidence
return false, session.AttachSkipNoTrackedFiles
}
// Only check files that were actually changed in this commit.
// Without this, files that exist in the tree but weren't changed
Expand All @@ -793,14 +802,17 @@ func (h *postCommitActionHandler) shouldCondenseWithOverlapCheck(isActive bool,
}
}
if len(committedTouchedFiles) == 0 {
return false
return false, session.AttachSkipNoCommittedOverlap
}
return filesOverlapWithContent(h.ctx, h.repo, h.shadowBranchName, h.commit, committedTouchedFiles, overlapOpts{
if filesOverlapWithContent(h.ctx, h.repo, h.shadowBranchName, h.commit, committedTouchedFiles, overlapOpts{
headTree: h.headTree,
shadowTree: h.shadowTree,
parentTree: h.parentTree,
hasParentTree: true,
})
}) {
return true, session.AttachReasonFileOverlap
}
return false, session.AttachSkipContentMismatch
}

const (
Expand Down Expand Up @@ -1225,7 +1237,11 @@ func (s *ManualCommitStrategy) postCommitProcessSessionLocked(
)
}
}
transitionCtx.HasFilesTouched = len(state.FilesTouched) > 0
// Manually attached sessions are routed through the condense action even
// without tracked files: the user's `entire attach` import is the signal
// we honor in place of file overlap. shouldCondenseWithOverlapCheck then
// short-circuits to AttachReasonManual once it sees AttachedManually.
transitionCtx.HasFilesTouched = len(state.FilesTouched) > 0 || state.AttachedManually

// Save FilesTouched BEFORE TransitionAndLog — the handler's condensation
// clears it, but we need the original list for carry-forward computation.
Expand Down
Loading
Loading