Skip to content

Commit e359fbc

Browse files
authored
feat(parse-diff): cover DB-backed Warp, Forge, and Piebald providers (#949)
parse-diff v1 (#662) rejected Warp, Forge, and Piebald because their sync phases had woven-in change detectors and no `DiscoverFunc`. The provider facade (#876#885, #924) made that reason obsolete — all three now have a unified `Discover()` via `dbBackedProviderFactory` — but both parse-diff gates still keyed on `FileBased`, which is false for these shared-SQLite stores. This relaxes the two gates (`parseDiffAgentDiscoverable`, `parseDiffAgentSupported`) to admit provider-authoritative agents with registered factories, instead of flipping `FileBased`: that flag is read at nine-plus other sites (watcher, sync engine, token accounting, remote sync, SSH resolve, settings) and genuinely means "reads a literal per-session file", so flipping it would change unrelated behavior. Two hazards the wider gate exposed are handled in the same change: - Virtual `<db>#<sessionID>` sources cannot be stat'd, so the raced-skew reclassification from #805 would have masked every real drift on these agents as "raced". `stripVirtualSourceSuffix` now knows their DB filenames and `parseDiffSourceReliableForRaced` re-requires `FileBased`, so DB-backed agents fail closed toward reporting a change rather than masking one. - These providers discover one source per session (like OpenCode, unlike per-DB Kiro), so `--limit` presence-keying moved to a shared per-session base list; sessions cut by `--limit` now report "not sampled" instead of false-positive presence findings. Quack (#930) is out of scope: it is the DuckDB remote-sync transport, not a registry agent with session sources to re-parse. `--limit` ordering: the provider stamps each session's real mtime onto an additive, advisory `SourceRef.DiscoveryMTimeNS` while `Discover()` already has the store's session metas in hand, and the parse-diff sampler prefers it over the failed-stat fallback — so limited runs sample these virtual sources newest-first like file-based agents. The field is ordering-only metadata; skip-cache and data-version freshness still resolve through `Fingerprint`, and the raced-guard is unchanged. Where to look: `internal/sync/parsediff.go` (gate, raced predicate, per-session base keying) and `internal/sync/parsediff_dbbacked_test.go` for the end-to-end coverage. Co-authored-by: Matthew Jacobs <mjacobs@users.noreply.github.com>
1 parent 80e1c0e commit e359fbc

14 files changed

Lines changed: 549 additions & 76 deletions

cmd/agentsview/parse_diff.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func newParseDiffCommand() *cobra.Command {
8383
},
8484
}
8585
cmd.Flags().StringArrayVar(&cfg.Agents, "agent", nil,
86-
"Restrict to these agents (repeatable; default: all file-based agents)")
86+
"Restrict to these agents (repeatable; default: all re-parseable agents)")
8787
cmd.Flags().IntVar(&cfg.Limit, "limit", 0,
8888
"Re-parse only the N most recently modified source files (0 = all)")
8989
cmd.Flags().BoolVar(&cfg.FailOnChange, "fail-on-change", false,
@@ -265,9 +265,12 @@ func parseDiffSupportedAgents() []string {
265265
}
266266

267267
func parseDiffAgentSupported(def parser.AgentDef) bool {
268-
if !def.FileBased {
269-
return false
270-
}
268+
// A provider-authoritative agent with a registered factory has a
269+
// Discover()/Parse() parse-diff can re-parse, whether it reads literal
270+
// per-session files or a shared SQLite store it fans out per session
271+
// (Kiro, OpenCode, Forge, Piebald, Warp). FileBased is not consulted:
272+
// import-only agents are already excluded because they are not
273+
// provider-authoritative.
271274
switch parser.ProviderMigrationModes()[def.Type] {
272275
case parser.ProviderMigrationProviderAuthoritative:
273276
_, ok := parser.ProviderFactoryByType(def.Type)

cmd/agentsview/parse_diff_test.go

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,27 @@ func TestParseDiff_UnknownAgentListsSupported(t *testing.T) {
5858
assert.Contains(t, err.Error(), want,
5959
"error should list supported agent %q", want)
6060
}
61-
for _, unwanted := range []string{"forge", "piebald", "warp"} {
61+
// The DB-backed provider-authoritative agents are re-parseable through
62+
// their providers, so they appear in the supported list too.
63+
for _, want := range []string{"forge", "piebald", "warp"} {
64+
assert.Contains(t, err.Error(), want,
65+
"error should list supported DB-backed agent %q", want)
66+
}
67+
// Import-only agents remain unsupported and must not be listed.
68+
for _, unwanted := range []string{"claude-ai", "chatgpt"} {
6269
assert.NotContains(t, err.Error(), unwanted,
63-
"error should not list unsupported agent %q", unwanted)
70+
"error should not list import-only agent %q", unwanted)
6471
}
6572
}
6673

6774
func TestParseDiff_RejectsAgentsWithoutOnDiskSource(t *testing.T) {
75+
// Only import-only agents have no source to re-parse. The DB-backed
76+
// provider-authoritative agents (Forge/Piebald/Warp) are re-parseable
77+
// through their providers and are covered as supported elsewhere.
6878
tests := []struct {
6979
name string
7080
agent string
7181
}{
72-
{"database-backed forge", "forge"},
73-
{"database-backed piebald", "piebald"},
74-
{"database-backed warp", "warp"},
7582
{"import-only claude-ai", "claude-ai"},
7683
{"import-only chatgpt", "chatgpt"},
7784
}
@@ -121,6 +128,11 @@ func TestParseDiffAgentTypes(t *testing.T) {
121128
in: []string{"omp"},
122129
want: []string{"omp"},
123130
},
131+
{
132+
name: "db-backed provider-authoritative agents",
133+
in: []string{"forge", "piebald", "warp"},
134+
want: []string{"forge", "piebald", "warp"},
135+
},
124136
{
125137
name: "trims and lowercases",
126138
in: []string{" Claude "},
@@ -137,8 +149,8 @@ func TestParseDiffAgentTypes(t *testing.T) {
137149
wantErr: `unknown agent "nope"`,
138150
},
139151
{
140-
name: "db-backed agent",
141-
in: []string{"forge"},
152+
name: "import-only agent",
153+
in: []string{"claude-ai"},
142154
wantErr: "no on-disk source to re-parse",
143155
},
144156
}
@@ -168,12 +180,13 @@ func TestParseDiffSupportedAgentsIncludesProviderAuthoritativeAgents(t *testing.
168180
supported := parseDiffSupportedAgents()
169181
modes := parser.ProviderMigrationModes()
170182
// Build the expected set from the registry so the contract covers every
171-
// current file-based, provider-authoritative agent and stays correct as
172-
// the migration manifest changes, rather than a hand-maintained subset.
183+
// current provider-authoritative agent and stays correct as the migration
184+
// manifest changes, rather than a hand-maintained subset. FileBased is not
185+
// part of the gate: DB-backed provider-authoritative agents
186+
// (Forge/Piebald/Warp) are re-parseable through their providers too.
173187
checked := 0
174188
for _, def := range parser.Registry {
175-
if !def.FileBased ||
176-
modes[def.Type] != parser.ProviderMigrationProviderAuthoritative {
189+
if modes[def.Type] != parser.ProviderMigrationProviderAuthoritative {
177190
continue
178191
}
179192
checked++
@@ -183,7 +196,23 @@ func TestParseDiffSupportedAgentsIncludesProviderAuthoritativeAgents(t *testing.
183196
"parse-diff supported list must include %s", def.Type)
184197
}
185198
require.Positive(t, checked,
186-
"expected at least one file-based provider-authoritative agent")
199+
"expected at least one provider-authoritative agent")
200+
201+
// Explicitly pin the DB-backed provider-authoritative agents so a
202+
// regression that re-adds a FileBased gate to the parse-diff support
203+
// check is caught by name, not just by the registry-wide sweep above.
204+
for _, agent := range []parser.AgentType{
205+
parser.AgentForge, parser.AgentPiebald, parser.AgentWarp,
206+
} {
207+
def, ok := parser.AgentByType(agent)
208+
require.True(t, ok, "agent %s", agent)
209+
assert.False(t, def.FileBased,
210+
"%s is expected to be DB-backed (FileBased=false)", agent)
211+
assert.True(t, parseDiffAgentSupported(def),
212+
"DB-backed %s must be supported by parse-diff", agent)
213+
assert.Contains(t, supported, string(agent),
214+
"parse-diff supported list must include DB-backed %s", agent)
215+
}
187216
}
188217

189218
func TestParseDiff_EmptyArchiveRunsClean(t *testing.T) {

internal/parser/db_backed_provider.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -190,11 +190,14 @@ func (s dbBackedSourceSet) Discover(ctx context.Context) ([]SourceRef, error) {
190190
return nil, err
191191
}
192192
for _, meta := range metas {
193-
addJSONLSource(
194-
s.newSourceRef(root, dbPath, meta.SessionID, meta.VirtualPath),
195-
&sources,
196-
seen,
197-
)
193+
ref := s.newSourceRef(root, dbPath, meta.SessionID, meta.VirtualPath)
194+
// Carry the per-session mtime captured here so parse-diff's --limit
195+
// sampler can order these virtual "<db>#<sessionID>" sources by each
196+
// session's real mtime rather than stat'ing a path that has no
197+
// on-disk existence. Ordering metadata only: skip-cache and
198+
// data-version freshness still resolve through Fingerprint.
199+
ref.DiscoveryMTimeNS = meta.FileMtime
200+
addJSONLSource(ref, &sources, seen)
198201
}
199202
}
200203
sortJSONLSources(sources)
@@ -465,7 +468,7 @@ func newForgeProviderFactory(def AgentDef) ProviderFactory {
465468
func forgeProviderSpec() dbBackedProviderSpec {
466469
return dbBackedProviderSpec{
467470
agent: AgentForge,
468-
dbName: forgeDBFilename,
471+
dbName: ForgeDBFilename,
469472
findDB: forgeDBPath,
470473
listMeta: func(dbPath string) ([]dbBackedSessionMeta, error) {
471474
metas, err := ListForgeSessionMeta(dbPath)
@@ -496,7 +499,7 @@ func newPiebaldProviderFactory(def AgentDef) ProviderFactory {
496499
func piebaldProviderSpec() dbBackedProviderSpec {
497500
return dbBackedProviderSpec{
498501
agent: AgentPiebald,
499-
dbName: piebaldDBFilename,
502+
dbName: PiebaldDBFilename,
500503
findDB: piebaldDBPath,
501504
listMeta: func(dbPath string) ([]dbBackedSessionMeta, error) {
502505
metas, err := ListPiebaldSessionMeta(dbPath)
@@ -527,7 +530,7 @@ func newWarpProviderFactory(def AgentDef) ProviderFactory {
527530
func warpProviderSpec() dbBackedProviderSpec {
528531
return dbBackedProviderSpec{
529532
agent: AgentWarp,
530-
dbName: warpDBFilename,
533+
dbName: WarpDBFilename,
531534
findDB: warpDBPath,
532535
listMeta: func(dbPath string) ([]dbBackedSessionMeta, error) {
533536
metas, err := ListWarpSessionMeta(dbPath)

internal/parser/db_backed_provider_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func TestForgeProviderSourceMethodsAndParse(t *testing.T) {
7474
})
7575
require.True(t, ok)
7676

77-
assertDBBackedWatchPlan(t, provider, root, forgeDBFilename)
77+
assertDBBackedWatchPlan(t, provider, root, ForgeDBFilename)
7878
assertDBBackedDiscoverFindFingerprint(
7979
t, provider, root, dbPath, "conv-001",
8080
)
@@ -109,7 +109,7 @@ func TestPiebaldProviderSourceMethodsAndParse(t *testing.T) {
109109
})
110110
require.True(t, ok)
111111

112-
assertDBBackedWatchPlan(t, provider, root, piebaldDBFilename)
112+
assertDBBackedWatchPlan(t, provider, root, PiebaldDBFilename)
113113
assertDBBackedDiscoverFindFingerprint(
114114
t, provider, root, dbPath, "42",
115115
)
@@ -152,7 +152,7 @@ func TestWarpProviderSourceMethodsAndParse(t *testing.T) {
152152
})
153153
require.True(t, ok)
154154

155-
assertDBBackedWatchPlan(t, provider, root, warpDBFilename)
155+
assertDBBackedWatchPlan(t, provider, root, WarpDBFilename)
156156
assertDBBackedDiscoverFindFingerprint(
157157
t, provider, root, dbPath, "conv-001",
158158
)
@@ -328,7 +328,7 @@ func TestDBBackedProviderRejectsInvalidStoredVirtualPaths(t *testing.T) {
328328
for _, path := range []string{
329329
dbPath + "#",
330330
filepath.Join(root, "forge-copy.db") + "#conv-001",
331-
filepath.Join(root, "nested", forgeDBFilename) + "#conv-001",
331+
filepath.Join(root, "nested", ForgeDBFilename) + "#conv-001",
332332
} {
333333
_, ok, err := provider.FindSource(context.Background(), FindSourceRequest{
334334
StoredFilePath: path,

internal/parser/forge.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import (
1212
"github.com/tidwall/gjson"
1313
)
1414

15-
const forgeDBFilename = ".forge.db"
15+
// ForgeDBFilename is the Forge session store filename inside its data dir.
16+
const ForgeDBFilename = ".forge.db"
1617

1718
// ForgeSessionMeta is lightweight metadata for a session,
1819
// used to detect changes without parsing messages.
@@ -27,7 +28,7 @@ func forgeDBPath(dir string) string {
2728
if dir == "" {
2829
return ""
2930
}
30-
path := filepath.Join(dir, forgeDBFilename)
31+
path := filepath.Join(dir, ForgeDBFilename)
3132
info, err := os.Stat(path)
3233
if err != nil || info.IsDir() {
3334
return ""

internal/parser/piebald.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ import (
1111
"time"
1212
)
1313

14-
const piebaldDBFilename = "app.db"
14+
// PiebaldDBFilename is the Piebald session store filename inside its data dir.
15+
const PiebaldDBFilename = "app.db"
1516

1617
// PiebaldSessionMeta is lightweight metadata for a Piebald chat.
1718
type PiebaldSessionMeta struct {
@@ -25,7 +26,7 @@ func piebaldDBPath(dir string) string {
2526
if dir == "" {
2627
return ""
2728
}
28-
path := filepath.Join(dir, piebaldDBFilename)
29+
path := filepath.Join(dir, PiebaldDBFilename)
2930
info, err := os.Stat(path)
3031
if err != nil || info.IsDir() {
3132
return ""

internal/parser/provider.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,16 @@ type SourceRef struct {
176176
FingerprintKey string
177177
// ProjectHint is advisory metadata for UI grouping and may be empty.
178178
ProjectHint string
179+
// DiscoveryMTimeNS is an optional per-source modification time in Unix
180+
// nanoseconds captured at discovery. Providers whose sources are virtual --
181+
// a shared store fanned out to one source per session, where DisplayPath is
182+
// "<db>#<sessionID>" and os.Stat cannot resolve a real mtime -- set it so
183+
// ordering consumers (parse-diff's --limit sampler) can rank sources by each
184+
// session's real mtime instead of a failed stat that collapses to 0. Zero
185+
// means unset. It is advisory ordering metadata only: it is never persisted
186+
// and must not be used for skip-cache or data-version freshness, which go
187+
// through Fingerprint.
188+
DiscoveryMTimeNS int64
179189
// Opaque is in-memory-only source state: never persisted and never required
180190
// for lookup from persisted rows, so any source that must survive a restart
181191
// has to be recoverable from Key, DisplayPath, FingerprintKey,

internal/parser/warp.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import (
1010
"time"
1111
)
1212

13-
const warpDBFilename = "warp.sqlite"
13+
// WarpDBFilename is the Warp session store filename inside its data dir.
14+
const WarpDBFilename = "warp.sqlite"
1415

1516
// WarpSessionMeta is lightweight metadata for a session,
1617
// used to detect changes without parsing messages.
@@ -486,7 +487,7 @@ func parseWarpTimestamp(s string) time.Time {
486487
// warpDBPath returns the path to warp.sqlite inside the
487488
// given directory, or "" if it doesn't exist.
488489
func warpDBPath(dir string) string {
489-
candidate := filepath.Join(dir, warpDBFilename)
490+
candidate := filepath.Join(dir, WarpDBFilename)
490491
if _, err := os.Stat(candidate); err == nil {
491492
return candidate
492493
}

internal/sync/engine_integration_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ type testEnv struct {
3434
mimocodeDir string
3535
forgeDir string
3636
piebaldDir string
37+
warpDir string
3738
iflowDir string
3839
ampDir string
3940
piDir string
@@ -115,6 +116,7 @@ func setupTestEnv(t *testing.T, opts ...TestEnvOption) *testEnv {
115116
mimocodeDir: t.TempDir(),
116117
forgeDir: t.TempDir(),
117118
piebaldDir: t.TempDir(),
119+
warpDir: t.TempDir(),
118120
iflowDir: t.TempDir(),
119121
ampDir: t.TempDir(),
120122
piDir: t.TempDir(),
@@ -183,6 +185,7 @@ func setupTestEnv(t *testing.T, opts ...TestEnvOption) *testEnv {
183185
parser.AgentMiMoCode: {env.mimocodeDir},
184186
parser.AgentForge: {env.forgeDir},
185187
parser.AgentPiebald: {env.piebaldDir},
188+
parser.AgentWarp: {env.warpDir},
186189
parser.AgentIflow: {env.iflowDir},
187190
parser.AgentAmp: {env.ampDir},
188191
parser.AgentPi: {env.piDir},
@@ -262,6 +265,10 @@ func assignFocusedAgentDir(
262265
env.mimocodeDir = dir
263266
case parser.AgentPiebald:
264267
env.piebaldDir = dir
268+
case parser.AgentForge:
269+
env.forgeDir = dir
270+
case parser.AgentWarp:
271+
env.warpDir = dir
265272
case parser.AgentPi:
266273
env.piDir = dir
267274
case parser.AgentOMP:

0 commit comments

Comments
 (0)