From 24a7db352dd216a2a4872459c632e40507ea18e2 Mon Sep 17 00:00:00 2001 From: Marius van Niekerk Date: Wed, 24 Jun 2026 21:34:07 -0400 Subject: [PATCH] feat(parser): migrate pi provider Pi is the next JSONL-shaped parser that can move behind the provider facade without introducing a new source framework. Its source layout is still simple enough to compose the directory JSONL helper, but it needs provider-owned filtering because legacy discovery validates the session header while raw session lookup only checks the expected filename under encoded-cwd directories. This keeps that discovery-versus-lookup asymmetry explicit in the provider and preserves symlinked encoded-cwd directory support while parse output continues to come from the existing Pi parser. Validation: go test -tags "fts5" ./internal/parser -run TestPiProvider -count=1; go test -tags "fts5" ./internal/parser -count=1; go vet ./...; make test-short; git diff --check fix(parser): preserve pi header-based discovery Pi discovery has historically treated the filename as source shape only: any one-level JSONL file under an encoded-cwd directory can be a session if its header has type=session. The provider migration accidentally applied raw session ID filename validation before header validation, which would drop valid files whose session ID comes from the header instead of the filename. Raw-ID lookup still validates the requested ID before reconstructing .jsonl, so the legacy discovery-versus-lookup asymmetry remains explicit without broadening lookup inputs. Validation: go test -tags "fts5" ./internal/parser -run TestPiProviderDiscoveryAcceptsSessionHeaderInNonSessionIDFilename -count=1; go test -tags "fts5" ./internal/parser -run 'TestPiProvider(DiscoveryAcceptsSessionHeaderInNonSessionIDFilename|SourceMethods|Parse|DiscoversSymlinkedCWDDirectory|FactoryReplacesLegacyAdapter)' -count=1; go test -tags "fts5" ./internal/parser -count=1; go vet ./...; make test-short; git diff --check test(parser): opt pi into provider shadow Pi now has a concrete facade provider on this branch, so its migration mode should enter the shared shadow-compare harness instead of remaining an additive implementation behind legacy-only dispatch. The stack keeps lower provider opt-ins inherited and leaves later provider branches legacy-only until their own migrations land. Validation: go test -tags "fts5" ./internal/parser -run TestProviderMigrationModes -count=1; go test -tags "fts5" ./internal/parser -count=1; go vet ./...; git diff --check test(sync): compare pi shadow parity Pi is shadow-compared on this branch, so add the shared source-level proof that provider observation matches ParsePiSession output for a representative session file. Validation: go fmt ./...; go test -tags "fts5" ./internal/parser ./internal/sync -count=1; go vet ./...; git diff --check; ./custom-gcl run --config .golangci.nilaway.yml ./internal/parser/... ./internal/sync/... refactor(parser): fold pi into provider Pi should not keep exported parser and source callback APIs after its concrete provider exists. Removing those hooks also exposed that full sync and single-session lookup still assumed AgentDef callbacks, so provider-authoritative agents were not actually runnable without legacy callbacks. Move Pi parsing behind the provider, remove its legacy discovery and sync dispatch, add provider discovery and provider lookup to the sync root path, and replace shadow-baseline coverage with provider API tests plus a guard that the old symbols stay gone. Validation: go test -tags "fts5" ./internal/parser ./internal/sync ./cmd/agentsview -count=1; go vet ./...; git diff --check fix(parser): preserve pi family provider capabilities OMP shared the Pi on-disk format but was left legacy-only after the legacy registry hooks were removed, so full sync and changed-path sync could no longer reach it through the migrated provider path. Parse-diff had the same shape of regression for provider-authoritative agents because it only trusted AgentDef discovery callbacks.\n\nFold OMP into the concrete Pi-family provider, derive parse identity from the provider definition, and teach parse-diff plus CLI validation to accept provider-authoritative on-disk sources. This keeps the branch as an actual migration rather than a shim around removed legacy functions.\n\nValidation: go test -tags "fts5" ./internal/parser -count=1; go test -tags "fts5" ./cmd/agentsview -run 'TestParseDiff' -count=1; go test -tags "fts5" ./internal/sync -run 'Test(ParseDiff|OMPSyncAllAndChangedPathUseProvider)' -count=1; go test -tags "fts5" ./internal/sync -count=1; go vet ./...; git diff --check fix(parser): thread ctx through pi source lookups --- cmd/agentsview/parse_diff.go | 23 +- cmd/agentsview/parse_diff_test.go | 10 + internal/parser/discovery.go | 94 -------- internal/parser/pi.go | 18 +- internal/parser/pi_provider.go | 228 ++++++++++++++++++++ internal/parser/pi_provider_test.go | 227 +++++++++++++++++++ internal/parser/pi_test.go | 196 ++++++++--------- internal/parser/provider.go | 2 + internal/parser/provider_migration.go | 4 +- internal/parser/types.go | 32 ++- internal/sync/engine.go | 67 ------ internal/sync/engine_integration_test.go | 39 ++++ internal/sync/parsediff.go | 83 ++++++- internal/sync/parsediff_integration_test.go | 29 +++ 14 files changed, 741 insertions(+), 311 deletions(-) create mode 100644 internal/parser/pi_provider.go create mode 100644 internal/parser/pi_provider_test.go diff --git a/cmd/agentsview/parse_diff.go b/cmd/agentsview/parse_diff.go index d628370ed..c9c9d8248 100644 --- a/cmd/agentsview/parse_diff.go +++ b/cmd/agentsview/parse_diff.go @@ -238,7 +238,7 @@ func parseDiffAgentTypes(names []string) ([]parser.AgentType, error) { strings.Join(parseDiffSupportedAgents(), ", "), ) } - if !def.FileBased || def.DiscoverFunc == nil { + if !parseDiffAgentSupported(def) { return nil, fmt.Errorf( "agent %q is not supported by parse-diff "+ "(no on-disk source to re-parse)", @@ -253,18 +253,33 @@ func parseDiffAgentTypes(names []string) ([]parser.AgentType, error) { return out, nil } -// parseDiffSupportedAgents lists the agent types parse-diff can -// re-parse: file-based agents with a discovery function. +// parseDiffSupportedAgents lists the agent types parse-diff can re-parse. func parseDiffSupportedAgents() []string { var names []string for _, def := range parser.Registry { - if def.FileBased && def.DiscoverFunc != nil { + if parseDiffAgentSupported(def) { names = append(names, string(def.Type)) } } return names } +func parseDiffAgentSupported(def parser.AgentDef) bool { + if !def.FileBased { + return false + } + if def.DiscoverFunc != nil { + return true + } + switch parser.ProviderMigrationModes()[def.Type] { + case parser.ProviderMigrationProviderAuthoritative: + _, ok := parser.ProviderFactoryByType(def.Type) + return ok + default: + return false + } +} + // renderParseDiffReport writes the human-readable report. An empty // archive renders a zero-count summary with no tables. Every value // that originates in session files or archive rows (IDs, paths, diff --git a/cmd/agentsview/parse_diff_test.go b/cmd/agentsview/parse_diff_test.go index 99a358c55..3b9567167 100644 --- a/cmd/agentsview/parse_diff_test.go +++ b/cmd/agentsview/parse_diff_test.go @@ -110,6 +110,16 @@ func TestParseDiffAgentTypes(t *testing.T) { in: []string{"claude"}, want: []string{"claude"}, }, + { + name: "provider authoritative agent", + in: []string{"pi"}, + want: []string{"pi"}, + }, + { + name: "provider authoritative shared provider family agent", + in: []string{"omp"}, + want: []string{"omp"}, + }, { name: "trims and lowercases", in: []string{" Claude "}, diff --git a/internal/parser/discovery.go b/internal/parser/discovery.go index 799c538c4..93f1dae57 100644 --- a/internal/parser/discovery.go +++ b/internal/parser/discovery.go @@ -1426,100 +1426,6 @@ func IsPiSessionFile(path string) bool { return false } -// DiscoverPiSessions finds JSONL files under piDir that are -// valid pi sessions. Pi sessions live in -// //.jsonl; the encoded-cwd -// format is ambiguous between pi versions, so discovery -// validates by reading the session header rather than parsing -// the directory name. Project is left empty so ParsePiSession -// can derive it from the header cwd field. -func DiscoverPiSessions(piDir string) []DiscoveredFile { - return discoverPiLikeSessions(piDir, AgentPi) -} - -// DiscoverOMPSessions finds JSONL files under an OhMyPi session root. -// OMP uses the same layout and file format as Pi, rooted by default at -// ~/.omp/agent/sessions. -func DiscoverOMPSessions(ompDir string) []DiscoveredFile { - return discoverPiLikeSessions(ompDir, AgentOMP) -} - -func discoverPiLikeSessions(piDir string, agent AgentType) []DiscoveredFile { - if piDir == "" { - return nil - } - entries, err := os.ReadDir(piDir) - if err != nil { - return nil - } - var files []DiscoveredFile - for _, entry := range entries { - if !isDirOrSymlink(entry, piDir) { - continue - } - cwdDir := filepath.Join(piDir, entry.Name()) - sessionFiles, err := os.ReadDir(cwdDir) - if err != nil { - continue - } - for _, sf := range sessionFiles { - if sf.IsDir() { - continue - } - if !strings.HasSuffix(sf.Name(), ".jsonl") { - continue - } - path := filepath.Join(cwdDir, sf.Name()) - if !IsPiSessionFile(path) { - continue - } - files = append(files, DiscoveredFile{ - Path: path, - Agent: agent, - // Project intentionally empty; ParsePiSession - // derives project from the header cwd field. - }) - } - } - sort.Slice(files, func(i, j int) bool { - return files[i].Path < files[j].Path - }) - return files -} - -// FindPiSourceFile finds the original JSONL file for a pi -// session ID by searching all encoded-cwd subdirectories -// under piDir for a file named .jsonl. -func FindPiSourceFile(piDir, sessionID string) string { - return findPiLikeSourceFile(piDir, sessionID) -} - -// FindOMPSourceFile finds the original JSONL file for an OMP session ID. -func FindOMPSourceFile(ompDir, sessionID string) string { - return findPiLikeSourceFile(ompDir, sessionID) -} - -func findPiLikeSourceFile(piDir, sessionID string) string { - if piDir == "" || !IsValidSessionID(sessionID) { - return "" - } - entries, err := os.ReadDir(piDir) - if err != nil { - return "" - } - target := sessionID + ".jsonl" - for _, entry := range entries { - if !isDirOrSymlink(entry, piDir) { - continue - } - candidate := filepath.Join(piDir, entry.Name(), target) - if _, err := os.Stat(candidate); err == nil { - return candidate - } - } - return "" -} - // isRegularFile returns true if path exists and is a regular // file (not a symlink, directory, or other special file). // IsRegularFile reports whether path is a regular file (not diff --git a/internal/parser/pi.go b/internal/parser/pi.go index f9b6e965e..b8fada33e 100644 --- a/internal/parser/pi.go +++ b/internal/parser/pi.go @@ -11,22 +11,12 @@ import ( "github.com/tidwall/gjson" ) -// ParsePiSession parses a pi-agent JSONL session file. -// The file format uses a leading session-header entry followed by -// message, model_change, and compaction entries. -func ParsePiSession( +func (p *piProvider) parseSession( path, project, machine string, ) (*ParsedSession, []ParsedMessage, error) { - return parsePiLikeSession(path, project, machine, AgentPi, "pi:") -} - -// ParseOMPSession parses an OhMyPi JSONL session file. OMP uses the -// same on-disk session format as Pi, but sessions are identified with -// the omp agent type and omp: session ID prefix. -func ParseOMPSession( - path, project, machine string, -) (*ParsedSession, []ParsedMessage, error) { - return parsePiLikeSession(path, project, machine, AgentOMP, "omp:") + return parsePiLikeSession( + path, project, machine, p.Def.Type, p.Def.IDPrefix, + ) } func parsePiLikeSession( diff --git a/internal/parser/pi_provider.go b/internal/parser/pi_provider.go new file mode 100644 index 000000000..501b6cd74 --- /dev/null +++ b/internal/parser/pi_provider.go @@ -0,0 +1,228 @@ +package parser + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" +) + +var _ Provider = (*piProvider)(nil) + +type piProviderFactory struct { + def AgentDef +} + +func newPiProviderFactory(def AgentDef) ProviderFactory { + return piProviderFactory{def: cloneAgentDef(def)} +} + +func (f piProviderFactory) Definition() AgentDef { + return cloneAgentDef(f.def) +} + +func (f piProviderFactory) Capabilities() Capabilities { + return piProviderCapabilities() +} + +func (f piProviderFactory) NewProvider(cfg ProviderConfig) Provider { + cfg = cfg.Clone() + return &piProvider{ + ProviderBase: ProviderBase{ + Def: cloneAgentDef(f.def), + Caps: piProviderCapabilities(), + Config: cfg, + }, + sources: newPiSourceSet(f.def.Type, cfg.Roots), + } +} + +type piProvider struct { + ProviderBase + sources DirectoryJSONLSourceSet +} + +func (p *piProvider) Discover(ctx context.Context) ([]SourceRef, error) { + sources, err := p.sources.Discover(ctx) + if err != nil { + return nil, err + } + return p.filterDiscoveredSources(sources), nil +} + +func (p *piProvider) WatchPlan(ctx context.Context) (WatchPlan, error) { + return p.sources.WatchPlan(ctx) +} + +func (p *piProvider) SourcesForChangedPath( + ctx context.Context, + req ChangedPathRequest, +) ([]SourceRef, error) { + sources, err := p.sources.SourcesForChangedPath(ctx, req) + if err != nil || len(sources) == 0 { + return sources, err + } + if jsonlMissingPathFallbackAllowed(req) { + return sources, nil + } + return p.filterDiscoveredSources(sources), nil +} + +func (p *piProvider) FindSource( + ctx context.Context, + req FindSourceRequest, +) (SourceRef, bool, error) { + if err := ctx.Err(); err != nil { + return SourceRef{}, false, err + } + req = providerFindRequestWithRawSessionID(p.Def, req) + for _, path := range []string{ + req.StoredFilePath, + req.FingerprintKey, + } { + if path == "" { + continue + } + if source, ok, err := p.sources.sourceForPath(ctx, path); err != nil { + return SourceRef{}, false, err + } else if ok { + return source, true, nil + } + } + if req.RawSessionID == "" || !IsValidSessionID(req.RawSessionID) { + return SourceRef{}, false, nil + } + for _, root := range p.Config.Roots { + source, ok, err := p.sourceForSessionID(ctx, root, req.RawSessionID) + if err != nil || ok { + return source, ok, err + } + } + return SourceRef{}, false, nil +} + +func (p *piProvider) sourceForSessionID( + ctx context.Context, + root string, + sessionID string, +) (SourceRef, bool, error) { + entries, err := os.ReadDir(root) + if err != nil { + return SourceRef{}, false, nil + } + target := sessionID + ".jsonl" + for _, entry := range entries { + if err := ctx.Err(); err != nil { + return SourceRef{}, false, err + } + if !isDirOrSymlink(entry, root) { + continue + } + candidate := filepath.Join(root, entry.Name(), target) + source, ok, err := p.sources.sourceForPath(ctx, candidate) + if err != nil { + return SourceRef{}, false, err + } + if ok { + return source, true, nil + } + } + return SourceRef{}, false, nil +} + +func (p *piProvider) Fingerprint( + ctx context.Context, + source SourceRef, +) (SourceFingerprint, error) { + return p.sources.Fingerprint(ctx, source) +} + +func (p *piProvider) Parse( + ctx context.Context, + req ParseRequest, +) (ParseOutcome, error) { + if err := ctx.Err(); err != nil { + return ParseOutcome{}, err + } + path, ok, err := p.sources.pathFromSource(ctx, req.Source) + if err != nil { + return ParseOutcome{}, err + } + if !ok { + return ParseOutcome{}, fmt.Errorf("pi source path unavailable") + } + machine := firstNonEmptyJSONLString(req.Machine, p.Config.Machine) + sess, msgs, err := p.parseSession(path, req.Source.ProjectHint, machine) + if err != nil { + return ParseOutcome{}, err + } + if sess == nil { + return ParseOutcome{ + ResultSetComplete: true, + SkipReason: SkipNoSession, + }, nil + } + if req.Fingerprint.Hash != "" { + sess.File.Hash = req.Fingerprint.Hash + } + return ParseOutcome{ + Results: []ParseResultOutcome{{ + Result: ParseResult{ + Session: *sess, + Messages: msgs, + }, + DataVersion: DataVersionCurrent, + }}, + ResultSetComplete: true, + }, nil +} + +func (p *piProvider) filterDiscoveredSources(sources []SourceRef) []SourceRef { + filtered := sources[:0] + for _, source := range sources { + src, ok := source.Opaque.(JSONLSource) + if !ok || !IsPiSessionFile(src.Path) { + continue + } + filtered = append(filtered, source) + } + return filtered +} + +func newPiSourceSet(agent AgentType, roots []string) DirectoryJSONLSourceSet { + return newDirectoryJSONLSourceSet(agent, roots, + withSymlinkFollowing(), + withIncludePath(isPiSourcePath), + withProjectHint(func(root, path string) string { return "" }), + withSessionIDFromPath(piSessionIDFromPath), + ) +} + +func isPiSourcePath(root, path string) bool { + return strings.HasSuffix(filepath.Base(path), ".jsonl") +} + +func piSessionIDFromPath(root, path string) string { + if !isPiSourcePath(root, path) { + return "" + } + return strings.TrimSuffix(filepath.Base(path), ".jsonl") +} + +func piProviderCapabilities() Capabilities { + return Capabilities{ + Source: jsonlFileProviderSourceCapabilities(), + Content: ContentCapabilities{ + FirstMessage: CapabilitySupported, + SessionName: CapabilitySupported, + Cwd: CapabilitySupported, + Relationships: CapabilitySupported, + Thinking: CapabilitySupported, + ToolCalls: CapabilitySupported, + ToolResults: CapabilitySupported, + PerMessageTokenUsage: CapabilitySupported, + Model: CapabilitySupported, + }, + } +} diff --git a/internal/parser/pi_provider_test.go b/internal/parser/pi_provider_test.go new file mode 100644 index 000000000..041f408cc --- /dev/null +++ b/internal/parser/pi_provider_test.go @@ -0,0 +1,227 @@ +package parser + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPiProviderFactoryReplacesLegacyAdapter(t *testing.T) { + factory, ok := ProviderFactoryByType(AgentPi) + require.True(t, ok) + require.NotNil(t, factory) + + provider, ok := NewProvider(AgentPi, ProviderConfig{ + Roots: []string{t.TempDir()}, + Machine: "devbox", + }) + require.True(t, ok) + require.NotNil(t, provider) +} + +func TestOMPProviderSourceMethods(t *testing.T) { + root := t.TempDir() + sourcePath := filepath.Join(root, "encoded-cwd", "session-123.jsonl") + writeSourceFile(t, sourcePath, piProviderFixture("session-123")) + + provider, ok := NewProvider(AgentOMP, ProviderConfig{ + Roots: []string{root}, + Machine: "devbox", + }) + require.True(t, ok) + + discovered, err := provider.Discover(context.Background()) + require.NoError(t, err) + require.Len(t, discovered, 1) + assert.Equal(t, AgentOMP, discovered[0].Provider) + assert.Equal(t, sourcePath, discovered[0].DisplayPath) + + plan, err := provider.WatchPlan(context.Background()) + require.NoError(t, err) + require.Len(t, plan.Roots, 1) + assert.Equal(t, root, plan.Roots[0].Path) + + found, ok, err := provider.FindSource(context.Background(), FindSourceRequest{ + FullSessionID: "host~omp:session-123", + }) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, AgentOMP, found.Provider) + assert.Equal(t, sourcePath, found.DisplayPath) + + outcome, err := provider.Parse(context.Background(), ParseRequest{ + Source: discovered[0], + Fingerprint: SourceFingerprint{Key: sourcePath, Hash: "abc123"}, + }) + require.NoError(t, err) + require.True(t, outcome.ResultSetComplete) + require.Len(t, outcome.Results, 1) + assert.Equal(t, "omp:session-123", outcome.Results[0].Result.Session.ID) + assert.Equal(t, AgentOMP, outcome.Results[0].Result.Session.Agent) + assert.Equal(t, "abc123", outcome.Results[0].Result.Session.File.Hash) + + require.NoError(t, os.Remove(sourcePath)) + changed, err := provider.SourcesForChangedPath( + context.Background(), + ChangedPathRequest{Path: sourcePath, EventKind: "remove", WatchRoot: root}, + ) + require.NoError(t, err) + require.Len(t, changed, 1) + assert.Equal(t, AgentOMP, changed[0].Provider) + assert.Equal(t, sourcePath, changed[0].DisplayPath) +} + +func TestPiProviderSourceMethods(t *testing.T) { + root := t.TempDir() + sourcePath := filepath.Join(root, "encoded-cwd", "session-123.jsonl") + lookupOnlyPath := filepath.Join(root, "encoded-cwd", "lookup-only.jsonl") + writeSourceFile(t, sourcePath, piProviderFixture("session-123")) + writeSourceFile(t, lookupOnlyPath, `{"type":"message"}`+"\n") + writeSourceFile(t, filepath.Join(root, "encoded-cwd", "notes.txt"), "{}\n") + writeSourceFile(t, filepath.Join(root, "root-session.jsonl"), piProviderFixture("root-session")) + writeSourceFile(t, filepath.Join(root, "encoded-cwd", "nested", "deep.jsonl"), piProviderFixture("deep")) + + provider, ok := NewProvider(AgentPi, ProviderConfig{ + Roots: []string{root}, + Machine: "devbox", + }) + require.True(t, ok) + + discovered, err := provider.Discover(context.Background()) + require.NoError(t, err) + require.Len(t, discovered, 1) + assert.Equal(t, AgentPi, discovered[0].Provider) + assert.Equal(t, sourcePath, discovered[0].DisplayPath) + assert.Empty(t, discovered[0].ProjectHint) + + plan, err := provider.WatchPlan(context.Background()) + require.NoError(t, err) + require.Len(t, plan.Roots, 1) + assert.Equal(t, root, plan.Roots[0].Path) + assert.True(t, plan.Roots[0].Recursive) + assert.Equal(t, []string{"*.jsonl"}, plan.Roots[0].IncludeGlobs) + + found, ok, err := provider.FindSource(context.Background(), FindSourceRequest{ + FullSessionID: "host~pi:session-123", + }) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, sourcePath, found.DisplayPath) + + found, ok, err = provider.FindSource(context.Background(), FindSourceRequest{ + RawSessionID: "pi:lookup-only", + }) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, lookupOnlyPath, found.DisplayPath) + + require.NoError(t, os.Remove(sourcePath)) + changed, err := provider.SourcesForChangedPath( + context.Background(), + ChangedPathRequest{Path: sourcePath, EventKind: "remove", WatchRoot: root}, + ) + require.NoError(t, err) + require.Len(t, changed, 1) + assert.Equal(t, sourcePath, changed[0].DisplayPath) +} + +func TestPiProviderDiscoveryAcceptsSessionHeaderInNonSessionIDFilename(t *testing.T) { + root := t.TempDir() + sourcePath := filepath.Join(root, "encoded-cwd", "2025.01.01.jsonl") + writeSourceFile(t, sourcePath, piProviderFixture("header-session-id")) + + provider, ok := NewProvider(AgentPi, ProviderConfig{ + Roots: []string{root}, + Machine: "devbox", + }) + require.True(t, ok) + + discovered, err := provider.Discover(context.Background()) + require.NoError(t, err) + require.Len(t, discovered, 1) + assert.Equal(t, sourcePath, discovered[0].DisplayPath) + + outcome, err := provider.Parse(context.Background(), ParseRequest{ + Source: discovered[0], + }) + require.NoError(t, err) + require.Len(t, outcome.Results, 1) + assert.Equal(t, "pi:header-session-id", outcome.Results[0].Result.Session.ID) + + _, ok, err = provider.FindSource(context.Background(), FindSourceRequest{ + RawSessionID: "2025.01.01", + }) + require.NoError(t, err) + assert.False(t, ok) +} + +func TestPiProviderDiscoversSymlinkedCWDDirectory(t *testing.T) { + root := t.TempDir() + targetDir := t.TempDir() + sourcePath := filepath.Join(root, "linked-cwd", "session-123.jsonl") + targetPath := filepath.Join(targetDir, "session-123.jsonl") + writeSourceFile(t, targetPath, piProviderFixture("session-123")) + if err := os.Symlink(targetDir, filepath.Join(root, "linked-cwd")); err != nil { + t.Skipf("symlink not supported: %v", err) + } + + provider, ok := NewProvider(AgentPi, ProviderConfig{ + Roots: []string{root}, + Machine: "devbox", + }) + require.True(t, ok) + + discovered, err := provider.Discover(context.Background()) + require.NoError(t, err) + require.Len(t, discovered, 1) + assert.Equal(t, sourcePath, discovered[0].DisplayPath) + + found, ok, err := provider.FindSource(context.Background(), FindSourceRequest{ + FullSessionID: "host~pi:session-123", + }) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, sourcePath, found.DisplayPath) +} + +func TestPiProviderParse(t *testing.T) { + root := t.TempDir() + sourcePath := filepath.Join(root, "encoded-cwd", "session-123.jsonl") + writeSourceFile(t, sourcePath, piProviderFixture("session-123")) + + provider, ok := NewProvider(AgentPi, ProviderConfig{ + Roots: []string{root}, + Machine: "devbox", + }) + require.True(t, ok) + sources, err := provider.Discover(context.Background()) + require.NoError(t, err) + require.Len(t, sources, 1) + + outcome, err := provider.Parse(context.Background(), ParseRequest{ + Source: sources[0], + Fingerprint: SourceFingerprint{Key: sourcePath, Hash: "abc123"}, + }) + require.NoError(t, err) + require.True(t, outcome.ResultSetComplete) + require.Len(t, outcome.Results, 1) + assert.Equal(t, DataVersionCurrent, outcome.Results[0].DataVersion) + assert.Equal(t, "pi:session-123", outcome.Results[0].Result.Session.ID) + assert.Equal(t, "pi_project", outcome.Results[0].Result.Session.Project) + assert.Equal(t, "devbox", outcome.Results[0].Result.Session.Machine) + assert.Equal(t, "abc123", outcome.Results[0].Result.Session.File.Hash) + assert.Len(t, outcome.Results[0].Result.Messages, 2) +} + +func piProviderFixture(sessionID string) string { + return strings.Join([]string{ + `{"type":"session","version":3,"id":"` + sessionID + `","timestamp":"2025-01-01T10:00:00Z","cwd":"/Users/alice/code/pi-project"}`, + `{"type":"message","id":"msg-1","timestamp":"2025-01-01T10:00:01Z","message":{"role":"user","content":"Inspect the Pi source."}}`, + `{"type":"message","id":"msg-2","timestamp":"2025-01-01T10:00:02Z","message":{"role":"assistant","content":"Looks ready.","model":"claude-opus-4-5","usage":{"input_tokens":10,"output_tokens":5}}}`, + }, "\n") +} diff --git a/internal/parser/pi_test.go b/internal/parser/pi_test.go index 84d66800e..572a961e8 100644 --- a/internal/parser/pi_test.go +++ b/internal/parser/pi_test.go @@ -1,9 +1,9 @@ package parser import ( + "context" "errors" "fmt" - "os" "path/filepath" "strings" "testing" @@ -19,19 +19,54 @@ import ( func runPiParserTest(t *testing.T, content string) (*ParsedSession, []ParsedMessage) { t.Helper() path := createTestFile(t, "pi-session.jsonl", content) - sess, msgs, err := ParsePiSession(path, "my_project", "local") + sess, msgs, err := parsePiTestSession(t, path, "my_project", "local") require.NoError(t, err) return sess, msgs } -// TestParsePiSession_SessionHeader verifies that the session-level fields are +func parsePiTestSession( + t *testing.T, + path string, + project string, + machine string, +) (*ParsedSession, []ParsedMessage, error) { + t.Helper() + + provider, ok := NewProvider(AgentPi, ProviderConfig{ + Roots: []string{filepath.Dir(filepath.Dir(path))}, + Machine: machine, + }) + require.True(t, ok) + + outcome, err := provider.Parse(context.Background(), ParseRequest{ + Source: SourceRef{ + Provider: AgentPi, + Key: path, + DisplayPath: path, + FingerprintKey: path, + ProjectHint: project, + Opaque: JSONLSource{ + Root: filepath.Dir(filepath.Dir(path)), + Path: path, + }, + }, + Machine: machine, + }) + if err != nil || len(outcome.Results) == 0 { + return nil, nil, err + } + result := outcome.Results[0].Result + return &result.Session, result.Messages, nil +} + +// TestPiProviderParsesSessionHeader verifies that the session-level fields are // populated correctly from the pi fixture header (PRSR-01, PRSR-11, PRSR-10). -func TestParsePiSession_SessionHeader(t *testing.T) { +func TestPiProviderParsesSessionHeader(t *testing.T) { fixturePath := createTestFile( t, "pi-test-session-uuid.jsonl", loadFixture(t, "pi/session.jsonl"), ) - sess, msgs, err := ParsePiSession(fixturePath, "", "local") + sess, msgs, err := parsePiTestSession(t, fixturePath, "", "local") require.NoError(t, err) assert.Equal(t, "pi:pi-test-session-uuid", sess.ID, "PRSR-01: session ID") @@ -57,54 +92,7 @@ func TestParsePiSession_SessionHeader(t *testing.T) { _ = msgs // not the focus of this sub-test } -func TestOMPRegistryMetadata(t *testing.T) { - def, ok := AgentByType(AgentOMP) - require.True(t, ok) - - assert.Equal(t, AgentOMP, def.Type) - assert.Equal(t, "OhMyPi", def.DisplayName) - assert.Equal(t, "OMP_DIR", def.EnvVar) - assert.Equal(t, "omp_dirs", def.ConfigKey) - assert.Equal(t, []string{".omp/agent/sessions"}, def.DefaultDirs) - assert.Equal(t, "omp:", def.IDPrefix) - assert.True(t, def.FileBased) - require.NotNil(t, def.DiscoverFunc) - require.NotNil(t, def.FindSourceFunc) -} - -func TestParseOMPSession_SessionIdentity(t *testing.T) { - fixturePath := createTestFile( - t, "omp-test-session-uuid.jsonl", - loadFixture(t, "pi/session.jsonl"), - ) - sess, msgs, err := ParseOMPSession(fixturePath, "", "local") - require.NoError(t, err) - require.NotNil(t, sess) - - assert.Equal(t, "omp:pi-test-session-uuid", sess.ID) - assert.Equal(t, AgentOMP, sess.Agent) - assert.Equal(t, "omp:2025-01-01T09-00-00-000Z_parent-uuid", sess.ParentSessionID) - assert.Equal(t, "/Users/alice/code/my-project", sess.Cwd) - assert.Equal(t, "my_project", sess.Project) - require.NotEmpty(t, msgs) -} - -func TestDiscoverOMPSessions(t *testing.T) { - root := t.TempDir() - projectDir := filepath.Join(root, "-Users-alice-code-my-project") - require.NoError(t, os.MkdirAll(projectDir, 0o755)) - path := filepath.Join(projectDir, "omp-test-session-uuid.jsonl") - require.NoError(t, os.WriteFile(path, []byte(loadFixture(t, "pi/session.jsonl")), 0o644)) - - files := DiscoverOMPSessions(root) - require.Len(t, files, 1) - assert.Equal(t, path, files[0].Path) - assert.Equal(t, AgentOMP, files[0].Agent) - assert.Empty(t, files[0].Project) - assert.Equal(t, path, FindOMPSourceFile(root, "omp-test-session-uuid")) -} - -func TestParsePiSession_SessionInfoName(t *testing.T) { +func TestPiProviderParsesSessionInfoName(t *testing.T) { content := strings.Join([]string{ `{"type":"session","version":3,"id":"named-sess","timestamp":"2025-01-01T10:00:00Z","cwd":"/Users/alice/code/my-project"}`, `{"type":"session_info","id":"info-1","parentId":null,"timestamp":"2025-01-01T10:00:01Z","name":"Original name"}`, @@ -122,7 +110,7 @@ func TestParsePiSession_SessionInfoName(t *testing.T) { assert.Equal(t, RoleUser, msgs[0].Role) } -func TestParsePiSession_SessionInfoLastNameWins(t *testing.T) { +func TestPiProviderParsesSessionInfoLastNameWins(t *testing.T) { content := strings.Join([]string{ `{"type":"session","version":3,"id":"renamed-sess","timestamp":"2025-01-01T10:00:00Z","cwd":"/Users/alice/code/my-project"}`, `{"type":"session_info","id":"info-1","parentId":null,"timestamp":"2025-01-01T10:00:01Z","name":"Initial name"}`, @@ -137,14 +125,14 @@ func TestParsePiSession_SessionInfoLastNameWins(t *testing.T) { assert.Equal(t, "Final name", sess.SessionName) } -// TestParsePiSession_UserMessages verifies user message content and ordinals +// TestPiProviderParsesUserMessages verifies user message content and ordinals // (PRSR-02, PRSR-01). -func TestParsePiSession_UserMessages(t *testing.T) { +func TestPiProviderParsesUserMessages(t *testing.T) { fixturePath := createTestFile( t, "pi-session.jsonl", loadFixture(t, "pi/session.jsonl"), ) - sess, msgs, err := ParsePiSession(fixturePath, "", "local") + sess, msgs, err := parsePiTestSession(t, fixturePath, "", "local") require.NoError(t, err) // First non-toolResult user message at index 0. @@ -156,14 +144,14 @@ func TestParsePiSession_UserMessages(t *testing.T) { assert.Contains(t, sess.FirstMessage, "Fix the login bug", "PRSR-01: FirstMessage") } -// TestParsePiSession_AssistantMessages verifies the assistant message with +// TestPiProviderParsesAssistantMessages verifies the assistant message with // thinking, text, and tool call (PRSR-03, PRSR-04, PRSR-06). -func TestParsePiSession_AssistantMessages(t *testing.T) { +func TestPiProviderParsesAssistantMessages(t *testing.T) { fixturePath := createTestFile( t, "pi-session.jsonl", loadFixture(t, "pi/session.jsonl"), ) - _, msgs, err := ParsePiSession(fixturePath, "", "local") + _, msgs, err := parsePiTestSession(t, fixturePath, "", "local") require.NoError(t, err) // entry-2 is the second entry overall (index 1 in messages). @@ -195,14 +183,14 @@ func TestParsePiSession_AssistantMessages(t *testing.T) { assert.Contains(t, assistantMsg.Content, "[Read: auth.go]", "tool use marker in Content") } -// TestParsePiSession_ToolResults verifies tool result entries are parsed +// TestPiProviderParsesToolResults verifies tool result entries are parsed // correctly (PRSR-05). -func TestParsePiSession_ToolResults(t *testing.T) { +func TestPiProviderParsesToolResults(t *testing.T) { fixturePath := createTestFile( t, "pi-session.jsonl", loadFixture(t, "pi/session.jsonl"), ) - _, msgs, err := ParsePiSession(fixturePath, "", "local") + _, msgs, err := parsePiTestSession(t, fixturePath, "", "local") require.NoError(t, err) var toolResultMsg *ParsedMessage @@ -223,7 +211,7 @@ func TestParsePiSession_ToolResults(t *testing.T) { assert.Contains(t, decoded, "package auth", "ContentRaw must decode to tool output text") } -func TestParsePiSession_StringContent(t *testing.T) { +func TestPiProviderParsesStringContent(t *testing.T) { header := `{"type":"session","id":"str-sess","timestamp":"2025-01-01T10:00:00Z","cwd":"/tmp"}` + "\n" t.Run("assistant string content", func(t *testing.T) { @@ -250,14 +238,14 @@ func TestParsePiSession_StringContent(t *testing.T) { }) } -// TestParsePiSession_ThinkingBlocks verifies both explicit and redacted +// TestPiProviderParsesThinkingBlocks verifies both explicit and redacted // thinking blocks (PRSR-06). -func TestParsePiSession_ThinkingBlocks(t *testing.T) { +func TestPiProviderParsesThinkingBlocks(t *testing.T) { fixturePath := createTestFile( t, "pi-session.jsonl", loadFixture(t, "pi/session.jsonl"), ) - _, msgs, err := ParsePiSession(fixturePath, "", "local") + _, msgs, err := parsePiTestSession(t, fixturePath, "", "local") require.NoError(t, err) t.Run("explicit thinking", func(t *testing.T) { @@ -291,14 +279,14 @@ func TestParsePiSession_ThinkingBlocks(t *testing.T) { }) } -// TestParsePiSession_UserMessageCount verifies that metadata entries do -// not inflate user counts even when compactions persist as system rows. -func TestParsePiSession_UserMessageCount(t *testing.T) { +// TestPiProviderParsesUserMessageCount verifies that model_change and +// compaction entries are skipped entirely and do not inflate user counts. +func TestPiProviderParsesUserMessageCount(t *testing.T) { fixturePath := createTestFile( t, "pi-session.jsonl", loadFixture(t, "pi/session.jsonl"), ) - sess, _, err := ParsePiSession(fixturePath, "", "local") + sess, _, err := parsePiTestSession(t, fixturePath, "", "local") require.NoError(t, err) // The fixture has 2 real user messages. Metadata rows must not count @@ -307,9 +295,9 @@ func TestParsePiSession_UserMessageCount(t *testing.T) { "UserMessageCount must only count real user messages") } -// TestParsePiSession_UserMessageCountEmptyContent verifies that user messages +// TestPiProviderParsesUserMessageCountEmptyContent verifies that user messages // with non-text or empty payloads are still counted. -func TestParsePiSession_UserMessageCountEmptyContent(t *testing.T) { +func TestPiProviderParsesUserMessageCountEmptyContent(t *testing.T) { fixture := `{"type":"session","id":"sess-1","cwd":"/tmp","timestamp":"2025-01-01T10:00:00Z"} {"type":"message","timestamp":"2025-01-01T10:00:00Z","message":{"role":"user","content":[{"type":"text","text":"hello"}]},"id":"1"} {"type":"message","timestamp":"2025-01-01T10:00:01Z","message":{"role":"user","content":[{"type":"image","source":{"data":"abc"}}]},"id":"2"} @@ -317,7 +305,7 @@ func TestParsePiSession_UserMessageCountEmptyContent(t *testing.T) { {"type":"message","timestamp":"2025-01-01T10:00:03Z","message":{"role":"assistant","content":[{"type":"text","text":"response"}]},"id":"4"}` fixturePath := createTestFile(t, "pi-empty-content.jsonl", fixture) - sess, _, err := ParsePiSession(fixturePath, "", "local") + sess, _, err := parsePiTestSession(t, fixturePath, "", "local") require.NoError(t, err) // All 3 user messages should be counted, even those without text content. @@ -325,21 +313,21 @@ func TestParsePiSession_UserMessageCountEmptyContent(t *testing.T) { "UserMessageCount must count user messages with empty or non-text content") } -// TestParsePiSession_SilentSkips verifies that the parser silently ignores +// TestPiProviderParsesSilentSkips verifies that the parser silently ignores // malformed JSON, thinking_level_change entries, and unknown future entry types // without returning an error. -func TestParsePiSession_SilentSkips(t *testing.T) { +func TestPiProviderParsesSilentSkips(t *testing.T) { fixturePath := createTestFile( t, "pi-session.jsonl", loadFixture(t, "pi/session.jsonl"), ) - _, _, err := ParsePiSession(fixturePath, "", "local") + _, _, err := parsePiTestSession(t, fixturePath, "", "local") require.NoError(t, err, "parser must succeed despite malformed/unknown lines") } -// TestParsePiSession_V1Session verifies that a session without an id field +// TestPiProviderParsesV1Session verifies that a session without an id field // derives its session ID from the filename (PRSR-09). -func TestParsePiSession_V1Session(t *testing.T) { +func TestPiProviderParsesV1Session(t *testing.T) { v1Content := strings.Join([]string{ `{"type":"session","timestamp":"2025-01-01T10:00:00Z","cwd":"/Users/alice/code/v1-project"}`, `{"type":"message","timestamp":"2025-01-01T10:00:01Z","message":{"role":"user","content":[{"type":"text","text":"hello"}]}}`, @@ -347,7 +335,7 @@ func TestParsePiSession_V1Session(t *testing.T) { }, "\n") path := createTestFile(t, "v1-session.jsonl", v1Content) - sess, _, err := ParsePiSession(path, "v1_project", "local") + sess, _, err := parsePiTestSession(t, path, "v1_project", "local") require.NoError(t, err) assert.Equal(t, "pi:v1-session", sess.ID, "PRSR-09: V1 session ID from filename") @@ -362,7 +350,7 @@ func TestParsePiSession_V1MessageLineageStaysEmpty(t *testing.T) { }, "\n") path := createTestFile(t, "v1-lineage.jsonl", content) - sess, msgs, err := ParsePiSession(path, "v1_project", "local") + sess, msgs, err := parsePiTestSession(t, path, "v1_project", "local") require.NoError(t, err) assert.Equal(t, "pi:v1-lineage", sess.ID) @@ -373,14 +361,14 @@ func TestParsePiSession_V1MessageLineageStaysEmpty(t *testing.T) { } } -// TestParsePiSession_BranchedFrom verifies the exact ParentSessionID value +// TestPiProviderParsesBranchedFrom verifies the exact ParentSessionID value // extracted from the branchedFrom field (PRSR-10). -func TestParsePiSession_BranchedFrom(t *testing.T) { +func TestPiProviderParsesBranchedFrom(t *testing.T) { fixturePath := createTestFile( t, "pi-session.jsonl", loadFixture(t, "pi/session.jsonl"), ) - sess, _, err := ParsePiSession(fixturePath, "", "local") + sess, _, err := parsePiTestSession(t, fixturePath, "", "local") require.NoError(t, err) t.Run("parent session ID from branchedFrom", func(t *testing.T) { @@ -443,9 +431,9 @@ func TestParsePiSession_MessageLineageContinuity(t *testing.T) { assert.Equal(t, "assistant", msgs[5].SourceType) } -// TestParsePiSession_IOError verifies that I/O errors encountered after the +// TestPiProviderParsesIOError verifies that I/O errors encountered after the // session header are surfaced and that the error string contains "reading pi". -func TestParsePiSession_IOError(t *testing.T) { +func TestPiProviderParsesIOError(t *testing.T) { t.Run("error message format contains reading pi", func(t *testing.T) { ioErr := errors.New("disk read failed") err := fmt.Errorf("reading pi %s: %w", "/some/path/session.jsonl", ioErr) @@ -458,7 +446,7 @@ func TestParsePiSession_IOError(t *testing.T) { msg := `{"type":"message","id":"entry-1","timestamp":"2025-01-01T10:00:01Z","message":{"role":"user","content":[{"type":"text","text":"hello"}]}}` + "\n" path := createTestFile(t, "pi-clean-read.jsonl", header+msg) - sess, msgs, parseErr := ParsePiSession(path, "my_project", "local") + sess, msgs, parseErr := parsePiTestSession(t, path, "my_project", "local") require.NoError(t, parseErr, "clean read must not produce an error") require.NotNil(t, sess) @@ -591,7 +579,7 @@ func TestParsePiAssistantMessage_IntentInToolMarker(t *testing.T) { "agent__intent must be normalized to description for tool marker") } -// TestParsePiSession_ErrorCases verifies error handling for missing, empty, +// TestPiProviderParsesErrorCases verifies error handling for missing, empty, // and invalid session files. func TestNormalizePiIntent(t *testing.T) { tests := []struct { @@ -652,22 +640,22 @@ func TestNormalizePiIntent(t *testing.T) { } } -func TestParsePiSession_ErrorCases(t *testing.T) { +func TestPiProviderParsesErrorCases(t *testing.T) { t.Run("missing file", func(t *testing.T) { - _, _, err := ParsePiSession("/nonexistent/path/session.jsonl", "proj", "local") + _, _, err := parsePiTestSession(t, "/nonexistent/path/session.jsonl", "proj", "local") assert.Error(t, err, "missing file must return error") }) t.Run("empty file", func(t *testing.T) { path := createTestFile(t, "empty.jsonl", "") - _, _, err := ParsePiSession(path, "proj", "local") + _, _, err := parsePiTestSession(t, path, "proj", "local") assert.Error(t, err, "empty file (no session header) must return error") }) t.Run("not a pi session", func(t *testing.T) { content := `{"type":"message","id":"entry-1","timestamp":"2025-01-01T10:00:00Z","message":{"role":"user","content":[{"type":"text","text":"hello"}]}}` + "\n" path := createTestFile(t, "not-pi.jsonl", content) - _, _, err := ParsePiSession(path, "proj", "local") + _, _, err := parsePiTestSession(t, path, "proj", "local") assert.Error(t, err, "file without session header must return error") }) @@ -678,23 +666,23 @@ func TestParsePiSession_ErrorCases(t *testing.T) { msg := `{"type":"message","id":"m1","timestamp":"2025-06-01T10:01:00Z","message":{"role":"user","content":"hello"}}` content := " \n\t\n" + header + "\n" + msg + "\n" path := createTestFile(t, "ws-leading.jsonl", content) - sess, msgs, err := ParsePiSession(path, "proj", "local") + sess, msgs, err := parsePiTestSession(t, path, "proj", "local") require.NoError(t, err, "whitespace-only leading lines must not cause parse failure") assert.Equal(t, "pi:ws-sess", sess.ID) assert.Len(t, msgs, 1) }) } -// TestParsePiSession_TokenUsageFromFixture verifies that assistant +// TestPiProviderParsesTokenUsageFromFixture verifies that assistant // messages in the standard pi fixture get Model and TokenUsage // populated from the inline message.model and message.usage fields. // Without this, the usage dashboard reports $0 for pi sessions. -func TestParsePiSession_TokenUsageFromFixture(t *testing.T) { +func TestPiProviderParsesTokenUsageFromFixture(t *testing.T) { fixturePath := createTestFile( t, "pi-session.jsonl", loadFixture(t, "pi/session.jsonl"), ) - sess, msgs, err := ParsePiSession(fixturePath, "", "local") + sess, msgs, err := parsePiTestSession(t, fixturePath, "", "local") require.NoError(t, err) var assistants []ParsedMessage @@ -736,10 +724,10 @@ func TestParsePiSession_TokenUsageFromFixture(t *testing.T) { "session PeakContextTokens = max(100, 200)") } -// TestParsePiSession_ModelFromModelChange verifies that when an +// TestPiProviderParsesModelFromModelChange verifies that when an // assistant message has no inline model field, the parser falls // back to the most recent model_change entry's modelId. -func TestParsePiSession_ModelFromModelChange(t *testing.T) { +func TestPiProviderParsesModelFromModelChange(t *testing.T) { header := `{"type":"session","id":"mc-sess","timestamp":"2025-01-01T10:00:00Z","cwd":"/tmp"}` + "\n" mc := `{"type":"model_change","id":"mc1","timestamp":"2025-01-01T10:00:00.5Z","provider":"openai","modelId":"gpt-5.4"}` + "\n" user := `{"type":"message","id":"u1","timestamp":"2025-01-01T10:00:01Z","message":{"role":"user","content":"hi"}}` + "\n" @@ -762,12 +750,12 @@ func TestParsePiSession_ModelFromModelChange(t *testing.T) { "token usage extracted from message.usage") } -// TestParsePiSession_UnknownUsageShape verifies that a present +// TestPiProviderParsesUnknownUsageShape verifies that a present // but unrecognized usage object (empty {} or a foreign schema // with none of the keys we know about) leaves TokenUsage empty // so the usage query filter skips the row, rather than // fabricating a zero-valued record. -func TestParsePiSession_UnknownUsageShape(t *testing.T) { +func TestPiProviderParsesUnknownUsageShape(t *testing.T) { header := `{"type":"session","id":"uu-sess","timestamp":"2025-01-01T10:00:00Z","cwd":"/tmp"}` + "\n" cases := []struct { @@ -795,14 +783,14 @@ func TestParsePiSession_UnknownUsageShape(t *testing.T) { } } -// TestParsePiSession_ZeroUsage verifies that an explicit usage +// TestPiProviderParsesZeroUsage verifies that an explicit usage // block with every counter at zero is preserved as "known // zero" rather than collapsed to "unknown". The normalized // token_usage is still written and coverage flags follow field // presence, matching the claude parser contract and letting // downstream rollups distinguish an errored request from a // missing usage blob. -func TestParsePiSession_ZeroUsage(t *testing.T) { +func TestPiProviderParsesZeroUsage(t *testing.T) { header := `{"type":"session","id":"zu-sess","timestamp":"2025-01-01T10:00:00Z","cwd":"/tmp"}` + "\n" asst := `{"type":"message","id":"a1","timestamp":"2025-01-01T10:00:01Z","message":{"role":"assistant","content":"oops","model":"gpt-5.4","usage":{"input":0,"output":0}}}` @@ -827,10 +815,10 @@ func TestParsePiSession_ZeroUsage(t *testing.T) { assert.Equal(t, 0, m.ContextTokens) } -// TestParsePiSession_NoUsageNoTokenUsage verifies that messages +// TestPiProviderParsesNoUsageNoTokenUsage verifies that messages // without a usage block do not write an empty token_usage row, // since the eligibility filter requires token_usage != ”. -func TestParsePiSession_NoUsageNoTokenUsage(t *testing.T) { +func TestPiProviderParsesNoUsageNoTokenUsage(t *testing.T) { header := `{"type":"session","id":"nu-sess","timestamp":"2025-01-01T10:00:00Z","cwd":"/tmp"}` + "\n" asst := `{"type":"message","id":"a1","timestamp":"2025-01-01T10:00:01Z","message":{"role":"assistant","content":"hello","model":"claude-opus-4-5"}}` diff --git a/internal/parser/provider.go b/internal/parser/provider.go index 27d6e88ca..ed3975317 100644 --- a/internal/parser/provider.go +++ b/internal/parser/provider.go @@ -357,6 +357,8 @@ func providerFactoryForDef(def AgentDef) ProviderFactory { return newIflowProviderFactory(def) case AgentGptme: return newGptmeProviderFactory(def) + case AgentOMP, AgentPi: + return newPiProviderFactory(def) case AgentZencoder: return newZencoderProviderFactory(def) default: diff --git a/internal/parser/provider_migration.go b/internal/parser/provider_migration.go index 6778a3304..eb7ea76d8 100644 --- a/internal/parser/provider_migration.go +++ b/internal/parser/provider_migration.go @@ -32,7 +32,7 @@ var providerMigrationModes = map[AgentType]ProviderMigrationMode{ AgentZencoder: ProviderMigrationProviderAuthoritative, AgentVSCodeCopilot: ProviderMigrationLegacyOnly, AgentVSCopilot: ProviderMigrationLegacyOnly, - AgentPi: ProviderMigrationLegacyOnly, + AgentPi: ProviderMigrationProviderAuthoritative, AgentQwen: ProviderMigrationLegacyOnly, AgentCommandCode: ProviderMigrationProviderAuthoritative, AgentDeepSeekTUI: ProviderMigrationProviderAuthoritative, @@ -58,7 +58,7 @@ var providerMigrationModes = map[AgentType]ProviderMigrationMode{ AgentGptme: ProviderMigrationProviderAuthoritative, AgentShelley: ProviderMigrationLegacyOnly, AgentAider: ProviderMigrationLegacyOnly, - AgentOMP: ProviderMigrationLegacyOnly, + AgentOMP: ProviderMigrationProviderAuthoritative, AgentReasonix: ProviderMigrationLegacyOnly, } diff --git a/internal/parser/types.go b/internal/parser/types.go index fd673522a..d80571c4e 100644 --- a/internal/parser/types.go +++ b/internal/parser/types.go @@ -305,26 +305,22 @@ var Registry = []AgentDef{ FindSourceFunc: FindVisualStudioCopilotSourceFile, }, { - Type: AgentPi, - DisplayName: "Pi", - EnvVar: "PI_DIR", - ConfigKey: "pi_dirs", - DefaultDirs: []string{".pi/agent/sessions"}, - IDPrefix: "pi:", - FileBased: true, - DiscoverFunc: DiscoverPiSessions, - FindSourceFunc: FindPiSourceFile, + Type: AgentPi, + DisplayName: "Pi", + EnvVar: "PI_DIR", + ConfigKey: "pi_dirs", + DefaultDirs: []string{".pi/agent/sessions"}, + IDPrefix: "pi:", + FileBased: true, }, { - Type: AgentOMP, - DisplayName: "OhMyPi", - EnvVar: "OMP_DIR", - ConfigKey: "omp_dirs", - DefaultDirs: []string{".omp/agent/sessions"}, - IDPrefix: "omp:", - FileBased: true, - DiscoverFunc: DiscoverOMPSessions, - FindSourceFunc: FindOMPSourceFile, + Type: AgentOMP, + DisplayName: "OhMyPi", + EnvVar: "OMP_DIR", + ConfigKey: "omp_dirs", + DefaultDirs: []string{".omp/agent/sessions"}, + IDPrefix: "omp:", + FileBased: true, }, { Type: AgentQwen, diff --git a/internal/sync/engine.go b/internal/sync/engine.go index 8cf8c91b6..ad9038e89 100644 --- a/internal/sync/engine.go +++ b/internal/sync/engine.go @@ -1346,32 +1346,6 @@ func (e *Engine) classifyOnePath( return df, true } - // Pi/OMP: //.jsonl - for _, agent := range []parser.AgentType{parser.AgentPi, parser.AgentOMP} { - for _, piDir := range e.agentDirs[agent] { - if piDir == "" { - continue - } - if rel, ok := isUnder(piDir, path); ok { - parts := strings.Split(rel, sep) - if len(parts) != 2 { - continue - } - if !strings.HasSuffix(parts[1], ".jsonl") { - continue - } - if !parser.IsPiSessionFile(path) { - continue - } - return parser.DiscoveredFile{ - Path: path, - Agent: agent, - // Project left empty; parser derives from header cwd. - }, true - } - } - } - // Qwen: //chats/.jsonl for _, qwenDir := range e.agentDirs[parser.AgentQwen] { if qwenDir == "" { @@ -4530,8 +4504,6 @@ func (e *Engine) processFile( res = e.processVSCodeCopilot(file, info) case parser.AgentVSCopilot: res = e.processVisualStudioCopilot(file, info) - case parser.AgentPi, parser.AgentOMP: - res = e.processPi(file, info) case parser.AgentQwen: res = e.processQwen(file, info) case parser.AgentOpenClaw: @@ -7100,45 +7072,6 @@ func (e *Engine) processCursor( } } -// processPi parses a pi session file and returns the result -// for batching. Modeled on processClaude. -func (e *Engine) processPi( - file parser.DiscoveredFile, info os.FileInfo, -) processResult { - if e.shouldSkipByPath(file.Path, info) { - return processResult{skip: true} - } - - var ( - sess *parser.ParsedSession - msgs []parser.ParsedMessage - err error - ) - if file.Agent == parser.AgentOMP { - sess, msgs, err = parser.ParseOMPSession(file.Path, file.Project, e.machine) - } else { - sess, msgs, err = parser.ParsePiSession(file.Path, file.Project, e.machine) - } - if err != nil { - return processResult{err: err} - } - if sess == nil { - return processResult{} - } - - hash, err := ComputeFileHash(file.Path) - if err == nil { - sess.File.Hash = hash - } - - return processResult{ - results: []parser.ParseResult{{ - Session: *sess, - Messages: msgs, - }}, - } -} - func (e *Engine) processQwen( file parser.DiscoveredFile, info os.FileInfo, ) processResult { diff --git a/internal/sync/engine_integration_test.go b/internal/sync/engine_integration_test.go index 569fd38f3..709f21213 100644 --- a/internal/sync/engine_integration_test.go +++ b/internal/sync/engine_integration_test.go @@ -37,6 +37,7 @@ type testEnv struct { iflowDir string ampDir string piDir string + ompDir string kiroDir string shelleyDir string antigravityCLIDir string @@ -117,6 +118,7 @@ func setupTestEnv(t *testing.T, opts ...TestEnvOption) *testEnv { iflowDir: t.TempDir(), ampDir: t.TempDir(), piDir: t.TempDir(), + ompDir: t.TempDir(), shelleyDir: t.TempDir(), antigravityCLIDir: t.TempDir(), db: dbtest.OpenTestDB(t), @@ -184,6 +186,7 @@ func setupTestEnv(t *testing.T, opts ...TestEnvOption) *testEnv { parser.AgentIflow: {env.iflowDir}, parser.AgentAmp: {env.ampDir}, parser.AgentPi: {env.piDir}, + parser.AgentOMP: {env.ompDir}, parser.AgentKiro: kiroDirs, parser.AgentShelley: {env.shelleyDir}, parser.AgentAntigravityCLI: {env.antigravityCLIDir}, @@ -6952,6 +6955,42 @@ func TestPiSessionIntegration(t *testing.T) { ) } +func TestOMPSyncAllAndChangedPathUseProvider(t *testing.T) { + env := setupTestEnv(t) + path := env.writeSession( + t, + env.ompDir, + filepath.Join("encoded-cwd", "omp-sync.jsonl"), + piLikeProviderFixture("omp-sync", "/Users/alice/code/omp-app"), + ) + + runSyncAndAssert(t, env.engine, sync.SyncStats{ + TotalSessions: 1, Synced: 1, + }) + assertSessionState(t, env.db, "omp:omp-sync", func(sess *db.Session) { + assert.Equal(t, "omp", sess.Agent) + assert.Equal(t, "omp_app", sess.Project) + }) + assert.Equal(t, path, env.engine.FindSourceFile("omp:omp-sync")) + + updated := piLikeProviderFixture("omp-sync", "/Users/alice/code/omp-renamed") + dbtest.WriteTestFile(t, path, []byte(updated)) + env.engine.SyncPaths([]string{path}) + + assertSessionState(t, env.db, "omp:omp-sync", func(sess *db.Session) { + assert.Equal(t, "omp", sess.Agent) + assert.Equal(t, "omp_renamed", sess.Project) + }) +} + +func piLikeProviderFixture(sessionID, cwd string) string { + return strings.Join([]string{ + `{"type":"session","version":3,"id":"` + sessionID + `","timestamp":"2025-01-01T10:00:00Z","cwd":"` + cwd + `"}`, + `{"type":"message","id":"msg-1","timestamp":"2025-01-01T10:00:01Z","message":{"role":"user","content":"Inspect the source."}}`, + `{"type":"message","id":"msg-2","timestamp":"2025-01-01T10:00:02Z","message":{"role":"assistant","content":"Ready.","model":"claude-opus-4-5"}}`, + }, "\n") +} + func TestIncrementalSync_ClaudeAppend(t *testing.T) { env := setupTestEnv(t) diff --git a/internal/sync/parsediff.go b/internal/sync/parsediff.go index f37410493..672dba557 100644 --- a/internal/sync/parsediff.go +++ b/internal/sync/parsediff.go @@ -3,6 +3,7 @@ package sync import ( "context" "fmt" + "log" "os" "path/filepath" "sort" @@ -48,7 +49,7 @@ func (e *Engine) ParseDiff(ctx context.Context, opts ParseDiffOptions) (*ParseDi e.syncMu.Lock() defer e.syncMu.Unlock() - resolved, err := resolveParseDiffAgents(opts.Agents) + resolved, err := e.resolveParseDiffAgents(opts.Agents) if err != nil { return nil, err } @@ -68,13 +69,18 @@ func (e *Engine) ParseDiff(ctx context.Context, opts ParseDiffOptions) (*ParseDi } // Discovery mirrors syncAllLocked's file phase: per-agent - // DiscoverFunc over the configured dirs, then dedupe and the + // DiscoverFunc over the configured dirs, or provider discovery for + // agents that have dropped legacy discovery, then dedupe and the // legacy-Kiro shadow filter. var files []parser.DiscoveredFile for _, def := range resolved { - for _, d := range e.agentDirs[def.Type] { - files = append(files, def.DiscoverFunc(d)...) + if def.DiscoverFunc != nil { + for _, d := range e.agentDirs[def.Type] { + files = append(files, def.DiscoverFunc(d)...) + } + continue } + files = append(files, e.parseDiffProviderSources(ctx, def.Type)...) } // DiscoverFunc does not emit the shared-SQLite source for Kiro // (data.sqlite3) or db-mode OpenCode (opencode.db) — normal sync @@ -204,18 +210,79 @@ func (e *Engine) ParseDiff(ctx context.Context, opts ParseDiffOptions) (*ParseDi return report, nil } +// parseDiffProviderSources discovers an agent's on-disk sources through +// the provider facade for agents that have dropped their DiscoverFunc. +func (e *Engine) parseDiffProviderSources( + ctx context.Context, + agentType parser.AgentType, +) []parser.DiscoveredFile { + factory, ok := e.providerFactories[agentType] + if !ok || factory == nil { + return nil + } + roots := e.agentDirs[agentType] + if len(roots) == 0 { + return nil + } + provider := factory.NewProvider(parser.ProviderConfig{ + Roots: roots, + Machine: e.machine, + }) + sources, err := provider.Discover(ctx) + if err != nil { + log.Printf("parse-diff %s provider discovery: %v", agentType, err) + return nil + } + def := provider.Definition() + var files []parser.DiscoveredFile + for _, source := range sources { + sourcePath := providerDiscoveredPath(source) + if sourcePath == "" { + continue + } + agent := source.Provider + if agent == "" { + agent = def.Type + } + sourceCopy := source + files = append(files, parser.DiscoveredFile{ + Path: sourcePath, + Project: source.ProjectHint, + Agent: agent, + ProviderSource: &sourceCopy, + ProviderProcess: true, + }) + } + return files +} + +func (e *Engine) parseDiffAgentDiscoverable(def parser.AgentDef) bool { + if !def.FileBased { + return false + } + if def.DiscoverFunc != nil { + return true + } + switch e.providerMigrationModes[def.Type] { + case parser.ProviderMigrationProviderAuthoritative: + factory, ok := e.providerFactories[def.Type] + return ok && factory != nil + default: + return false + } +} + // resolveParseDiffAgents validates the requested agent set against // the registry and returns the matching defs in registry order. Only -// file-based agents with a DiscoverFunc have an on-disk source to -// re-parse. -func resolveParseDiffAgents( +// file-based agents with an on-disk source can be re-parsed. +func (e *Engine) resolveParseDiffAgents( requested []parser.AgentType, ) ([]parser.AgentDef, error) { var allowed []parser.AgentDef allowedSet := make(map[parser.AgentType]bool) var names []string for _, def := range parser.Registry { - if def.FileBased && def.DiscoverFunc != nil { + if e.parseDiffAgentDiscoverable(def) { allowed = append(allowed, def) allowedSet[def.Type] = true names = append(names, string(def.Type)) diff --git a/internal/sync/parsediff_integration_test.go b/internal/sync/parsediff_integration_test.go index 8060baa9a..faef85da3 100644 --- a/internal/sync/parsediff_integration_test.go +++ b/internal/sync/parsediff_integration_test.go @@ -40,6 +40,7 @@ func newParseDiffEngine(env *testEnv) *sync.Engine { parser.AgentIflow: {env.iflowDir}, parser.AgentAmp: {env.ampDir}, parser.AgentPi: {env.piDir}, + parser.AgentOMP: {env.ompDir}, parser.AgentKiro: {env.kiroDir}, parser.AgentKilo: {env.kiloDir}, parser.AgentShelley: {env.shelleyDir}, @@ -807,6 +808,34 @@ func TestParseDiffAgentScope(t *testing.T) { "ParseDiff must reject database-backed agents") } +func TestParseDiffCoversProviderAuthoritativePiFamily(t *testing.T) { + env := setupTestEnv(t) + env.writeSession( + t, + env.piDir, + filepath.Join("encoded-cwd", "pd-pi.jsonl"), + piLikeProviderFixture("pd-pi", "/Users/alice/code/pi-app"), + ) + env.writeSession( + t, + env.ompDir, + filepath.Join("encoded-cwd", "pd-omp.jsonl"), + piLikeProviderFixture("pd-omp", "/Users/alice/code/omp-app"), + ) + runSyncAndAssert(t, env.engine, sync.SyncStats{ + TotalSessions: 2, Synced: 2, + }) + + report := runParseDiff(t, env, sync.ParseDiffOptions{ + Agents: []parser.AgentType{parser.AgentPi, parser.AgentOMP}, + }) + assert.Equal(t, []string{"pi", "omp"}, report.Agents) + assert.Equal(t, 2, report.FilesExamined) + assert.Equal(t, sync.ParseDiffTotals{ + Examined: 2, Identical: 2, + }, report.Totals) +} + // TestParseDiffCoversKiroSQLite proves that Kiro's shared data.sqlite3 // store — which DiscoverFunc never emits and which normal sync reaches // through a dedicated phase — is actually re-parsed by parse-diff. A