feat: Improve File Sync DX — hardening, factory, Convex adapter, docs

[gopinav]

What & Why

Security

1. Path traversal protection — remote DB paths could write anywhere on disk. Added assertSafePath() with OWASP canonicalize-then-verify + symlink escape detection.
2. Sync denylist — .env, credentials, .git/, node_modules/, sync meta-files blocked from syncing regardless of user config.
3. Input validation — appId, ownerId, userUid validated to prevent filter injection in Supabase Realtime.
4. Supabase client-side owner filter — Realtime only supports one server filter. Added client-side owner_id check to prevent cross-tenant data leaks.

Resilience

5. Init flag fix — set after success, not before. Transient failures no longer permanently disable sync.
6. stop() with AbortController — graceful shutdown: cancels ops, drains retry queue + in-flight pushes, closes watchers, disposes adapter.
7. Retry queue — failed pushes queued (max 100), retried every 30s with dead letter log for evicted entries.
8. dispose() on adapter interface — prevents leaked gRPC channels and WebSocket connections.

Performance

9. Parallel startup sync — p-limit(10) drops 100-file startup from ~10s to ~1s.
10. Remove read-before-write — in-memory content hash instead of adapter.get() on every push.
11. Content hashes — SHA-256 hashes + LRU merge base cache instead of storing full file contents.
12. LCS backtrack fix — push()+reverse() instead of unshift() (O(N) vs O(N^2)).

Race Conditions

13. Per-file push serialization — pushInFlight map prevents hash corruption from rapid writes.
14. Content-hash echo suppression — replaces TTL-based dedup that dropped legitimate collaborator edits.
15. SSE crash protection — safeWrite wrapper prevents res.write() crash on closed connections.

DX — Factory & Wiring

16. createFileSync() factory — zero-config from env vars, returns discriminated union (disabled/error/ready).
17. Default template wired — sync, diagnostic endpoint, shutdown, agent conflict notification.
18. useFileSyncStatus() hook — SSE subscription + initial REST fetch.
19. Agent-native parity — sync status file, conflict notification, AGENTS.md updated.

Convex Adapter

20. ConvexFileSyncAdapter — third backend alongside Firestore/Supabase. Duck-typed ConvexClient interface.
21. Snapshot-diffing subscribe — content-hash comparison with processingChain serialization for safe reconnection handling.
22. Tenant isolation — Convex functions require app/ownerId on get/remove. Partial update support for set().
23. Factory integration — FILE_SYNC_BACKEND=convex + CONVEX_URL (HTTPS validated).

Documentation

24. File Sync docs page — 10-section guide replacing "Database Adapters": overview, quick start, config, 3 backend setup guides (Firestore/Supabase/Convex with copyable schema+functions), factory pattern, diagnostics, agent parity, custom adapter guide.
25. Redirect from /docs/database-adapters to /docs/file-sync.

Other

26. Doc ID separator — __ → : to fix collisions. Runtime warning for legacy format.
27. Type safety — branded types, typed EventEmitter, eliminated any from sync engine.

Testing

• 182 tests total (51 new), all passing
• Convex adapter: CRUD + subscribe diffing (added/modified/removed/unchanged)
• FileSync integration: startup sync, remote changes, echo suppression, path traversal
• Config security: assertSafePath, denylist, validation, hashing
• Factory: all result states for all 3 backends

Post-Deploy Monitoring & Validation

No additional operational monitoring required: This is a framework library change. Sync is opt-in via FILE_SYNC_ENABLED=true and disabled by default — zero behavior change for existing apps.

Before & After

Enabling file sync (before)

// Developer had to discover undocumented exports, manually instantiate,
// wire SSE, handle shutdown — with zero guidance
import { FileSync } from "@agent-native/core/adapters/sync";
import { FirestoreFileSyncAdapter } from "@agent-native/core/adapters/firestore";
import admin from "firebase-admin";

const app = admin.initializeApp({ credential: admin.credential.applicationDefault() });
const adapter = new FirestoreFileSyncAdapter(() => app.firestore().collection("files"));
const sync = new FileSync({ appId: "my-app", ownerId: "shared", contentRoot: "./data", adapter });
await sync.initFileSync();
// SSE wiring? Shutdown? Conflict handling? Good luck.

Enabling file sync (after)

# .env — that's it
FILE_SYNC_ENABLED=true
FILE_SYNC_BACKEND=firestore
GOOGLE_APPLICATION_CREDENTIALS=./service-account.json

// server/index.ts — factory handles everything
const syncResult = await createFileSync({ contentRoot: "./data" });
// Returns { status: "ready", fileSync, sseEmitter, shutdown }
// SSE wiring, shutdown hooks, conflict notification — all bundled

Handling remote changes (before)

// Every push called adapter.get() first — doubled latency
const existing = await adapter.get(docId);
if (existing && existing.data?.content === content) return;
// TTL-based echo suppression — 3s window dropped real collaborator edits
if (Date.now() - lastPulled < 3000) return; // oops, real change dropped

Handling remote changes (after)

// In-memory content hash — zero network calls on hot path
const hash = hashContent(content);
if (lastSyncedHash.get(relPath) === hash) return;
// Content-hash echo suppression — never drops real changes
if (pushedHash === incomingHash) continue; // genuine echo
// Different hash = real remote change, always processed

Path security (before)

// Remote database path written directly to disk — no validation
const absPath = path.resolve(projectRoot, data.path);
fs.writeFileSync(absPath, content); // ../../etc/cron.d/backdoor? Sure!

Path security (after)

// Every remote path validated before touching filesystem
const absPath = assertSafePath(projectRoot, filePath); // throws on traversal
assertNotSymlink(absPath); // throws on symlink escape
fs.writeFileSync(absPath, content);

Startup sync (before)

// Sequential — 100 files × 100ms = 10 seconds blocking startup
for (const file of dirtyFiles) {
  await adapter.set(docId, payload); // one at a time
}

Startup sync (after)

// Parallel with concurrency control — ~1 second
const limit = pLimit(10);
await Promise.all(pushOps.map(fn => limit(fn)));

Documentation (before)

• Single "Database Adapters" page showing only the raw FileSyncAdapter interface
• No setup guides, no env var docs, no factory docs, no diagnostics

Documentation (after)

• "File Sync" page with 10 sections: overview, quick start, configuration, 3 backend setup guides (Firestore, Supabase, Convex with full copyable code), factory pattern, diagnostics, agent-native parity, custom adapter guide

Naming: "File Sync" vs "Database Adapters"

The docs page was renamed from "Database Adapters" to "File Sync". The code still uses adapters/ in directory structure. Here's why:

• "File Sync" is the feature name — it's what users want to do ("sync files across instances"). The docs are organized around user intent.
• "Adapters" is the implementation pattern — it's how the code is structured. Firestore, Supabase, and Convex are adapters architecturally.
• "Database Adapters" was misleading — it implies the database is primary, which contradicts the core principle that files are the database. The database is a sync target, not a source of truth.

The word "database" is still used in prose where it's accurate (e.g., "syncs files to a database"), just not as the feature name. Users think "I want file sync", not "I want to configure a database adapter".

[netlify]

✅ Deploy Preview for agent-native-fw ready!

Name	Link
🔨 Latest commit	4daf542
🔍 Latest deploy log	https://app.netlify.com/projects/agent-native-fw/deploys/69bdb512ffebdd0008f7349a
😎 Deploy Preview	https://deploy-preview-57--agent-native-fw.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[builder-io-integration]

Builder has reviewed your changes and found 6 potential issues.

Review Details

PR #57 — File Sync DX Improvement: Code Review

This PR adds substantial security hardening to the file sync system: path traversal protection (branded SafePath type), a hardcoded sync denylist, identifier validation, Supabase compound filter for cross-tenant isolation, content-hash dedup replacing TTL-only echo suppression, parallel startup sync with p-limit, LCS backtrack fix, retry queue with dead-letter logging, SSE crash protection, and a createFileSync() factory for env-driven setup. The architectural approach is sound and the test coverage (168 tests, 37 new) is thorough.

Risk Assessment: 🔴 HIGH — Fixes a confirmed cross-tenant data leak, touches security-sensitive I/O paths, and changes API contracts.

Critical Issues Found

🔴 Supabase compound filter breaks Realtime subscriptions — The filter was changed from app=eq.${appId} (single clause + client-side owner_id check) to app=eq.${appId}&owner_id=eq.${ownerId}. Supabase Realtime postgres_changes subscriptions only support a single equality expression — the old code even had a comment documenting this: "Realtime only supports one filter." This either causes CHANNEL_ERROR / no events at all, or silently falls back to filtering only by app, reopening the cross-tenant leak this PR is trying to close. Found by 5/5 agents.

🔴 p-limit missing from package.json dependencies — Imported in file-sync.ts and added to pnpm-lock.yaml, but not added to packages/core/package.json. Works locally via pnpm hoisting; published consumers of @agent-native/core will get a module-not-found crash when file sync is enabled. Found by 3/5 agents.

🔴 createAppServer made async — callers not updated — The default template's createAppServer now returns Promise<Express>. The production bootstrap (node-build.ts) still calls createProductionServer(createAppServer()), passing a Promise where an Express instance is expected. app.use() is called on the Promise immediately, crashing both the production build and dev server. Found by 3/5 agents.

🔴 Scoped npm package names crash createFileSync before error handling — readPackageName() returns the raw package.json name. Scoped names like @company/my-app contain @ and /, which fail validateIdentifier()'s regex. This throw happens before the try/catch in createFileSync, so it propagates as an unhandled rejection, crashing the server on startup instead of returning { status: "error" }. Found by 2/5 agents.

🔴 stop() can lose in-flight file pushes — stop() flushes retryQueue but never awaits pushInFlight promises. A push that started via adapter.set() but hasn't completed yet is not in retryQueue. After stop() disposes the adapter, the in-flight .set() rejects, the .catch() tries to enqueue a retry — but shutdown has already completed. The last local edit before SIGTERM can be silently dropped.

🟡 emitSyncEvent drops rich conflict fields (regression) — Previously this.syncEvents.emit("sync", event) forwarded the full SyncEvent. Now emitSyncEvent strips it to { source, type, path }, losing localSnippet/remoteSnippet (needed for LLM-assisted resolution) and strategy. The server template writes application-state/sync-conflict.json from this event — the agent loses the context it needs. Also, the path conditional (event.type === "conflict-saved" ? event.path : event.path) is dead code (both branches identical).

🟡 createFileSync reports "ready" when init fails — initFileSync() now catches its own startup errors and doesn't rethrow. createFileSync() only detects failures when initFileSync() throws, so a transient backend outage during startup returns { status: "ready" } with no watchers or listeners installed. Found by 2/5 agents.

🟡 fs.rm fire-and-forget on remote "removed" events — Empty callback () => {} silently swallows I/O errors. State maps (lastSyncedHash, mergeBaseCache, retryQueue) are cleared synchronously before the deletion completes. On failure, the engine believes the file is gone while it still exists, preventing future conflict detection.

🟡 useFileSyncStatus opens a duplicate SSE connection and misses batched events — Creates a new EventSource("/api/events") independent of the existing one from useFileWatcher, consuming an extra browser connection slot. Also: the SSE handler sends startup sync events in batches ({ type: "batch", events: [...] }), but the hook only checks data.source === "sync" — batch messages are silently ignored, leaving conflict counts stale after startup.

Found by running 5 parallel code-review agents with randomized file ordering to combat position bias.

---

View in Builder.io

_{Code review by Builder.io}

[builder-io-integration]

Builder has reviewed your changes and found 6 potential issues.

Review Details

PR #57 — Update Review (Convex Adapter + Docs)

This update adds a Convex backend adapter, wires it into the createFileSync factory, and adds a new /docs/file-sync documentation page. The Convex adapter uses a content-hash diffing approach over onUpdate query subscriptions. Risk level remains 🔴 HIGH — new adapter code handles file system writes and the docs page is the primary user-facing guide for configuring sync.

The previously reported p-limit missing dependency was fixed ✅. The remaining 5 open issues (Supabase compound filter, async createAppServer, scoped package names, stop() loses pushInFlight, emitSyncEvent drops fields) are still unresolved.

New Findings

🔴 /docs/file-sync route missing from routeTree.gen.ts — TanStack Router requires every route to be registered in routeTree.gen.ts. The file routes/docs/file-sync.tsx was added but routeTree.gen.ts was never regenerated (tsr generate). The file currently has no DocsFileSyncRoute entry. Every user clicking the "File Sync" link in the docs sidebar hits a 404.

🔴 Convex get()/delete() pass only {id} but documented schema requires {id, app, ownerId} — The docs in file-sync.tsx show files:get and files:remove as Convex functions with required app and ownerId args. The adapter only passes { id }. Any user following the documented setup gets Convex argument-validation errors on file deletions and orphan cleanup (the delete() path runs during every startup sync). Found by 2/5 agents.

🔴 useFileSyncStatus silently drops all startup sync events — SSE handler batches events during the startup burst as { type: "batch", events: [...] }. The hook only checks data.source === "sync" at the top level; batch wrappers don't have source, so all events during the most critical window are discarded. Found by 2/5 agents.

🟡 Convex processingChain fires onChange after unsubscribe() — The serialized promise chain is not guarded by an isUnsubscribed flag. Queued updates continue to call onChange (writing files to disk) after FileSync.stop() has disposed the adapter. Found by 3/5 agents.

🟡 SSEHandlerOptions.extraEmitters typed as EventEmitter but receives TypedEventEmitter — TypedEventEmitter doesn't extend Node's EventEmitter (it only implements on/off/emit/removeAllListeners). Assigning syncResult.sseEmitter to extraEmitters in the template server is a TypeScript compile error.

🟡 Status endpoint hardcodes connected: true — FileSync.hasError is private with no public getter, so the /api/file-sync/status endpoint always returns connected: true when sync is enabled, even after adapter errors. The correct value is already written to .sync-status.json by writeSyncStatus() but never exposed over HTTP.

🟡 sync-conflict.json never deleted after conflict resolution — The template server writes application-state/sync-conflict.json on conflict-needs-llm but never removes it on conflict-resolved. In the agent-native architecture, file existence = state is active, so resolved conflicts remain visible to the agent indefinitely.

🟡 useFileSyncStatus sets connected: data.enabled instead of data.connected — Initial fetch maps the wrong field; when sync is enabled but disconnected, the hook always shows connected: true. Found by 3/5 agents.

Found by running 5 parallel code-review agents with randomized file ordering.

---

View in Builder.io

_{Code review by Builder.io}

[builder-io-integration]

🔴 routeTree.gen.ts not regenerated — /docs/file-sync route is a 404

The file-sync.tsx route file was added but routeTree.gen.ts was never regenerated (tsr generate). The file currently has no DocsFileSyncRoute entry, so TanStack Router won't serve this page. Run pnpm tsr generate and commit the updated routeTree.gen.ts.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 Convex get()/delete() pass only {id} but documented schema requires {id, app, ownerId}

The docs show files:get and files:remove with required app and ownerId args, but the adapter only calls client.query("files:get", { id }) and client.mutation("files:remove", { id }). Users following the documented Convex schema will get argument-validation errors on every file delete and orphan-cleanup call. Either simplify the documented schema to accept only id, or store appId/ownerId on the adapter and pass them through.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 Hook drops all sync events during startup burst — batch SSE envelope not unwrapped

The SSE handler sends startup sync events as { type: "batch", events: [...] }. The handler checks data.source === "sync" but batch wrappers have no source field, so every batched event is silently discarded. Unwrap batch messages: const evts = data.type === "batch" ? data.events : [data]; for (const e of evts) { if (e.source === "sync") { ... } }.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 processingChain fires onChange/onError after unsubscribe — can write files post-shutdown

The returned unsubscribe() detaches the Convex listener but does not cancel the queued promise chain. Pending chain entries will still call onChange (triggering disk writes) after FileSync.stop() disposes the adapter. Add let stopped = false; and flip it in unsubscribe(), then short-circuit each .then() with if (stopped) return;.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 SSEHandlerOptions.extraEmitters typed as EventEmitter but receives TypedEventEmitter — TypeScript compile error

extraEmitters uses EventEmitter from Node's "events" module, but createFileSync() returns a TypedEventEmitter<FileSyncEvents> which doesn't extend it. The template server assignment is a TypeScript compile error. Fix: replace EventEmitter with a minimal interface { on(e: string, l: (...args: unknown[]) => void): this; off(...): this }.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 Status endpoint hardcodes connected: true — FileSync.hasError is private and never surfaced

FileSync.hasError is private with no public getter, so the /api/file-sync/status endpoint always returns connected: true when sync is ready. After adapter errors, clients will show a false healthy state. Add a public getter get isConnected() { return !this.hasError; } to FileSync and use it in the status endpoint.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

Builder has reviewed your changes and found 6 potential issues.

Review Details

PR #57 — Update Review (Fixes Applied + New Issues)

This update fixed createAppServer/node-build.ts async handling (both the production bootstrap and Vite plugin now properly await the Promise), and the scoped npm package name crash (@scope/name is now stripped before validateIdentifier). Both of those previously reported issues are resolved ✅.

Risk: 🔴 HIGH — security-sensitive filesystem writes, data mutation paths, graceful shutdown data loss.

Still-open issues from prior reviews: Supabase compound filter, stop() not awaiting pushInFlight, emitSyncEvent drops fields, routeTree.gen.ts not regenerated, Convex get()/delete() args mismatch, useFileSyncStatus drops batched SSE, Convex processingChain after unsubscribe, SSEHandlerOptions TypeScript type mismatch, status endpoint hardcodes connected: true.

New Findings

🔴 stop() calls abort() before flushRetryQueue() — shutdown flush is always a no-op — this.abortController.abort() fires on line ~190, before the "final flush attempt" call to flushRetryQueue(). But flushRetryQueue immediately breaks when signal.aborted is true. Every pending retry write is guaranteed to be logged as lost on shutdown, even when the adapter is healthy. Found by 3/5 agents.

🔴 sync-conflict.json never deleted after conflict resolves — stale agent state — Template server writes application-state/sync-conflict.json on conflict-needs-llm but has no handler for conflict-resolved. Per agent-native architecture, file existence = active state. The agent will permanently see the conflict as unresolved. Found by 5/5 agents.

🔴 Manual conflict resolution silently ignored — .conflict file deletions never detected — When a user deletes a .conflict sidecar file to manually resolve a conflict, startFileWatcher ignores this because .conflict is in the sync denylist. conflictPaths is never cleared, conflict-resolved is never emitted, and the UI permanently shows an unresolved conflict. Found by 2/5 agents.

🔴 assertSafePath crashes for projects in symlinked directories (macOS /tmp) — resolvedRoot = path.resolve(root) doesn't resolve symlinks, but realParent = fs.realpathSync(parentDir) does. On macOS, /tmp → /private/tmp. The comparison always fails, throwing "Symlink escape blocked" for any legitimate file write. The test files themselves use fs.realpathSync() to work around this bug, confirming it's real.

🔴 Startup sync populates expectedWrites before the watcher is running — first edit post-boot silently dropped — initStartupSync() calls writeSyncedFile() which adds all pulled files to expectedWrites. But the file watcher starts afterwards with ignoreInitial: true, so these entries are never consumed. The first real local edit to every file pulled at startup hits wasSyncPulled(), returns true, and is silently skipped — never pushed to the backend.

🟡 useFileSyncStatus sets connected: data.enabled instead of data.connected — When sync is enabled but the adapter has errored (hasError=true), the hook always shows connected: true. Found by 5/5 agents (was dropped by cap in previous round).

🟡 Convex subscribe() emits all existing files as "added" on first call — previousHashes starts empty, so the first onUpdate callback treats every file as new. Since initStartupSync() just ran, FileSync redundantly reads and hashes the entire file tree again. Found by 2/5 agents.

Found by running 5 parallel code-review agents with randomized file ordering.

---

View in Builder.io

_{Code review by Builder.io}

[builder-io-integration]

🔴 stop() aborts signal before flushRetryQueue — shutdown flush is always a guaranteed no-op

this.abortController.abort() is called on line ~190, before flushRetryQueue() on line ~195. But flushRetryQueue immediately breaks when signal.aborted is true, so every entry in the retry queue is written to dead letter even when the adapter is healthy. Move abort() to after the final flush.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 sync-conflict.json never deleted on conflict-resolved — permanent stale agent state

The server writes application-state/sync-conflict.json on conflict-needs-llm but never deletes it when conflict-resolved fires. Per agent-native architecture, file existence = active state, so the agent will permanently see the conflict as unresolved. Add an else if (event.type === "conflict-resolved") handler that calls fs.rmSync("application-state/sync-conflict.json", { force: true }).

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 Manual .conflict file deletion never detected — conflictPaths stuck forever

.conflict files are in the sync denylist, so handleDelete in startFileWatcher ignores sidecar deletions. When a user manually resolves a conflict by deleting the .conflict file, conflictPaths is never cleared and conflict-resolved is never emitted. The UI permanently shows an unresolved conflict. Watch for .conflict deletions specifically before the shouldSyncFile guard.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 assertSafePath crashes on symlinked project roots (macOS /tmp → /private/tmp)

resolvedRoot = path.resolve(root) doesn't follow symlinks, but realParent = fs.realpathSync(parentDir) does. On macOS /tmp is a symlink to /private/tmp, so the comparison always fails and throws "Symlink escape blocked" for every legitimate write. The test files themselves call fs.realpathSync() as a workaround, confirming the bug. Fix: use fs.realpathSync(resolvedRoot) as the comparison baseline when the path exists.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 initStartupSync populates expectedWrites before watcher starts — first local edit silently dropped

initStartupSync() calls writeSyncedFile() which adds pulled files to expectedWrites, but the watcher starts afterwards with ignoreInitial: true. Those entries are never consumed, so the first real local edit to any file pulled at startup hits wasSyncPulled() → true and is silently skipped and never pushed to the backend.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 Initial fetch sets connected: data.enabled instead of data.connected

Sets connected: data.enabled on mount. When sync is enabled but the adapter errored (hasError=true), the server returns { enabled: true, connected: false } but the hook always shows connected: true. Fix: connected: data.connected. Found by 5/5 review agents (previously dropped by review cap).

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

Builder has reviewed your changes and found 4 potential issues.

Review Details

Code Review Summary (Review Cycle 5)

PR #57 is a major security, resilience, and DX overhaul of the file sync system in agent-native. This cycle found the PR has significantly matured: three previously-reported bugs were fixed (Supabase compound filter now uses client-side owner_id check; stop() now properly awaits pushInFlight; emitSyncEvent now spreads all fields with { source: "sync", ...event }). Risk: 🔴 HIGH — security-sensitive path validation, data mutation logic, new adapters.

New Issues Found (4 new, from 5 parallel agents)

🔴 HIGH — assertSafePath false-negative on non-existent parent (4/5 agents)
The symlink escape check is guarded by fs.existsSync(parentDir). If the immediate parent doesn't yet exist (new nested path), the check is skipped. An attacker-controlled data/ symlink pointing to /etc passes the guard when the path is data/newdir/file.json — newdir doesn't exist, so existsSync returns false, symlink check is skipped, and mkdirSync(..., { recursive: true }) writes outside contentRoot.

🟡 MEDIUM — createFileSync returns ready despite init failure (2/5 agents)
initFileSync() now swallows errors internally (to allow retry), so createFileSync()'s try/catch never sees the exception. If the adapter query fails at boot, createFileSync still returns { status: "ready" } — wiring up SSE and status endpoints for a sync engine that never actually initialized.

🟡 MEDIUM — Supabase dispose() is a no-op (1/5 agents)
The comment in dispose() names removeAllChannels as the correct teardown, but the call is absent — the method body is empty. Supabase Realtime WebSocket channels are never closed on stop(), leaking connections and potentially preventing clean process exit.

🟡 MEDIUM — handleDelete races with pushInFlight, can resurrect deleted files (1/5 agents)
handleDelete() calls adapter.delete() immediately without waiting for pushInFlight. If a file is edited and then quickly deleted, the in-flight set() can resolve after the delete and recreate the remote row even though the file no longer exists locally.

Found by running 5 parallel code-review agents with randomized file ordering.

---

View in Builder.io

_{Code review by Builder.io}

[builder-io-integration]

🔴 assertSafePath skips symlink check when parent dir doesn't exist yet

The symlink escape check is gated on fs.existsSync(parentDir). If an attacker sends data/newdir/file.json where data/ is a symlink outside the project, newdir doesn't exist yet so existsSync returns false — the entire symlink check is skipped. Subsequent mkdirSync(..., { recursive: true }) then follows the symlink and writes outside contentRoot. Walk up to the nearest existing ancestor to check it instead.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 createFileSync returns ready even when initFileSync silently failed

initFileSync() now catches all startup errors internally (to support retry) and returns normally. This means createFileSync()'s surrounding try/catch never sees the exception and still returns { status: "ready" }. A DB outage at boot leaves the server wiring up SSE and status endpoints for a sync engine that never started.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 dispose() body is empty — Supabase channels never closed

The comment names removeAllChannels as the correct teardown but the actual call is absent; the method body is empty. Supabase Realtime WebSocket channels are never closed on stop(), leaking connections and potentially preventing clean Node.js process exit.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 handleDelete races with pushInFlight, can resurrect deleted files

handleDelete() sends adapter.delete() without waiting for pushInFlight. If a file is edited and then quickly deleted, an in-flight set() can resolve after the delete, recreating the remote row for a file that no longer exists locally.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

Builder has reviewed your changes and found 1 potential issue.

Review Details

Code Review Summary (Review Cycle 6)

PR #57 continues incremental improvements: the latest update fixed two Convex schema documentation bugs (renamed reserved by_id index to by_sync_id; corrected schema import from convex/server instead of convex/values). Risk: 🔴 HIGH — security-sensitive path validation, data mutation logic. Reviewed via 3/5 parallel agents (2 aborted mid-review due to concurrent PR update).

New Issues Found

🟡 MEDIUM — initFileSync() leaks purge timer handles on retry (1/3 agents)
startPurgeTimer() unconditionally overwrites this.purgeTimer = setInterval(...) without clearing the previous handle. Because initFileSync() now allows retries after startup failure (the flag stays false on error), each retry call starts a new purge interval. The old setInterval handle is lost — its reference is overwritten before clearInterval() is ever called. On repeated retries (e.g., during adapter outages), the number of leaked timers grows unboundedly and only the last one is cleared on stop().

Found by running 3 parallel code-review agents with randomized file ordering (2 agents aborted due to concurrent PR update).

---

View in Builder.io

_{Code review by Builder.io}

[builder-io-integration]

🟡 startPurgeTimer() overwrites purgeTimer handle without clearing previous, leaking intervals on retry

Each failed initFileSync() retry calls startPurgeTimer() which sets this.purgeTimer = setInterval(...) without first checking or clearing an existing timer. On repeated retries, old interval handles are leaked — only the most recent is tracked and cleared on stop().

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

Builder has reviewed your changes and found 1 potential issue.

Review Details

Code Review Summary (Review Cycle 7)

The latest PR update adds Firestore doc ID encoding (encodeDocId/decodeDocId) to handle the new : separator format — Firestore rejects IDs containing /, so / is encoded as %2F. It also fixes the firebase-admin ESM dynamic import unwrapping. Risk: 🔴 HIGH — data mutation logic, security-sensitive sync engine. Reviewed via 4/5 parallel agents (1 aborted due to concurrent PR update).

New Issue Found

🔴 HIGH — decodeDocId corrupts IDs that legitimately contain %2F (2/4 agents)
encodeDocId replaces / with %2F, and decodeDocId replaces ALL %2F back to /. But on Linux/macOS, file paths can legitimately contain %2F as literal characters (e.g. data/file%2Fname.json). Round-trip: appId:data/file%2Fname.json → encoded: appId:data%2Ffile%2Fname.json → decoded: appId:data/file/name.json. The decoded ID no longer matches getDocId(appId, filePath), so on next startup initStartupSync treats the document as an orphan and deletes it. Any file with %2F in its name triggers an infinite orphan-cleanup loop (delete → re-upload → delete → …).

Found by running 4 parallel code-review agents with randomized file ordering.

---

View in Builder.io

_{Code review by Builder.io}

[builder-io-integration]

🔴 decodeDocId is not the inverse of encodeDocId — corrupts paths containing literal %2F

encodeDocId encodes / as %2F but doesn't first escape existing %. On decode, decodeDocId converts ALL %2F back to /, corrupting paths that legitimately contain %2F (valid on Linux/macOS). The mismatched round-trip ID triggers infinite orphan-cleanup cycles for any such file. Fix: encodeDocId should first replace % with %25 before encoding / as %2F, then decode in reverse.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

Builder has reviewed your changes and found 1 potential issue.

Review Details

Code Review Summary (Review Cycle 8)

This update refactors Supabase key detection in createFileSync() to support both the new (SUPABASE_PUBLISHABLE_KEY/SUPABASE_SECRET_KEY) and legacy (SUPABASE_ANON_KEY/SUPABASE_SERVICE_ROLE_KEY) key formats. Risk: 🔴 HIGH — credential selection logic directly impacts RLS enforcement. Reviewed via 5 parallel agents.

New Issue Found

🟡 MEDIUM — Factory silently prefers secret/service-role key when both keys are configured (2/5 agents)
The selection logic const key = secretKey || publishableKey always picks the secret key if SUPABASE_SECRET_KEY or SUPABASE_SERVICE_ROLE_KEY is present in the environment — even if SUPABASE_PUBLISHABLE_KEY is also set and intended to be used. In typical server environments both variables are present (different services use different keys). The warning fires but the behavior is still a footgun: the file-sync subsystem silently escalates to full-table RLS-bypassing access without explicit opt-in. The factory should default to the publishable/anon key and require an explicit FILE_SYNC_SUPABASE_KEY_TYPE=secret signal (or similar) to opt into the secret key.

Found by running 5 parallel code-review agents with randomized file ordering.

---

View in Builder.io

_{Code review by Builder.io}

[builder-io-integration]

Builder has reviewed your changes and found 6 potential issues.

Review Details

Code Review Summary — PR #57 (File Sync DX)

This PR comprehensively hardens and improves the file sync engine: security fixes (path traversal, denylist, input validation, tenant isolation), resilience improvements (retry queue, graceful shutdown, init flag fix, AbortController), performance gains (parallel startup sync, content hashing, LCS fix), a new Convex adapter, a createFileSync() factory, and a useFileSyncStatus() hook — alongside full documentation. Risk: High (security-sensitive code, data deletion, cross-tenant isolation).

The overall architecture is sound. The factory pattern, discriminated union result, typed EventEmitter, and per-file push serialization are all clean improvements. However, 3 parallel code-review agents surfaced several confirmed bugs spanning correctness, resilience, and UI state.

Key Findings

🔴 HIGH

• Convex delete() missing required app/ownerId args — "files:remove" mutation requires all 3 args per docs; only { id } is passed, causing runtime Convex validation errors for all deletes.

🟡 MEDIUM (confirmed by 2+ agents or clearly valid)

• useFileSyncStatus sets connected: data.enabled — ignores data.connected; UI can never show disconnected state.
• /api/file-sync/status hardcodes connected: true — ignores hasError set after init; inconsistent with .sync-status.json.
• sync-burst-start without guaranteed sync-burst-end — if adapter.query() throws, SSE clients stay in batchMode permanently.
• Convex processingChain poisoned when onError throws — chain permanently rejects; all future remote changes silently dropped.
• Legacy __ orphans deleted but not migrated — remote DB records deleted but never re-inserted under new : format; ignoreInitial: true prevents re-push.
• Merge base cache is FIFO, not LRU — Map.set() on existing key doesn't update insertion order; actively-edited files evicted first.
• Unhandled rejection in production entry — createAppServer().then(...) has no .catch(); Node 15+ terminates process silently.

Found by running 3 parallel code-review agents with randomized file ordering.

---

View in Builder.io

_{Code review by Builder.io}

[builder-io-integration]

🔴 Convex delete() missing required app/ownerId args — all deletes fail at runtime

Calls "files:remove" with only { id } but the documented Convex function requires { id, app, ownerId } as mandatory v.string() args. Convex validates mutation args at runtime, so every delete (orphan cleanup and local file deletion) throws a validation error and the remote record is never removed.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 useFileSyncStatus sets connected: data.enabled — should be data.connected

Sets connected: data.enabled instead of data.connected, so the hook can never surface a disconnected/error state — connected will always equal enabled regardless of adapter errors detected after init.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 /api/file-sync/status hardcodes connected: true after init

Always returns connected: true when sync is ready, ignoring FileSync.hasError set on post-init adapter failures. The .sync-status.json file correctly reflects the error state but this endpoint does not, so the hook above can never show a real disconnect.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 sync-burst-start emitted without guaranteed sync-burst-end on failure

If adapter.query() throws after sync-burst-start is emitted, the function exits before emitting sync-burst-end. Every connected SSE client stays in batchMode = true permanently, degrading all real-time file-change events to batched 150 ms delivery for the server lifetime.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 Convex processingChain permanently poisoned if onError callback throws

If the caller's onError throws, the .then() rejects with no .catch() recovery path — all subsequent remote updates are silently dropped. A .catch(() => {}) guard after each .then() slot prevents the chain from poisoning.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 Legacy __-separator orphans deleted but never re-inserted under new : format

Orphan docs are deleted from the remote DB but their content is never re-pushed under the new canonical ID. Since the file watcher uses ignoreInitial: true, pre-existing local files won't trigger a push, leaving the remote DB empty for all previously-synced files after an upgrade.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

Builder has reviewed your changes and found 6 potential issues.

Review Details

Code Review Summary — PR #57: Improve File Sync DX

This PR is a major overhaul of the file sync subsystem: a createFileSync() factory for zero-config setup, a Convex adapter (third backend alongside Firestore/Supabase), extensive security hardening (path traversal, denylist, input validation, client-side owner filter), resilience improvements (AbortController shutdown, retry queue, per-file serialization), and performance work (p-limit parallel startup, SHA-256 content hashing, LCS fix). Risk level: 🔴 HIGH — security-sensitive code, data deletion logic, API contract changes, and multi-tenant isolation.

The approach is architecturally sound and the breadth of hardening is impressive. However, 5 parallel code-review agent passes (with randomized file ordering) uncovered several confirmed bugs, some of which cause silent data loss or security issues.

Key Findings

🔴 HIGH — Confirmed by 3/5 agents:

• Legacy doc ID migration deletes all existing remote data — upgrading users lose all synced files when startup sees every __-format ID as an orphan and deletes it without re-uploading under the new : format.

🔴 HIGH — Confirmed by 2/5 agents:

• SIGTERM race: async shutdown bypassed by createProductionServer's synchronous process.exit(0) — retry queue flush, in-flight pushes, and adapter disposal are all skipped on container shutdown.
• createFileSync() returns "ready" even when initFileSync() failed — internal error swallowing means factory consumers see a false-positive ready state.
• Convex files:remove/files:get args mismatch with docs — adapter passes only { id } but documented sample mutations require app+ownerId; every delete fails with a Convex validation error.

🔴 HIGH — Single-agent, confirmed critical:

• Path traversal can bypass the sync denylist via unnormalized paths (foo/../.env)
• docId omits ownerId, allowing cross-tenant overwrite of private files
• Supabase DELETE events silently dropped (missing owner_id in payload.old without REPLICA IDENTITY FULL)
• Echo suppression stale entries from startup block first user edits

Found by running 5 parallel code-review agents with randomized file ordering.

---

View in Builder.io

_{Code review by Builder.io}

[builder-io-integration]

🔴 Legacy __ doc ID migration deletes all existing remote data on upgrade

Every document stored with the old __ separator fails the canonical ID match and lands in orphanedDocIds, which are then deleted. Because they're skipped via continue before being added to docsByPath, the startup sync loop never re-uploads them, and the file watcher uses ignoreInitial: true so unchanged local files are never re-pushed. Other connected clients receive removed events and delete their local files. Impact: upgrading users silently lose all previously synced remote data. Fix: migrate before deleting — await adapter.set(newId, row.data) then await adapter.delete(row.id) — and populate docsByPath with the migrated record.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 SIGTERM race: createProductionServer's synchronous process.exit(0) kills the process before async sync shutdown completes

The template registers an async SIGTERM handler that awaits syncResult.shutdown(), but createProductionServer() registers a subsequent synchronous SIGTERM handler that calls process.exit(0) immediately. Node does not await async listeners, so the production handler fires while the template's handler is suspended at await, killing the process before the retry-queue flush, in-flight push drain, or adapter disposal finish. Fix: remove the template-level SIGTERM handler and have createProductionServer accept an async onShutdown callback, or ensure only one shutdown path owns process.exit(0) after async cleanup.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 Factory returns status:"ready" even when initFileSync failed internally

initFileSync() catches all initialization errors internally (file-sync.ts:134-164), sets hasError = true, and returns normally without rethrowing. The try/catch in createFileSync() is therefore dead code for startup failures — adapter errors, missing tables, and transient network failures all fall through to { status: "ready" }. Impact: callers wire SSE and routes as if sync is functional while no remote listener was ever started. Fix: check sync.hasError after initFileSync() returns, or rethrow from initFileSync() so createFileSync() can return { status: "error" } as documented.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 Echo suppression stale entries from startup sync cause first user edit to be silently dropped

writeSyncedFile() during initStartupSync adds paths to expectedWrites for echo suppression, but the file watcher starts afterward with ignoreInitial: true and never fires for those writes — leaving stale entries in expectedWrites. When the user next modifies one of those files, handleChange calls wasSyncPulled(), finds the stale entry, consumes it, and returns without syncing. Impact: the first manual edit to every file pulled during startup is silently lost. Fix: call this.expectedWrites.clear() at the end of initStartupSync, before the watcher starts.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 Path traversal bypasses sync denylist via unnormalized remote paths

The denylist (isDenylisted) and shouldSyncFile checks run against the unnormalized remote path before assertSafePath canonicalizes it. A path like foo/../.env passes the **/.env* minimatch check because minimatch evaluates the literal string, but assertSafePath then resolves it to the real .env inside the project root and allows the write. Impact: a remote database write with a traversal path can overwrite .env or other protected files despite the denylist. Fix: normalize the path first with assertSafePath, then run shouldSyncFile/isDenylisted against the canonical relative path.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 Supabase client-side owner filter silently drops all DELETE events

On Supabase DELETE events, payload.old only contains the primary key unless REPLICA IDENTITY FULL is set (which the provided schema does not do). row.owner_id is therefore undefined, and the check if (row.owner_id !== ownerId) return evaluates undefined !== "shared" → true, so every remote deletion is silently ignored. Impact: files deleted by remote peers are never removed locally, permanently breaking sync for deletes. Fix: either add ALTER TABLE files REPLICA IDENTITY FULL to the setup schema, or skip the owner filter for DELETE events and instead verify ownership via the doc ID prefix.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[steve8708]

@builderio-bot look at latest PR feedback and fix anything you agree with. Be skeptical. Reply to every comment (directly on the comment thread of each comment) if you fixed it or not and why

[steve8708]

oops this isn't a Builder-created PR, @gopinav can you peek at latest review feedback and have agent fix whatever it agrees with

[builder-io-integration]

Builder has reviewed your changes and found 6 potential issues.

Review Details

Code Review Summary — PR #57 (Round 3): Improve File Sync DX

This PR adds a createFileSync() factory, Convex adapter, extensive security hardening (path traversal, denylist, input validation), resilience improvements (AbortController shutdown, retry queue, per-file serialization), and performance optimizations. Risk level: 🔴 HIGH — security-sensitive code, multi-tenant isolation, data deletion logic, and API contract changes.

The six issues from the previous review round remain open. This round of 5 parallel agents (with randomized file ordering) uncovered additional new bugs not previously posted.

New Key Findings

🔴 HIGH — 2 agents:

• docId omits ownerId — private-sync documents from different users sharing the same file path get the same remote ID, causing cross-user overwrites

🔴 HIGH — 1 agent (confirmed logic bug):

• pushInFlight serialization broken — multiple rapid changes all await the same prior promise and wake simultaneously, defeating per-file serialization
• Convex diffing ignores lastUpdated — metadata-only updates (conflict resolution, echo suppression touches) are invisible to the Convex subscriber

🟡 MEDIUM — 2 agents:

• Convex get/remove arg mismatch — adapter sends only { id } but documented sample functions require app+ownerId; deletes fail for anyone following the docs
• dispose() now required on FileSyncAdapter — breaks existing custom adapters at compile time and crashes at shutdown without a guard
• Startup pushOps not tracked in pushInFlight — stop() can dispose the adapter while startup writes are still running

🟡 MEDIUM — 1 agent:

• Promise.all over startup pushOps fails entire init on one file error
• SSE safePush swallows stream errors without cleanup, leaking watcher listeners
• Convex processingChain continues executing after unsubscribe, causing concurrent mutation on reconnect
• stop() race when background retry flush is already running

Found by running 5 parallel code-review agents with randomized file ordering.

---

View in Builder.io

_{Code review by Builder.io}

[builder-io-integration]

🔴 pushInFlight serialization fails: all concurrent awaiters wake simultaneously

When multiple rapid file-change events fire for the same path, they all fetch the same prior promise and await it together. When prior resolves, every waiting execution resumes at the same time and races to read the file, compute hashes, and call adapter.set() concurrently — defeating the intended serialization. Fix: chain promises instead of just awaiting them: const next = (prior || Promise.resolve()).then(async () => { /* push logic */ }); this.pushInFlight.set(relPath, next);

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 docId omits ownerId — private sync channels collide across different users

getDocId() builds keys as ${appId}:${filePath} with no ownerId. Two users syncing the same relative path under initPrivateSync produce identical remote IDs. All adapters use this ID as the upsert key, so user B's write overwrites user A's row and changes its owner_id, making user A's document invisible. Fix: include ownerId in the canonical ID: ${appId}:${ownerId}:${filePath}.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🔴 Convex snapshot diffing ignores lastUpdated — metadata-only updates are silently dropped

The subscriber detects modifications by comparing only content hashes (prevHash !== hash). If a file's content doesn't change but its lastUpdated timestamp does (e.g., conflict resolution or echo-suppression touch), the hash stays identical and no "modified" event is emitted. Other clients never learn about the update, leaving them in stale state. Fix: also compare lastUpdated: if (prevHash !== hash || prevRecord?.lastUpdated !== record.lastUpdated).

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 Convex adapter sends only { id } but documented sample functions require app+ownerId — deletes always fail

adapter.get() calls files:get with { id } and adapter.delete() calls files:remove with { id }, but the Convex setup guide in the docs defines those mutations with required app: v.string() and ownerId: v.string() args. Convex validates all required args at the edge, so every delete() call fails with an argument-validation error for anyone following the documented setup. Fix: either pass appId/ownerId from the adapter, or update the docs to make those args optional.

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 dispose() is now required on FileSyncAdapter — breaks all existing custom adapters

dispose(): Promise<void> was added as a mandatory interface member, and FileSync.stop() calls it unconditionally. Any existing custom adapter built against the prior 5-method contract fails TypeScript compilation after upgrade, and JavaScript adapters throw TypeError: adapter.dispose is not a function at shutdown. The docs custom-adapter section still shows the old 5-method signature. Fix: make it optional (dispose?(): Promise<void>) and guard the call with await this.options.adapter.dispose?.().

---

_{How did I do? React with 👍 or 👎 to help me improve.}

[builder-io-integration]

🟡 Startup pushOps run outside pushInFlight — stop() disposes adapter while writes are in-flight

The startup sync push batch (Promise.all(pushOps.map(fn => limit(fn)))) is never registered in pushInFlight. stop() only awaits pushInFlight before calling abortController.abort() and adapter.dispose(). If stop() fires mid-startup (e.g., container restart), those orphaned push promises continue writing to an already-disposed adapter, causing errors and corrupted sync state. Fix: store the startup batch promise and await it in stop() before the abort signal.

---

_{How did I do? React with 👍 or 👎 to help me improve.}