Skip to content

feat(parser): migrate opencode-family providers (opencode, kilo, mimocode)#881

Open
mariusvniekerk wants to merge 3 commits into
fam/claw-familyfrom
fam/opencode-family
Open

feat(parser): migrate opencode-family providers (opencode, kilo, mimocode)#881
mariusvniekerk wants to merge 3 commits into
fam/claw-familyfrom
fam/opencode-family

Conversation

@mariusvniekerk

@mariusvniekerk mariusvniekerk commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Migrates opencode, kilo, and mimocode — which share the OpenCode source format — onto a shared concrete provider, folding their discovery/lookup/parse and dropping the legacy sync dispatch.

@roborev-ci

roborev-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (fcaeb45)

Summary verdict: One medium issue remains; no high or critical findings were reported.

Medium

  • internal/parser/opencode_provider.go:301 — A bad or stale SQLite file now aborts the entire OpenCode-family provider discovery, even after valid storage/session sources have been found. In a hybrid root, an unreadable or corrupt opencode.db, kilo.db, or mimocode.db can cause sync to drop all discovered storage sessions for that provider, whereas the legacy path synced storage and only skipped/logged the bad DB-backed portion.

    Suggested fix: Make SQLite enumeration failures root-local/non-fatal when storage discovery has usable sources, or preserve partial provider discovery results while surfacing the DB error. Add a hybrid storage-plus-invalid-DB test.


Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 9m27s), codex_security (codex/security, done, 2m5s) | Total: 11m41s

@roborev-ci

roborev-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (5ee99e5)

Medium issues remain in legacy hook consumers after the OpenCode-family provider migration.

Medium

  • cmd/agentsview/token_use.go:92
    resolveRawSessionID still only probes FindSourceFunc. OpenCode, Kilo, and MiMoCode now have nil legacy lookup hooks, so unsynced on-disk sessions for those agents are treated as unknown and session usage / token-use will not run the on-demand sync.
    Fix: Add provider-authoritative source lookup to this resolver, or keep compatible lookup hooks for migrated agents.

  • internal/ssh/resolve.go:77
    Remote SSH resolution skips file-based agents with nil DiscoverFunc. After this migration, OpenCode, Kilo, and MiMoCode are no longer emitted into the remote transfer script, so remote sync silently stops collecting their default/env-configured directories.
    Fix: Include provider-authoritative file-based agents in buildResolveScript when they have remote-resolvable roots, and update the resolver tests accordingly.


Panel: ci_default_security | Synthesis: codex, 13s | Members: codex_default (codex/default, done, 7m14s), codex_security (codex/security, done, 3m21s) | Total: 10m48s

@roborev-ci

roborev-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (d54906c)

Summary verdict: changes need follow-up for OpenCode provider correctness; no security findings were reported.

Medium

  • internal/parser/opencode_provider.go:302
    In storage/hybrid roots, a stale or malformed opencode.db makes Discover return an error after storage JSON sources have already been found, dropping valid storage sessions for the run. The legacy path discovered storage files independently and only skipped/logged the bad DB-backed portion.
    Fix: Treat SQLite metadata errors as fatal only for pure SQLite roots; in storage mode, keep discovered storage sources and skip/log DB-derived sources before continuing.

  • internal/parser/opencode_provider.go:127
    Parse overwrites the parser-computed OpenCode storage hash with the hash captured earlier during Fingerprint. If files change between fingerprinting and parsing, stored messages and file_hash can describe different snapshots, weakening archive-preservation checks that depend on that fingerprint.
    Fix: Do not overwrite a non-empty hash produced by parseOpenCodeStorageFile; only fill sess.File.Hash from the request when the parser left it empty, or recompute from the parsed snapshot.


Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 7m49s), codex_security (codex/security, done, 2m2s) | Total: 9m59s

@roborev-ci

roborev-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (65d514f)

Medium severity finding found.

Medium

  • internal/parser/opencode_provider.go:301
    Discover aborts the entire OpenCode-family provider when sqliteSources returns an error, even after valid storage-backed sources have already been discovered. In hybrid roots, a stale, corrupt, or locked opencode.db can prevent canonical storage/session files from syncing, and one bad root can also drop sources from other roots.
    Fix: Isolate SQLite discovery failures per root. In storage/hybrid mode, keep the storage sources and skip or log the SQLite fallback on error. For pure SQLite roots, continue to later roots and only return an error if no usable sources can be discovered.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 9m2s), codex_security (codex/security, done, 2m59s) | Total: 12m8s

@roborev-ci

roborev-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (289b2d6)

Medium-risk issue found: OpenCode-family discovery can now fail entirely when an optional SQLite DB is corrupt.

Medium

  • internal/parser/opencode_provider.go:301
    A bad optional SQLite DB now aborts all OpenCode-family discovery, including valid storage-backed sessions already found in the same root. In hybrid roots, a stale or corrupt opencode.db, kilo.db, or mimocode.db makes sqliteSources return an error and Discover return nil, err, so sync drops canonical storage/session* files that legacy discovery handled independently.
    Fix: Treat SQLite listing failures as scoped to the DB portion when storage mode is available, continuing with discovered storage sources. Add a regression test with a valid storage session plus an invalid DB file.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 9m39s), codex_security (codex/security, done, 2m17s) | Total: 12m3s

@roborev-ci

roborev-ci Bot commented Jun 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (11719a4)

Summary verdict: One medium issue remains; no high or critical findings were reported.

Medium

  • internal/parser/opencode_provider.go:487 - sqliteSources already has session IDs from ListOpenCodeSessionMeta, but it calls sourceRef for each row, which reopens the same SQLite DB via OpenCodeSQLiteSessionExists. This makes discovery and WAL-change classification O(n) SQLite opens for n sessions and can silently drop valid rows if any redundant probe fails.
    • Fix: Build the SourceRef directly from each meta after validating the virtual path/root, or add a source-ref helper that skips the existence probe for rows just read from the DB.

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 10m35s), codex_security (codex/security, done, 2m34s) | Total: 13m15s

@roborev-ci

roborev-ci Bot commented Jun 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (e1386cc)

Medium-risk regressions remain in legacy-hook consumers for the OpenCode-family provider migration.

Medium

  • Location: internal/parser/types.go:172, internal/parser/types.go:187, internal/parser/types.go:202
    Problem: Removing the OpenCode-family DiscoverFunc hooks makes internal/ssh/resolve.go:77 skip OpenCode, Kilo, and MiMoCode when building the remote resolve script, so remote sync no longer transfers those session roots.
    Fix: Update SSH resolution to include provider-authoritative file-based agents, or keep compatible discovery metadata/wrappers for remote transfer.

  • Location: internal/parser/types.go:172, internal/parser/types.go:187, internal/parser/types.go:202
    Problem: Removing the OpenCode-family FindSourceFunc hooks breaks unsynced session ID resolution in cmd/agentsview/token_use.go:92 and cmd/agentsview/token_use.go:113; session usage/token-use will no longer detect an on-disk OpenCode/Kilo/MiMoCode session that is not already in the DB, skipping the on-demand single-session sync and reporting not found.
    Fix: Teach resolveRawSessionID to use the provider facade for authoritative providers, or retain compatible source lookup wrappers.


Panel: ci_default_security | Synthesis: codex, 10s | Members: codex_default (codex/default, done, 8m44s), codex_security (codex/security, done, 4m11s) | Total: 13m5s

@roborev-ci

roborev-ci Bot commented Jun 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (71d614d)

Summary verdict: one Medium finding remains; no Critical or High findings were reported.

Medium

  • internal/parser/opencode_provider.go:535
    A DB/WAL path under an earlier configured root is treated as handled even when that root has no matching DB. With nested OpenCode/Kilo/MiMoCode roots, an event for the child root’s opencode.db-wal can be swallowed by the parent root and never classified for the child.
    Fix: Return ok=false when filepath.Join(root, dbName) is not a regular file, or otherwise continue checking later roots.

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 12m29s), codex_security (codex/security, done, 3m43s) | Total: 16m18s

OpenCode, Kilo, and MiMoCode share the same storage/session, message, part, and legacy SQLite source model. Moving them behind one concrete provider keeps that shared source contract explicit instead of spreading it across sync-only classifier paths.

The provider preserves storage-first discovery, hybrid SQLite fallback, duplicate filtering, child-file changed-path classification, SQLite virtual paths, composite source mtimes, storage fingerprints, and fork-specific ID relabeling.

fix(parser): classify removed opencode storage sessions

OpenCode-family storage sessions are watched recursively, so delete and rename-style events for the primary session JSON need to map back to the same provider source even after the file no longer exists. Without that syntactic fallback, provider-path sync can miss stale storage sources until a broader resync.

Move OpenCode, Kilo, and MiMoCode into shadow comparison on this branch so the stack continues as a real migration rather than an additive provider implementation.

Validation: go test -tags "fts5" ./internal/parser -run 'Test(OpenCodeProvider|OpenCodeFamilyProvider|ProviderMigration)' -count=1; go test -tags "fts5" ./internal/parser -count=1; go vet ./...; git diff --check

fix(sync): classify removed opencode session files

The provider path now handles deleted OpenCode-family storage session JSONs, but the legacy SyncPaths classifier is still active during the migration. It needs the same syntactic fallback so watcher-driven sync remains behaviorally equivalent while both forms run.

Validation: go test -tags "fts5" ./internal/sync -run 'TestEngine_ClassifyPathsOpenCodeFamilyRemovedSessionFile' -count=1; go test -tags "fts5" ./internal/parser -run 'Test(OpenCodeProvider|OpenCodeFamilyProvider|ProviderMigration)' -count=1; go test -tags "fts5" ./internal/sync -count=1; go vet ./...; git diff --check

test(sync): compare opencode family shadow parity

OpenCode, Kilo, and MiMoCode share the OpenCode-format provider implementation on this branch, so add source-level migration coverage for all three storage-mode source shapes.

The table test compares provider observation with each legacy parser and verifies session/message/data-version parity while preserving provider-computed storage fingerprints.

Validation: go test -tags "fts5" ./internal/parser ./internal/sync -run 'TestObserveProviderSourceMatchesOpenCodeFamilyLegacyParsers|TestOpenCode|TestParseOpenCode|TestParseKilo|TestParseMiMoCode|TestDiscoverKilo|TestDiscoverMiMoCode|TestProviderMigrationModes' -count=1; go test -tags "fts5" ./internal/parser ./internal/sync -count=1; go fmt ./...; go vet ./...; ./custom-gcl run --config .golangci.nilaway.yml ./internal/parser/... ./internal/sync/...; git diff --check

refactor(parser): fold opencode family into providers

OpenCode, Kilo, and MiMoCode share one on-disk format (storage/session
JSON plus message/part files, with a legacy SQLite fallback exposed as
<db>#<sessionID> virtual paths). They were still shims: the concrete
provider delegated to package-level free functions and the agents stayed
LegacyOnly, which violated the migration manifest invariant (a concrete
provider must not remain legacy-only) and left the runtime on the legacy
sync dispatch.

Make the three providers authoritative and own their behavior. One
shared openCodeFormatProvider implementation is parameterized per agent
by a format struct (SQLite filename, storage session subdir, ID prefix);
Kilo and MiMoCode reuse the OpenCode storage and SQLite readers and only
relabel the parsed session onto their own agent and ID prefix, so the
parse/discover/find logic is not duplicated three times.

The 15 legacy free functions (Discover/Find/ParseFile/ParseSession/
ParseSQLiteVirtualPath for each of opencode/kilo/mimocode) are deleted.
ParseOpenCodeFile/ParseOpenCodeSession move to unexported helpers
(parseOpenCodeStorageFile/parseOpenCodeDBSession) that the provider spec
drives. SQLite virtual-path resolution now goes through the
provider-neutral ParseVirtualSourcePathForBase, so engine, parsediff, and
resume callers no longer reference deleted parsers.

Remove the opencode-family legacy engine dispatch: the classify blocks
and classifyOpenCodeFormatPath, the processFile arm and
processOpenCodeFormat, the DB-backed sync pass, single-session resync,
and orphaned helpers. Runtime now routes through provider changed-path
classification and processProviderFile. Because these agents now flow
through file discovery, the resync empty-discovery guard tracks a
non-container discovered count so a self-preserving storage store cannot
mask plain file-backed sessions whose directories went empty, and
parse-diff discovers them through the provider facade.

fix(sync): skip provider-authoritative agents in parse-diff db synthesis

parseDiffDatabaseSources synthesized a raw opencode.db/kilo.db source so
the legacy processOpenCode fan-out re-parsed every DB session. Once those
agents became provider-authoritative, parseDiffProviderSources already
enumerates their DB sessions through the provider, which applies the
storage-ID filter that drops a file-backed storage session's stale db
row. Re-adding the raw db then double-counted those sessions and parsed
the filtered storage row, surfacing a spurious ParseError and an extra
examined file. Skip agents that have dropped their DiscoverFunc; the Kiro
data.sqlite3 synthesis still runs because Kiro keeps its legacy
DiscoverFunc until its own fold.

refactor(parser): delete opencode legacy whole-database parser

ParseOpenCodeDB parsed every session in an OpenCode SQLite database, but
the provider (and the Kilo/MiMoCode reuse) routes per-session through
parseOpenCodeDBSession, so the free function survived only as
test-exercised dead production code.

Delete ParseOpenCodeDB along with the orphaned loadOpenCodeSessions and
the OpenCodeSession bundle type; loadOpenCodeProjects stays since the
per-session path also resolves worktrees through it. The retained parse
tests reproduce the whole-database walk with the provider's own
primitives (ListOpenCodeSessionMeta + parseOpenCodeDBSession).

fix(sync): preserve parse-diff virtual sqlite identity

OpenCode-family providers expose SQLite sessions as per-session virtual sources. Parse-diff was still collapsing those db#session paths to the shared database path before error attribution, presence sweep, and limit accounting, which could apply one session's parse failure or omitted sample to every sibling in the DB.

Keep exact source keys for OpenCode, Kilo, and MiMoCode provider virtual SQLite paths while retaining shared-base grouping for true physical multi-session jobs. Source existence checks still stat the physical DB path so virtual identities do not look missing.

Validation: go test -tags "fts5" ./internal/sync -run 'TestParseDiffProviderVirtualSQLite(ErrorUsesExactSource|PresenceUsesExactSource|LimitUsesExactSource)|TestStripVirtualSourceSuffixVisualStudioCopilot' -count=1; go test -tags "fts5" ./internal/sync -run 'TestParseDiff(CoversMixedOpenCodeRoot|CoversMixedKiloRoot|ProviderVirtualSQLite|PresenceSweep|LimitNewestFirst|ReportHasFailures)' -count=1; go test -tags "fts5" ./internal/parser -run 'Test(OpenCodeProvider|OpenCodeFamilyProvider|Kilo|MiMoCode)' -count=1; go vet ./...; git diff --check
OpenCode-family roots can carry both filesystem storage sessions and an
optional SQLite DB (opencode.db/kilo.db/mimocode.db). When the DB was
present but corrupt, sqliteSources returned an error and Discover returned
nil, err, dropping the valid storage-backed sessions already collected for
that root. Legacy discovery handled the two sources independently and only
logged DB listing failures. Scope the SQLite failure to the DB portion when
storage mode is available: log and continue with the storage sources, while
still propagating context cancellation and failing SQLite-only roots.
sqliteSources listed every session row from the SQLite DB and then called sourceRef for each, which reopened the same DB via OpenCodeSQLiteSessionExists once per row. For n sessions that is n redundant SQLite opens on every discovery and every WAL-change classification, and a row whose redundant probe failed would be silently dropped even though it was just read from that DB.

Build the SourceRef directly from the listed metadata, keeping the virtual-path parse and under-root validation but skipping the existence probe the caller already satisfied.
@roborev-ci

roborev-ci Bot commented Jun 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (29c9b7d)

Medium issue found: remote SSH sync loses OpenCode-family directory targets after provider discovery migration.

Medium

  • internal/parser/types.go:172 - OpenCode, Kilo, and MiMoCode now have DiscoverFunc == nil, but SSH remote discovery still skips file-based agents without a DiscoverFunc. Remote sync will no longer emit OPENCODE_DIR, KILO_DIR, or MIMOCODE_DIR targets, so those sessions silently stop syncing from remote hosts.
    • Fix: update the SSH resolver to include provider-authoritative file-based agents via their env/default dirs, or keep the legacy discover hooks until remote sync is migrated. Add resolver coverage for these agents.

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 10m1s), codex_security (codex/security, done, 1m59s) | Total: 12m6s

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant