Skip to content

feat(parser): add Windsurf workspace chat support (#696) - #998

Merged
wesm merged 12 commits into
kenn-io:mainfrom
rodboev:pr/windsurf-workspace-chat
Jul 8, 2026
Merged

feat(parser): add Windsurf workspace chat support (#696)#998
wesm merged 12 commits into
kenn-io:mainfrom
rodboev:pr/windsurf-workspace-chat

Conversation

@rodboev

@rodboev rodboev commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Agentsview currently has no Windsurf parser, so Windsurf workspace chat history is invisible to sync and import. Windsurf stores workspace chat in workspaceStorage/*/state.vscdb, and the payloads are close enough to the existing VS Code family that the right implementation is an adapter over the shared VS Code parser, not a separate parser stack.

This adds Windsurf as a first-class parser provider. The provider discovers workspaceStorage/*/state.vscdb, reads the known chat rows from ItemTable, translates the tab-container shape into VS Code session JSON when needed, and then reuses the existing VS Code chat parser while relabeling the normalized session and usage events as windsurf. Each parseable session is exposed as a state.vscdb#sessionID virtual source so discovery, fingerprinting, watch updates, and restart-time lookup all stay scoped to one session instead of one shared database file.

The rest of the change is the minimum consumer routing needed for a new DB-backed virtual source family: generic source-path resolution, per-session parse-diff scoping, sync-by-path lookup, session export, and narrow workspaceStorage remote targeting. The SSH resolver change stays because the longer resolver script hit the Windows command-length limit in CI, and sending the script over stdin fixes that at the transport layer without changing parser ownership.

This PR intentionally keeps Cascade trajectory protobufs, encrypted .pb cache files, and other non-workspace Windsurf storage out of scope. Those are a separate format surface and can be handled later without blocking workspace chat support. The remaining proof gap is format ownership: validation here is based on synthetic SQLite fixtures and public non-owner references, not a captured real Windsurf session database, so the claim stays scoped to the handled workspace chat shapes rather than all Windsurf transcript variants.

Closes #696

@roborev-ci

roborev-ci Bot commented Jul 5, 2026

Copy link
Copy Markdown

roborev: Combined Review (c8366f4)

Medium-risk issues remain in the Windsurf parser around deleted DB handling and numeric chat bubble type decoding.

Medium

  • internal/parser/windsurf_provider.go:294
    Problem: Fingerprint returns an error when state.vscdb has been deleted. SourcesForChangedPath can still return stored virtual sources for that DB, and Parse has deletion tombstone handling, but the engine fingerprints before parsing, so deleted Windsurf chats can remain archived.
    Fix: Return an empty keyed fingerprint on os.IsNotExist so Parse can emit ForceReplace/SkipNoSession; add a deletion changed-path test.

  • internal/parser/windsurf_provider.go:637
    Problem: windsurfChatBubble.Type is a string, but isAssistant tries to support "2" numeric-style assistant types. If actual chat data uses JSON numbers like "type": 2, unmarshalling the whole tab container fails and no sessions are discovered.
    Fix: Decode type with a custom string-or-number helper or json.RawMessage, and add coverage for numeric user/assistant bubble types.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 7m50s

@roborev-ci

roborev-ci Bot commented Jul 5, 2026

Copy link
Copy Markdown

roborev: Combined Review (6b93c2a)

Medium findings:

  • internal/parser/windsurf_provider.go:566 - Windsurf persists file_path as a virtual <state.vscdb>#<session> path, but existing source-path resolution only understands Visual Studio Copilot and Aider virtual paths. Sync-by-ID, SyncPaths, source export, and mtime lookup may treat it as a literal path and fail or return empty data. Add Windsurf virtual-path parsing and route stored Windsurf paths through provider-backed session resolution/export handling.

  • internal/parser/types.go:293 - The Windsurf provider registers the whole Windsurf/User profile as a file-based default root. Once provider-authoritative, remote archive sync can tar and stream unrelated editor state such as settings, extension/global storage, or other secret-bearing files. Narrow Windsurf archive targets to the required workspaceStorage DB/manifest files, or add provider-specific target selection that transfers only session data while preserving import expectations.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 10m51s

@roborev-ci

roborev-ci Bot commented Jul 5, 2026

Copy link
Copy Markdown

roborev: Combined Review (1602c4c)

High-level verdict: changes need fixes before merge due to one high-risk Windsurf archive deletion path and one medium virtual-path parsing bug.

High

  • Location: internal/parser/windsurf_provider.go:101
  • Problem: A missing state.vscdb is treated as ForceReplace + SkipNoSession, so a remove event for the whole Windsurf workspace DB can cause archived Windsurf sessions to be parser-excluded and deleted. Whole source-file disappearance should preserve the archive.
  • Fix: Do not set ForceReplace when the physical DB is missing; only force-replace when the DB still exists and the selected session row is absent. Also avoid emitting stored tombstones for a removed whole DB.

Medium

  • Location: internal/parser/windsurf_provider.go:753
  • Problem: SplitWindsurfVirtualPath accepts any path containing #, so ordinary source files with # in their names can be misclassified as Windsurf DB virtual paths by session export, ResolveSourceFilePath, and sync paths.
  • Fix: Scope the parser to Windsurf containers, e.g. split via ParseVirtualSourcePathForBase(path, windsurfStateDBName), and add coverage for non-Windsurf # paths.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 8m25s

@rodboev
rodboev force-pushed the pr/windsurf-workspace-chat branch from 1602c4c to a403234 Compare July 5, 2026 08:08
@roborev-ci

roborev-ci Bot commented Jul 5, 2026

Copy link
Copy Markdown

roborev: Combined Review (a403234)

Summary: One medium-severity correctness issue needs attention before merge.

Medium

  • Location: internal/parser/windsurf_provider.go:618
    Problem: Empty raw sessionId values fall back to the constant "session" while the JSON payload is left unchanged. parseVSCodeCopilotData then falls back to filepath.Base(virtualPath), so multiple workspaces without embedded session IDs parse to the same windsurf:state.vscdb#session ID and overwrite each other.
    Fix: Generate a stable unique fallback from the workspace/DB identity and make the parsed session ID use that value, with a regression test covering two workspaces missing sessionId.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 12m36s

@rodboev
rodboev force-pushed the pr/windsurf-workspace-chat branch from a403234 to 090c8ab Compare July 5, 2026 08:21
@roborev-ci

roborev-ci Bot commented Jul 5, 2026

Copy link
Copy Markdown

roborev: Combined Review (090c8ab)

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

Medium

  • internal/service/direct.go:428 - session sync <path-to-state.vscdb> syncs Windsurf sessions but then fails to resolve the synced session because resolveSessionIDByPath only expands physical container paths to virtual path#... rows for Visual Studio Copilot.
    • Fix: Extend the virtual-container lookup to Windsurf state.vscdb paths and add single-match plus ambiguous-match tests.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 9m55s

@rodboev
rodboev force-pushed the pr/windsurf-workspace-chat branch from 090c8ab to da36a2e Compare July 5, 2026 08:35
@roborev-ci

roborev-ci Bot commented Jul 5, 2026

Copy link
Copy Markdown

roborev: Combined Review (da36a2e)

Review verdict: changes need fixes before merge due to two Medium Windsurf data-loss/reporting risks.

Medium

  • internal/sync/parsediff.go:409 - Windsurf emits one virtual source per chat as state.vscdb#<sessionID>, but state.vscdb is not included in perSessionDBVirtualSourceBases. Parse-diff can collapse all Windsurf chats in the same DB to one source key, so --limit may falsely report unparsed sibling chats as missing, and a per-session parse error can fan out to every sibling row. Add state.vscdb/windsurfStateDBName to the per-session virtual DB base list and cover a multi-tab Windsurf parse-diff limit case.

  • internal/parser/windsurf_provider.go:640 - Malformed Windsurf chat JSON is treated as “no records” instead of a parse error. On changed-path sync, stored source paths are appended, Parse then sees sql.ErrNoRows and force-replaces the archived session, so transient or corrupt chatdata can delete previously synced sessions. Return an error when the chatdata value cannot unmarshal into any supported shape, so sync fails safely instead of interpreting it as deletion.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 13m7s

@rodboev
rodboev force-pushed the pr/windsurf-workspace-chat branch from da36a2e to 0875538 Compare July 5, 2026 08:54
@roborev-ci

roborev-ci Bot commented Jul 5, 2026

Copy link
Copy Markdown

roborev: Combined Review (0875538)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 12m45s

@rodboev
rodboev marked this pull request as draft July 5, 2026 11:34
@rodboev
rodboev marked this pull request as ready for review July 5, 2026 12:38
- Prevent resolver script truncation on Windows
- Handle Windsurf deletion and numeric bubbles
- Drop obsolete SSH command helper
- Retry corrupted benchmark capture
- Keep benchmark output parseable during setup
- fix(parser): scope windsurf virtual sources (kenn-io#696)
@wesm
wesm force-pushed the pr/windsurf-workspace-chat branch from 0875538 to 439a9a4 Compare July 7, 2026 18:03
@roborev-ci

roborev-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (439a9a4)

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

Medium

  • internal/parser/windsurf_provider.go:330 - Windsurf computes and stores a source hash, but the sync freshness path does not require that hash for Windsurf. A same-size/same-mtime change to state.vscdb, WAL, or workspace.json can be skipped by DB freshness or skip cache, leaving stale messages.
    • Fix: Make the Windsurf fingerprint hash stable/content-based and include AgentWindsurf in the provider hash freshness checks, with a same-size/same-mtime regression test.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 13m58s

VALID (fixed): kenn-io#1 -- Windsurf same-size/same-mtime SQLite rewrites need to compare the provider file_hash before taking skip-cache or stored-freshness shortcuts.

Windsurf now uses a component-labeled content digest for state.vscdb, WAL/SHM, and workspace.json so the fingerprint is stable across roots while still moving when parsed source content changes. The sync freshness allowlist now requires that hash for Windsurf, matching the composite fingerprint contract.

The commit also includes the repository hook's automatic Qoder slices.Backward modernization; without staging that generated fix, the pre-commit hook re-applies it and aborts before the Windsurf fix can be committed.
@roborev-ci

roborev-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (cd4edb2)

Verdict: One medium issue should be addressed before merge.

Medium

  • internal/remotesync/resolve.go:89: resolveWindsurfTarget allows the entire <Windsurf User>/workspaceStorage directory as a remote-sync target. A client with access to /api/v1/remote-sync/archive can request that target and receive a recursive tar archive, exposing unrelated Windsurf and extension workspace state beyond the session data the parser needs.

    Fix: Avoid archiving workspaceStorage wholesale. Enumerate only the required files, or generate a sanitized Windsurf export containing only the needed chat-data records and workspace metadata.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 15m6s

VALID (fixed): kenn-io#1 -- Windsurf remote-sync targets exposed workspaceStorage as a recursive archive root.

Windsurf targets now separate import roots from archived files, so the HTTP archive endpoint streams only state.vscdb, sidecars, and workspace.json. Dir-only archive requests for file-scoped Windsurf targets are rejected instead of falling back to recursive tar.
@roborev-ci

roborev-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (08a4011)

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

Medium

  • Location: internal/ssh/resolve.go:80
  • Problem: SSH remote sync still resolves Windsurf to the whole workspaceStorage directory, and buildTarCommand archives directory targets recursively. This bypasses the new file-scoped TargetSet.Files behavior used by HTTP remote sync, so the default SSH transport can still transfer unrelated workspace files such as extension secrets.
  • Fix: Extend SSH resolution/transfer to carry file-scoped Windsurf targets (state.vscdb, state.vscdb-wal, state.vscdb-shm, workspace.json) and pass them through import as TargetSet.Files, or have the SSH tar command enumerate only those files for Windsurf.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 13m43s

wesm added 2 commits July 7, 2026 15:56
VALID (fixed): kenn-io#1 -- SSH remote sync still resolved Windsurf to workspaceStorage and recursively tarred unrelated workspace files.

The SSH resolver now emits the Windsurf import root separately from agent-scoped file records, so transfer can include only state.vscdb, WAL/SHM sidecars, and workspace.json while import keeps the same root metadata. This aligns the default SSH transport with the HTTP remote-sync file-scoped behavior and prevents dir-only Windsurf roots from being archived recursively.
Windows CI runs the SSH resolver script through MSYS sh, which rewrites native temp paths from C:/... to /c/.... The test was asserting exact native drive-letter records even though the behavior under test is the resolver record type and the Windsurf path shape.

Match record prefixes and path suffixes instead, preserving coverage that only the Windsurf root and allowed session files are emitted while making the assertion independent of the shell path dialect.
@roborev-ci

roborev-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (95fec13)

Remote sync has one medium-risk scalability issue; no high or critical findings were reported.

Medium

  • internal/ssh/transfer.go:36 - Windsurf remote sync expands every workspace DB, WAL/SHM, and workspace.json into a single tar ... <paths> shell command. With many Windsurf workspaces, this can exceed the remote shell/exec argument length and fail before transfer starts. Feed paths to tar via stdin or a temporary list file, or batch transfers into bounded chunks.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 14m11s

wesm added 3 commits July 7, 2026 16:45
VALID (fixed): kenn-io#1 -- SSH Windsurf remote sync expanded all file-scoped targets into one tar argv list.

The SSH transfer now sends the tar script over stdin and feeds selected paths to tar with -T -, so large Windsurf exports do not consume ssh/exec argument space. Message-write benchmarks also silence slow-operation logs because those logs can interleave with benchmark rows and leave benchgate with too few parseable candidate samples.
Baseline benchmarks run from the merge base, so they can predate candidate fixes that keep benchmark output parseable. Treat baseline syntax errors like other partial baseline conditions: report them and gate the parseable data, while candidate syntax errors remain a hard configuration failure.
The SSH tar script is executed by a POSIX remote shell, but the unit test runs it locally. On Windows, MSYS sh receives native temp paths that do not represent remote POSIX paths, so keep the structural coverage there and run the end-to-end script execution only on Unix-like hosts.
@roborev-ci

roborev-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (360fc2e)

Summary verdict: Two medium-risk Windsurf remote-sync issues should be addressed before merge.

Medium

  • internal/remotesync/resolve.go:128
    Windsurf remote sync exposes the raw state.vscdb workspace database, plus WAL/SHM files, through the HTTP archive path. A bearer-token holder authorized for agentsview API access could download the full workspace state DB, including extension state or recently deleted WAL content that may contain non-session secrets.
    Fix: Do not expose raw state.vscdb via remote sync. Query server-side and archive a sanitized artifact containing only Windsurf chat keys and required metadata, or generate a filtered temporary SQLite/JSON source for import.

  • internal/parser/windsurf_provider.go:323 and internal/remotesync/resolve.go:130
    Windsurf fingerprints and remote-sync targets include state.vscdb-shm. SQLite SHM files are transient WAL-index/lock files and may be rewritten or removed during normal SQLite activity. Including them can make unchanged sessions appear stale, and exact remote archive selection can fail if the file disappears between resolution and archive creation.
    Fix: Drop -shm from the Windsurf fingerprint and remote file list. If -wal remains, treat it as optional or re-resolve at archive time so checkpointed WAL removal does not abort sync.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 14m18s

wesm added 2 commits July 7, 2026 18:18
VALID (fixed): kenn-io#1 -- HTTP remote sync could stream raw Windsurf workspace SQLite state and sidecars to any bearer-token holder allowed to request remote archives.

VALID (fixed): kenn-io#2 -- Windsurf freshness and remote target selection treated transient SQLite SHM files as durable session inputs.

Windsurf HTTP archives now synthesize a filtered state.vscdb containing only the chat keys the parser consumes, while preserving workspace metadata for project naming. SHM is no longer watched, fingerprinted, or resolved, and SSH tar input skips paths that disappear after resolution so optional WAL sidecars do not abort transfer.
The Windsurf freshness regression test only needed an initial stored session, but it reached into writeBatch directly. Current main changed that private helper's return shape, so the PR merge ref failed to compile even though the branch alone still built.

Seed the initial row through SyncAll and derive the old hash-scoped skip-cache key from stored session metadata instead. That keeps the test focused on Windsurf freshness behavior and avoids coupling it to writeBatch internals.
@roborev-ci

roborev-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (19f29b4)

Medium issue found: tar path-list mode needs option-shaped path protection.

Medium

  • internal/ssh/transfer.go:55
    The new tar -T - path-list mode no longer protects option-shaped paths. Entries read via -T can be interpreted by GNU tar as options unless verbatim mode is used, so a resolved path beginning with - after trimming the leading slash can alter tar behavior or fail. The previous argv form used --, which avoided this issue.

    Fix: Emit file-list entries in a verbatim-safe form, such as prefixing relative paths with ./ after the existence check, or use tar’s verbatim/null file-list mode where supported.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 18m17s

VALID (fixed): kenn-io#1 -- SSH tar path-list mode could treat a dash-leading relative archive input as a tar option.

Prefix streamed entries with ./ before they reach tar -T -, preserving root-relative behavior under -C / while keeping file-list parsing safe on GNU tar.
@roborev-ci

roborev-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (e650e31)

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

Medium

  • internal/ssh/transfer.go:53 — SSH tar paths are emitted as a newline-delimited tar -T - list. A resolved path containing an embedded newline is shell-quoted as one argument to av_emit_tar_path, passes the existence check, then printf '%s\n' splits it into multiple tar entries. This can archive unintended readable paths or fail on crafted filenames under a scanned agent directory.

    Suggested fix: Emit a NUL-delimited file list and use tar’s null-file-list mode where supported, or reject/skip resolved paths containing \n/\r before building the tar script.


Reviewers: 2 done | Synthesis: codex, 7s | Total: 13m59s

VALID (fixed): kenn-io#1 -- SSH tar path-list mode could split newline-bearing resolved paths into multiple tar entries.

Reject CR/LF/NUL target paths before generating the remote shell script. This keeps newline-delimited tar -T input from reinterpreting one existing path as multiple archive requests while preserving the portable tar invocation used for normal paths.
@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (234f411)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 16m31s

@wesm
wesm merged commit de6fb8c into kenn-io:main Jul 8, 2026
13 checks passed
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.

please support windsurf

2 participants