diff --git a/cmd/logos/activity.go b/cmd/logos/activity.go index 46699ee..0a57671 100644 --- a/cmd/logos/activity.go +++ b/cmd/logos/activity.go @@ -284,6 +284,14 @@ func recordActivity(args []string) error { // once-per-session rule needs no second store. const kindCheckpointAsked = "checkpoint-asked" +// fullSessionID keeps only the host's uncut conversation id from e.Extra. +func fullSessionID(e activity.Event) map[string]any { + if full := activity.FullSessionID(e); full != "" { + return map[string]any{"session_id": full} + } + return nil +} + // askForCheckpoint prints Claude Code's stop-hook decision to block the stop // once, when this session changed files or ran commands after its last // checkpoint. A session that ends without one hands nothing to the next, and @@ -326,6 +334,10 @@ func askForCheckpoint(vault string, stop activity.Event, raw []byte) { Session: stop.Session, Agent: stop.Agent, Summary: "asked the agent to checkpoint before stopping", + // The marker is the newest event when the agent answers it, and the + // checkpoint names its conversation folder from the newest event: with + // only the cut Session it was filed beside the real folder, not in it. + Extra: fullSessionID(stop), }) != nil { return } diff --git a/cmd/logos/activity_compliance_test.go b/cmd/logos/activity_compliance_test.go index 31fcbab..20c79d7 100644 --- a/cmd/logos/activity_compliance_test.go +++ b/cmd/logos/activity_compliance_test.go @@ -59,3 +59,32 @@ func mustAppend(t *testing.T, vault string, e activity.Event) { t.Fatal(err) } } + +// The marker is the newest event when the agent answers the ask, and the +// checkpoint names its conversation from the newest event; with only the cut +// id on it, the answer was filed in a second folder for the same chat. +func TestTheStopHooksAskCarriesTheFullConversationID(t *testing.T) { + vault := t.TempDir() + if err := activity.SetRecording(vault, true); err != nil { + t.Fatal(err) + } + full := "b8ef6d21-141a-43b2-934d-1de0b746714c" + stop := activity.Event{Kind: activity.KindTurnEnd, Project: "logos", Session: "b8ef6d21", Agent: "claude-code", + Extra: map[string]any{"session_id": full}} + mustAppend(t, vault, activity.Event{TS: 1, Kind: activity.KindTool, Tool: "Edit", Project: "logos", Session: "b8ef6d21", Agent: "claude-code"}) + captureStdout(t, func() { askForCheckpoint(vault, stop, []byte(`{}`)) }) + + events, err := activity.Read(vault, activity.Query{Project: "logos"}) + if err != nil { + t.Fatal(err) + } + for _, e := range events { + if e.Kind == kindCheckpointAsked { + if got, _ := e.Extra["session_id"].(string); got != full { + t.Errorf("checkpoint-asked carries session_id %q, want %q", got, full) + } + return + } + } + t.Fatal("the stop hook did not ask for a checkpoint") +} diff --git a/cmd/logos/index.go b/cmd/logos/index.go index c8277bc..ff8fa82 100644 --- a/cmd/logos/index.go +++ b/cmd/logos/index.go @@ -286,10 +286,13 @@ func indexPass(ix *index.Index, w io.Writer, p *provider.Provider, embed string) // and touches nothing the counts above depend on. Announced for the same // reason every count above is: a rebuild that silently moved a file reads // as one that might have lost it. - if refiled, err := ix.SyncCheckpointHomes(); err != nil { + if refiled, rejoined, err := ix.SyncCheckpointHomes(); err != nil { fmt.Fprintln(os.Stderr, "· could not re-file checkpoints:", err) } else if refiled > 0 { fmt.Fprintf(w, "re-filed %d checkpoint%s to match host_session, 0 lost\n", refiled, plural(refiled, "")) + if rejoined > 0 { + fmt.Fprintf(w, " %d of them had been filed under a cut conversation id and rejoined their conversation\n", rejoined) + } } printConflictCopies(w, "", rep.ConflictCopies, edits) diff --git a/internal/activity/hook.go b/internal/activity/hook.go index 6ddeaed..4459c6d 100644 --- a/internal/activity/hook.go +++ b/internal/activity/hook.go @@ -72,8 +72,7 @@ func FromHook(event string, raw []byte, project string) (Event, error) { // itself under the conversation's host_session needs the id the host // actually gave, uncut, so it lands in the same folder a transcript-driven // checkpoint of the same conversation does. Carried in Extra, not a second - // top-level field, because nothing else here reads it — only - // mcpserver.hostSession. + // top-level field, and read back only through FullSessionID. if full := str(p, "session_id"); full != "" { e.Extra = map[string]any{"session_id": full} } @@ -237,3 +236,11 @@ func oneLine(s string, max int) string { func ShortID(s string) string { return text.Truncate(strings.TrimSpace(s), 8) } + +// FullSessionID is the host's uncut conversation id an event carries in Extra, +// or "" for one written without it. Every reader goes through here so the key +// a hook writes and the key a checkpoint files itself under cannot drift. +func FullSessionID(e Event) string { + full, _ := e.Extra["session_id"].(string) + return full +} diff --git a/internal/index/index.go b/internal/index/index.go index 57bbd47..4ca259e 100644 --- a/internal/index/index.go +++ b/internal/index/index.go @@ -265,8 +265,9 @@ func (ix *Index) SyncInsights() (restored, rescued int, err error) { // honest after an interrupted migration, a sync conflict copy, or a hand // move, the same way the rest of indexPass keeps the cache honest. // -// Returns how many checkpoints it moved. -func (ix *Index) SyncCheckpointHomes() (int, error) { +// Returns how many checkpoints it moved, and how many of those were rejoined +// to their conversation from a cut host_session. +func (ix *Index) SyncCheckpointHomes() (moved, rejoined int, err error) { return session.Refile(ix.Vault) } diff --git a/internal/mcpserver/continuity.go b/internal/mcpserver/continuity.go index 873d0a3..27f1fe2 100644 --- a/internal/mcpserver/continuity.go +++ b/internal/mcpserver/continuity.go @@ -457,8 +457,8 @@ func (s *Session) agentFor(args map[string]any) string { // eight characters for a human scanning the log, and a checkpoint filed under // the cut id would land in a different conversation folder than the full id // internal/ingest's transcript-driven path uses for the very same -// conversation. An older log line with no Extra falls back to the cut id — -// still wrong against a full-id writer, but no worse than before this fix. +// conversation. An older log line with no Extra falls back to the cut id; +// `logos index` rejoins a checkpoint filed that way (session.Refile). // // Only an event the checkpointing host itself logged counts. Only Claude // Code's hooks write this log, so a host with none (opencode, Codex without @@ -488,7 +488,7 @@ func (s *Session) hostSession(proj string) string { if e == nil { return "" } - if full, ok := e.Extra["session_id"].(string); ok && full != "" { + if full := activity.FullSessionID(*e); full != "" { return full } return e.Session diff --git a/internal/session/checkpoint.go b/internal/session/checkpoint.go index f486431..5557cfb 100644 --- a/internal/session/checkpoint.go +++ b/internal/session/checkpoint.go @@ -6,6 +6,7 @@ import ( "os" "path" "path/filepath" + "regexp" "sort" "strings" "time" @@ -642,27 +643,43 @@ func ByHostSession(vaultDir, hostSession string) ([]Checkpoint, error) { // between the two leaves the checkpoint readable twice — the next Refile call // finishes the move and removes the stale copy — never a checkpoint that // briefly existed nowhere. -func Refile(vaultDir string) (int, error) { +// +// It also rejoins a checkpoint filed under a cut id (see rejoinCutIDs): the +// second count is how many of the moved ones had their host_session rewritten. +func Refile(vaultDir string) (refiled, rejoined int, err error) { scopes, err := Scopes(vaultDir) if err != nil { - return 0, err + return 0, 0, err } - refiled := 0 for _, scope := range scopes { dir := filepath.Join(vaultDir, CheckpointDir, filepath.FromSlash(safeScope(scope))) entries, err := CheckpointEntries(dir) if err != nil { continue // one unreadable scope must not hide the others } - for _, en := range entries { - oldPath := filepath.Join(dir, filepath.FromSlash(en.Rel)) - raw, err := os.ReadFile(oldPath) + raws := make([][]byte, len(entries)) + parsed := make([]Checkpoint, len(entries)) + for i, en := range entries { + raw, err := os.ReadFile(filepath.Join(dir, filepath.FromSlash(en.Rel))) if err != nil { continue // unreadable file: nothing to refile, not a reason to stop } - c := ParseCheckpoint(string(raw)) - if c.Empty() { - continue // a torn write, not a misfiled checkpoint + raws[i], parsed[i] = raw, ParseCheckpoint(string(raw)) + } + fullOf := fullIDsByCut(parsed) + for i, en := range entries { + oldPath := filepath.Join(dir, filepath.FromSlash(en.Rel)) + raw, c := raws[i], parsed[i] + if raw == nil || c.Empty() { + continue // unreadable, or a torn write: not a misfiled checkpoint + } + wasCut := false + if full := fullOf[c.HostSession]; full != "" { + fixed, ok := replaceHostSession(raw, c.HostSession, full) + if !ok { + return refiled, rejoined, fmt.Errorf("re-filing %s: its host_session %q is a cut of %q, but the line could not be found to rewrite", en.Rel, c.HostSession, full) + } + raw, c.HostSession, wasCut = fixed, full, true } wantRel := filepath.ToSlash(filepath.Join(ConvosDir, convoFolder(c.HostSession), en.Name)) if en.Rel == wantRel { @@ -678,28 +695,97 @@ func Refile(vaultDir string) (int, error) { // the checkpoint in both folders. copies, err := vault.CopiesOf(filepath.Dir(oldPath), en.Name) if err != nil { - return refiled, fmt.Errorf("re-filing %s: listing its sync copies: %w", en.Rel, err) + return refiled, rejoined, fmt.Errorf("re-filing %s: listing its sync copies: %w", en.Rel, err) } for _, cp := range copies { if _, err := os.Lstat(filepath.Join(filepath.Dir(newPath), cp)); err == nil { - return refiled, fmt.Errorf("re-filing %s: its sync copy %s is already in %s, so neither was moved", en.Rel, cp, filepath.ToSlash(filepath.Dir(wantRel))) + return refiled, rejoined, fmt.Errorf("re-filing %s: its sync copy %s is already in %s, so neither was moved", en.Rel, cp, filepath.ToSlash(filepath.Dir(wantRel))) } } if err := vault.WriteAtomic(newPath, raw); err != nil { - return refiled, fmt.Errorf("re-filing %s: %w", en.Rel, err) + return refiled, rejoined, fmt.Errorf("re-filing %s: %w", en.Rel, err) } for _, cp := range copies { if err := os.Rename(filepath.Join(filepath.Dir(oldPath), cp), filepath.Join(filepath.Dir(newPath), cp)); err != nil { - return refiled, fmt.Errorf("re-filing %s: moving its sync copy %s: %w", en.Rel, cp, err) + return refiled, rejoined, fmt.Errorf("re-filing %s: moving its sync copy %s: %w", en.Rel, cp, err) } } if err := os.Remove(oldPath); err != nil { - return refiled, fmt.Errorf("re-filing %s: removing the old copy: %w", en.Rel, err) + return refiled, rejoined, fmt.Errorf("re-filing %s: removing the old copy: %w", en.Rel, err) + } + // A conversation folder emptied by the move would otherwise sit + // there as a stray. os.Remove refuses a non-empty directory, so + // this cannot take anything with it. + if strings.HasPrefix(en.Rel, ConvosDir+"/") { + _ = os.Remove(filepath.Dir(oldPath)) } refiled++ + if wasCut { + rejoined++ + } + } + } + return refiled, rejoined, nil +} + +// Before 0.5.8 the Stop hook's checkpoint-asked marker carried only the +// activity log's 8-character cut of the host's id, and a checkpoint answering +// it was filed under that cut, beside its conversation's real folder: one chat +// read as two everywhere a host_session is grouped on. The frontmatter is the +// truth, so the repair is to the frontmatter. +// +// Only a UUID-shaped full id is a cut's source (Claude Code's and Codex's ids +// both are), and only when exactly one in the scope starts with it: a real +// 8-character id from another host, or two conversations sharing a prefix, +// is left as written rather than merged on a guess. +var ( + cutIDShape = regexp.MustCompile(`^[0-9a-f]{8}$`) + fullIDShape = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`) +) + +// fullIDsByCut maps each cut id some checkpoint here is filed under to the one +// full id it was cut from. +func fullIDsByCut(cps []Checkpoint) map[string]string { + cuts := map[string]bool{} + fulls := map[string]map[string]bool{} + for _, c := range cps { + switch { + case cutIDShape.MatchString(c.HostSession): + cuts[c.HostSession] = true + case fullIDShape.MatchString(c.HostSession): + p := c.HostSession[:8] + if fulls[p] == nil { + fulls[p] = map[string]bool{} + } + fulls[p][c.HostSession] = true + } + } + out := map[string]string{} + for cut := range cuts { + if len(fulls[cut]) != 1 { + continue + } + for full := range fulls[cut] { + out[cut] = full } } - return refiled, nil + return out +} + +// replaceHostSession rewrites the host_session line inside the frontmatter +// only, so a body that happens to quote the line is left alone. +func replaceHostSession(raw []byte, from, to string) ([]byte, bool) { + s := string(raw) + end := strings.Index(s, "\n---") + if !strings.HasPrefix(s, "---\n") || end < 0 { + return nil, false + } + line := "\nhost_session: " + yamlStr(from) + "\n" + i := strings.Index(s[:end+1], line) + if i < 0 { + return nil, false + } + return []byte(s[:i] + "\nhost_session: " + yamlStr(to) + "\n" + s[i+len(line):]), true } // ResolvePath finds a checkpoint a stored citation names, wherever it diff --git a/internal/session/host_session_test.go b/internal/session/host_session_test.go index 8c003d3..7f257da 100644 --- a/internal/session/host_session_test.go +++ b/internal/session/host_session_test.go @@ -149,7 +149,7 @@ func TestAMisfiledCheckpointIsReFiledFromItsFrontmatterAndAnnounced(t *testing.T t.Fatal(err) } - n, err := Refile(dir) + n, _, err := Refile(dir) if err != nil { t.Fatal(err) } @@ -164,7 +164,7 @@ func TestAMisfiledCheckpointIsReFiledFromItsFrontmatterAndAnnounced(t *testing.T } // A second pass over an already-correct vault finds nothing left to move. - n, err = Refile(dir) + n, _, err = Refile(dir) if err != nil { t.Fatal(err) } @@ -265,7 +265,7 @@ func TestReFilingACheckpointTakesItsSyncCopiesWithIt(t *testing.T) { } } - if _, err := Refile(dir); err != nil { + if _, _, err := Refile(dir); err != nil { t.Fatal(err) } entries, err := CheckpointEntries(scope) @@ -284,3 +284,63 @@ func TestReFilingACheckpointTakesItsSyncCopiesWithIt(t *testing.T) { } } } + +// Before 0.5.8 a checkpoint the Stop hook asked for was filed under the +// activity log's 8-character cut of the host's id, beside its conversation's +// real folder, and every reader that groups on host_session — the resume +// picker, ByHostSession, the cost report — showed one chat as two. +func TestRefileRejoinsACheckpointFiledUnderTheCutID(t *testing.T) { + dir := t.TempDir() + full := "b8ef6d21-141a-43b2-934d-1de0b746714c" + commitIn(t, dir, &Checkpoint{Project: "kestrel", Agent: "a", Task: "fix checkout", HostSession: full}) + cut := commitIn(t, dir, &Checkpoint{Project: "kestrel", Agent: "a", Task: "fix checkout", State: "asked to stop", HostSession: "b8ef6d21"}) + commitIn(t, dir, &Checkpoint{Project: "kestrel", Agent: "a", Task: "speed up search", HostSession: "c0ffee00"}) + + moved, rejoined, err := Refile(dir) + if err != nil { + t.Fatal(err) + } + if moved != 1 || rejoined != 1 { + t.Fatalf("want 1 moved and 1 rejoined, got %d and %d", moved, rejoined) + } + if _, err := os.Stat(filepath.Dir(cut)); !os.IsNotExist(err) { + t.Fatalf("the cut-id folder is still there: %v", err) + } + got, err := ByHostSession(dir, full) + if err != nil { + t.Fatal(err) + } + if len(got) != 2 { + t.Fatalf("want both checkpoints under the full id, got %d", len(got)) + } + convs, err := Conversations(dir, "kestrel") + if err != nil { + t.Fatal(err) + } + if len(convs) != 2 { + t.Fatalf("want 2 conversations, got %d", len(convs)) + } + + if moved, rejoined, err = Refile(dir); err != nil || moved != 0 || rejoined != 0 { + t.Fatalf("a second pass changed something: %d moved, %d rejoined, %v", moved, rejoined, err) + } +} + +// A cut is only a guess at its source. Two conversations sharing the prefix, +// or a host whose real ids are eight characters, must be left as written. +func TestRefileLeavesACutIDAloneUnlessExactlyOneConversationOwnsIt(t *testing.T) { + dir := t.TempDir() + commitIn(t, dir, &Checkpoint{Project: "kestrel", Agent: "a", Task: "one", HostSession: "b8ef6d21-141a-43b2-934d-1de0b746714c"}) + commitIn(t, dir, &Checkpoint{Project: "kestrel", Agent: "a", Task: "two", HostSession: "b8ef6d21-0000-4000-8000-000000000000"}) + commitIn(t, dir, &Checkpoint{Project: "kestrel", Agent: "a", Task: "three", HostSession: "b8ef6d21"}) + commitIn(t, dir, &Checkpoint{Project: "kestrel", Agent: "a", Task: "four", HostSession: "c0ffee00-not-a-uuid"}) + commitIn(t, dir, &Checkpoint{Project: "kestrel", Agent: "a", Task: "five", HostSession: "c0ffee00"}) + + moved, rejoined, err := Refile(dir) + if err != nil { + t.Fatal(err) + } + if moved != 0 || rejoined != 0 { + t.Fatalf("want nothing touched, got %d moved and %d rejoined", moved, rejoined) + } +}