Skip to content

Windows cross-platform compatibility fixes#62

Closed
Day-Anger wants to merge 17 commits into
iHildy:mainfrom
Day-Anger:pr/windows-bugfixes
Closed

Windows cross-platform compatibility fixes#62
Day-Anger wants to merge 17 commits into
iHildy:mainfrom
Day-Anger:pr/windows-bugfixes

Conversation

@Day-Anger
Copy link
Copy Markdown

Summary

This PR makes opencode-synced fully functional on Windows (Electron/Node.js Desktop) while maintaining backward compatibility with Linux/macOS (Bun Terminal).

Changes

Path & Platform fixes

  • \�xpandHome(): use template string instead of \path.join\ (which produces backslashes on Windows)

  • ormalizePath(): normalize backslashes \\ → /\ before comparison
  • \platformJoin(): posix vs win32 join based on platform parameter
  • CONFIG_DIRS: plural \plugins, \commands\ for Desktop compatibility

SQLite robustness

  • \�sSQLValue(): wrapper for all SQLite parameters to handle edge cases
  • \INSERT OR REPLACE\ instead of DELETE+INSERT (atomic upsert)
  • Skip \writeSessionsToDB\ when remote directory is empty
  • .broken\ file recovery for corrupt JSON sessions

Cross-platform shell

  • \createNodeShell(): async \�xec\ (not \�xecSync) with all values in double quotes
  • Fallback for \ctx.$\ on Desktop

Misc fixes

  • \parseFrontmatter(): strip UTF-8 BOM (\\\uFEFF), normalize CRLF → LF, fallback to first line
  • \�uildSyncPlan(): exclude \opencode.db\ and \storage/\ from sync
  • Session merge: fixed
    emote!\ non-null assertion → proper type guard

  • eadSessionsFromDir: handle legacy columnar JSON format

Testing

All 79 tests pass on Windows. Cross-platform tests use \platform\ parameter.

Day-Anger added 17 commits May 22, 2026 15:39
- Add notify() helper: emoji + toast + log in one call
- startupSync: 🔄 Syncing..., ✅ ready, ❌ error
- pull: 📥 Pulling..., 📥 result
- push: 📤 Pushing..., 📤 result
- link: 🔗 Linking..., 🔗 result
- init: 🚀 Sync configured
- session-db.ts: read/write sessions from SQLite and JSON dir
- session-merge.ts: syncSessions() — union-merge per record
  (messages, parts, todos, session_messages, shares by id+time_updated)
- service.ts: integrate syncSessions in pull, push, runStartup
- @types/node: ^22.15.0 for node:sqlite types
- Config field includeProjects (bool, default false)
- resolveProjectsFilePath(): Windows → %APPDATA%\ai.opencode.desktop\, Linux/macOS → /opencode
- SyncItem with preserveWhenMissing: true for graceful handling
- shell.ts: createNodeShell() wraps child_process.execSync with Bun-compatible tagged-template API
- service.ts: detect ctx.$ === undefined at createSyncService entry, replace with createNodeShell()
- Enables gh/git commands in OpenCode Desktop (Electron/Node.js)
…ned/objects

- All insert* functions now pass values through v() = asSQLValue()
- asSQLValue() now also serializes objects/arrays via JSON.stringify
- Fixes 'Provided value cannot be bound to SQLite parameter' on startup
projects-merge.ts: syncGlobalData() — union-merge проектов
  * server.projects.local — объединение по worktree
  * layout.page.lastProjectSession — объединение по директории, новее at
  * остальные ключи — если нет в локальном, берётся с удалённого
service.ts: вызов syncGlobalData в pull/push/startup
paths.ts: удалён старый SyncItem для простого копирования
При пустой директории remoteSessions (первый запуск, нет
удалённых сессий) syncSessions пишет только JSON-файлы,
не перезаписывая локальную БД. Это избегает ошибки
'Provided value cannot be bound to SQLite parameter 1',
которая возникала из-за NOT NULL constraint в таблицах
session/message/part при DELETE+INSERT
- Убирает deleteSession + каскадное удаление
- Все insert* функции заменены на upsert* (INSERT OR REPLACE)
- Избегает NOT NULL constraint violations при перезаписи сессий
- Предотвращает ошибку 'Provided value cannot be bound to SQLite parameter 1'
- Убрано копирование opencode.db и storage/* (SESSION_DIRS)
- Сессии синхронизируются только через per-session JSON merge (syncSessions)
- Предотвращает затирание локальных сессий при pull
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request significantly enhances the synchronization service by introducing diff-based directory syncing, project data merging, and a new session merging system using SQLite. It also improves cross-platform path handling, adds a custom shell executor, and implements a new notification system. Feedback identifies critical compatibility issues with the node:sqlite module in older Node.js environments and potential shell injection vulnerabilities in the Windows shell implementation. Additionally, the review highlights performance concerns regarding synchronous I/O and inefficient database queries, alongside suggestions for code deduplication and minor cleanup in the .gitignore file.

Comment thread src/sync/session-db.ts
@@ -0,0 +1,523 @@
import fs from 'node:fs';
import path from 'node:path';
import { DatabaseSync } from 'node:sqlite';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The node:sqlite module (DatabaseSync) was introduced in Node.js v22.5.0. Many current environments, including stable versions of Electron (e.g., Electron 33 uses Node.js 20.18.0), do not yet support this built-in module. Using it will cause the plugin to crash on those platforms. Consider using a more widely compatible library like better-sqlite3 or ensuring the target environment meets the minimum version requirement.

Comment thread src/sync/shell.ts
if (i < values.length) {
const v = values[i];
const str = typeof v === 'string' ? v : String(v);
cmd += `"${str.replace(/"/g, '\\"')}"`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The manual escaping of double quotes using "\"" is often insufficient for cmd.exe on Windows, which typically uses "" to escape quotes within a double-quoted string. This can lead to command execution failures or potential shell injection vulnerabilities when handling paths or messages containing special characters. Since this PR aims for Windows compatibility, consider using child_process.spawn with an arguments array, which avoids the shell and its escaping complexities entirely.

Comment thread .gitignore
Comment on lines +16 to +17
package-lock.json
package-lock.json
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Redundant duplicate entry for package-lock.json.

package-lock.json

Comment on lines +30 to +41
function readGlobalData(filePath: string): GlobalData | null {
try {
const raw = fs.readFileSync(filePath, 'utf-8');
return JSON.parse(raw) as GlobalData;
} catch {
return null;
}
}

function writeGlobalData(filePath: string, data: GlobalData): void {
fs.writeFileSync(filePath, JSON.stringify(data, null, '\t'), 'utf-8');
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The functions readGlobalData and writeGlobalData use synchronous I/O (fs.readFileSync, fs.writeFileSync). In a Node.js environment, especially for a desktop application (Electron), synchronous I/O blocks the event loop and can cause UI unresponsiveness or performance degradation. These should be refactored to use fs.promises.

Comment thread src/sync/session-db.ts
Comment on lines +170 to +176
return rows.map((row) => {
const msg = row as unknown as Message;
const partRows = db
.prepare(`SELECT ${PART_COLUMNS.join(', ')} FROM part WHERE message_id = ?`)
.all(msg.id) as Record<string, unknown>[];
msg.parts = partRows.map((pr) => pr as unknown as Part);
return msg;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This implementation suffers from an $N+1$ query problem, executing a separate SQL query for parts for every message in the session. It is more efficient to fetch all parts for the session in a single query and group them by message_id in memory.

Comment thread src/sync/session-db.ts
Comment on lines +263 to +265
(msg as Record<string, unknown>).parts = (parsed.parts ?? [])
.filter((p: unknown[]) => p[1] === msg.id)
.map((p: unknown[]) => arrayToObject(p, parsed.columns.parts));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This filtering logic has $O(N*M)$ complexity where $N$ is the number of messages and $M$ is the number of parts. For sessions with many messages and parts, this can be slow. It is more efficient to group parts by message_id into a Map first, then look them up while mapping messages.

Comment thread src/sync/session-db.ts
Comment on lines +334 to +352
if (Array.isArray(parsed.session) && parsed.columns?.session) {
const session = arrayToObject(parsed.session, parsed.columns.session);
const messages = (parsed.message ?? []).map((row: unknown[]) => {
const msg = arrayToObject(row, parsed.columns.message);
(msg as Record<string, unknown>).parts = (parsed.parts ?? [])
.filter((p: unknown[]) => p[1] === msg.id)
.map((p: unknown[]) => arrayToObject(p, parsed.columns.parts));
return msg;
});
const session_messages = (parsed.session_messages ?? []).map((row: unknown[]) =>
arrayToObject(row, parsed.columns.session_message)
);
return {
session: session as unknown as SessionMeta,
messages,
session_messages,
todos: [],
session_shares: [],
} as Session;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for parsing the legacy columnar JSON format is duplicated from readSessionFromFile. This should be extracted into a shared helper function to improve maintainability and reduce the risk of logic divergence.

Comment thread src/sync/session-merge.ts
if (hasRemote && local) {
toWrite.push(merged);
}
writeSessionToFile(sessionsDir, merged);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Calling writeSessionToFile (which uses synchronous fs.writeFileSync) inside a loop can lead to significant performance degradation when processing a large number of sessions. This blocks the event loop for the duration of all I/O operations. Consider using asynchronous file writes.

@Day-Anger Day-Anger closed this May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant