Refactor Code Link sync runtime for traceable, nonblocking prompt handling#633
Refactor Code Link sync runtime for traceable, nonblocking prompt handling#633huntercaron wants to merge 19 commits intomainfrom
Conversation
…ectResult Made-with: Cursor
Clear stale plugin prompt state on reconnect boundaries and ignore stale delete replies so reconnects cannot resurrect or apply old confirmations. Made-with: Cursor
Replace sync-mode/CliSyncMode/sync-complete with coarse SyncPhase (initial_sync|ready) on sync-phase messages. CLI uses internalPhase; emit phases at effect boundaries. Plugin: pluginMode + syncPhase; sync-phase helper. Update plan and tests. Made-with: Cursor
…pt sessions - Narrow kernel + PeerBaseView; disconnect UI in kernel; serial event queue - Session-scoped delete/conflict prompts; shared PromptSession protocol - Plugin UiState discriminated union; PluginBase single snapshot map; remove sync-phase helpers - Ignore *.tsbuildinfo; update refactor plan status Made-with: Cursor
…el→runtime, sync-base→sync-memory Every Effect variant now flows through `describeEffect` → `EffectResult` → `applyEffectResult`, with `applyKernelOp` as the single (async) mutation site. `describeEffect` takes a `ReadonlyRuntime`, so purity is a compile-time check. Duplicate handshakes route through a new `RESEND_SYNC_PHASE` SyncEvent instead of bypassing the state machine. Drop the vestigial `pendingRemoteChanges` field from `SyncState`; `CONFLICTS_DETECTED` now carries `remoteTotal` directly. Rename `SyncKernel`→`Runtime` and `SyncBase`→`SyncMemory` (+`SyncMemoryHandle`) for clarity. Made-with: Cursor
huntercaron
changed the title
huntercaron
changed the title
There was a problem hiding this comment.
Pull request overview
Refactors Code Link’s sync pipeline to make prompt handling traceable and non-blocking by moving race-sensitive lifecycle state into SyncRuntime and file-truth/echo/tombstone state into SyncMemory, while keeping controller logic as a flat Effect -> applyEffect plan.
Changes:
- Introduces session-based prompt updates/clears (conflicts + deletes) so the serial queue never blocks on user interaction and prompts can refresh in place.
- Adds CLI
SyncRuntime/SyncMemoryplus a namedSchedulerto centralize echo/tombstone timing and other race-sensitive state. - Updates shared message/types surface (
SyncPhase,PromptSession) and bumps Vitest versions across packages/plugins (with updated Yarn cache artifacts).
Reviewed changes
Copilot reviewed 44 out of 137 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/global-search/package.json | Removes @vitest/ui and bumps vitest version. |
| plugins/code-versions/package.json | Removes @vitest/ui and bumps vitest version. |
| plugins/ashby/package.json | Bumps vitest version. |
| plugins/airtable/package.json | Bumps vitest version. |
| packages/plugin-tools/package.json | Bumps vitest version. |
| packages/code-link-shared/package.json | Bumps vitest version for shared package tests. |
| packages/code-link-cli/package.json | Bumps vitest version for CLI tests. |
| package.json | Bumps root vitest version. |
| plugins/code-link/src/plugin-base.ts | Adds plugin-side snapshot/echo map utilities used by the plugin API. |
| plugins/code-link/src/messages.ts | Updates plugin message handling for sync-phase + session-aware prompt updates/clears. |
| plugins/code-link/src/messages.test.ts | Adds focused tests for new session-aware message handling (conflicts/deletes). |
| plugins/code-link/src/app-state.ts | Adds unified UI state machine keyed by SyncPhase and prompt sessions. |
| plugins/code-link/src/app-state.test.ts | Adds reducer tests for session staleness, prompt refresh, and prompt invalidation. |
| plugins/code-link/src/api.ts | Refactors CodeFilesAPI to extend PluginBase for snapshot/echo behavior. |
| plugins/code-link/src/api.test.ts | Updates API tests to assert echo tracking via PluginBase rather than old tracker. |
| plugins/code-link/src/App.tsx | Switches UI orchestration to new reducer/state model and session-scoped prompt responses. |
| packages/code-link-shared/src/types.ts | Replaces Mode with SyncPhase and adds PromptSession; updates message unions. |
| packages/code-link-shared/src/sync-tracker.ts | Removes legacy plugin-side sync tracker abstraction. |
| packages/code-link-shared/src/index.ts | Updates exports to match new shared types (drops tracker exports). |
| packages/code-link-cli/src/utils/logging.ts | Removes disconnect-suppression timer helpers (moved into runtime/scheduler). |
| packages/code-link-cli/src/utils/hash-tracker.ts | Removes legacy CLI hash tracker. |
| packages/code-link-cli/src/sync-memory.ts | Adds SyncMemory for metadata truth + content echoes + delete tombstones. |
| packages/code-link-cli/src/sync-memory.test.ts | Adds unit tests for SyncMemory invariants and rollback semantics. |
| packages/code-link-cli/src/sync-events.ts | Adds CLI state machine vocabulary (SyncEvent, Effect, SyncState). |
| packages/code-link-cli/src/scheduler.ts | Adds named-timer scheduler and shared timing constants. |
| packages/code-link-cli/src/runtime.ts | Adds SyncRuntime lifecycle owner (workspace, prompts, renames, disconnect UI, phase). |
| packages/code-link-cli/src/runtime.test.ts | Adds tests for prompt session merging, convergence clearing, and staleness rejection. |
| packages/code-link-cli/src/helpers/watcher.ts | Switches rename/add/delete coalescing timers to the new Scheduler. |
| packages/code-link-cli/src/helpers/sync-validator.ts | Removes old pure validation helper (replaced by runtime/state-machine structure). |
| packages/code-link-cli/src/helpers/plugin-prompts.ts | Removes blocking prompt coordinator (replaced by session-based prompt events). |
| packages/code-link-cli/src/helpers/installer.ts | Minor formatting/cleanup plus pinned dependency key style change. |
| packages/code-link-cli/src/helpers/installer.test.ts | Updates fetch mock typing and formatting; aligns pinned dependency key style. |
| packages/code-link-cli/src/helpers/files.ts | Refactors write/delete helpers to use SyncMemory echo/tombstone APIs and return results. |
| packages/code-link-cli/src/helpers/files.test.ts | Minor formatting changes for conflict construction. |
| packages/code-link-cli/src/helpers/connection.ts | Import ordering/node: prefix consistency adjustments. |
| packages/code-link-cli/src/helpers/connection.test.ts | Formatting adjustments for readability. |
| packages/code-link-cli/src/helpers/certs.ts | Migrates imports to node: specifiers; minor formatting. |
| packages/code-link-cli/src/helpers/certs.test.ts | Updates mocks to node: modules; adds promisify.custom wiring for execFile mock. |
| packages/code-link-cli/src/controller.test.ts | Updates controller tests to new SyncState naming and prompt resolution effects. |
| packages/code-link-cli/src/controller.rename.test.ts | Rewrites rename/apply tests around applyEffect + SyncRuntime semantics. |
| packages/code-link-cli/src/controller.once.test.ts | Rewrites once-mode tests around applyEffect shutdown ordering + sync-phase emission. |
| packages/code-link-cli/src/controller.integration.test.ts | Adds cross-component integration tests for start/handshake/watcher/state-machine. |
| packages/code-link-cli/src/controller.apply.test.ts | Adds transaction-boundary tests for applyEffect send/write/delete/prompt behavior. |
| .gitignore | Adds *.tsbuildinfo and .cursor/plans/ ignores. |
| .yarn/cache/tinyrainbow-npm-3.1.0-35ba47f8ae-4c2c01dde1.zip | Updates Yarn cache artifact. |
| .yarn/cache/@vitest-spy-npm-4.1.5-4f8c9ce4ca-4db4bb3aea.zip | Adds/updates Yarn cache artifact for Vitest 4.1.5. |
| .yarn/cache/@rolldown-pluginutils-npm-1.0.0-rc.17-c8be250a71-d659ea756e.zip | Adds/updates Yarn cache artifact. |
| .yarn/cache/@oxc-project-types-npm-0.127.0-db76f58945-f154f47203.zip | Adds/updates Yarn cache artifact. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 137 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| state.internalPhase === "snapshot_processing" || | ||
| state.internalPhase === "handshaking" || | ||
| state.internalPhase === "conflict_resolution" |
There was a problem hiding this comment.
Just a Q: if an event with remote file changes comes in while snapshot_processing or some of the other states, the event is ignored
How does it see those changes after transitioning from the current state?
| state.internalPhase === "disconnected" | ||
| ? state |
There was a problem hiding this comment.
nit: is this disconnected check like necessary? How could it be receiving transition events while disconnected? Or am I misunderstanding its meaning
| // Only process changes in watching mode | ||
| if (state.mode !== "watching") { | ||
| effects.push(log("debug", `Ignoring watcher event in ${state.mode} mode: ${kind} ${relativePath}`)) | ||
| if (state.internalPhase !== "watching") { |
There was a problem hiding this comment.
silly naming nit but can't this just be state.phase everywhere?
| syncState: SyncState | ||
| // Apply: the only place that mutates or does I/O | ||
|
|
||
| type LogLevel = "info" | "debug" | "warn" | "success" | "status" |
There was a problem hiding this comment.
nit: LogLevel already exists in logger as an enum, maybe just make it a string literal type there so you don't have duplicate LogLevels definitions
| async function writeFiles( | ||
| files: Extract<Effect, { type: "WRITE_FILES" }>["files"], | ||
| ctx: ApplyCtx, | ||
| options: { silent?: boolean; skipEcho?: boolean } = {} |
There was a problem hiding this comment.
could be nice to add a comment what skipEcho means and why its useful, not sure I understand 🙏
There was a problem hiding this comment.
I remember adding that and already forget why so clearly a good point
| } | ||
|
|
||
| async function applyConflictChange( | ||
| change: ReturnType<SyncRuntime["updateActiveConflictLocal"]>, |
There was a problem hiding this comment.
nit:
| change: ReturnType<SyncRuntime["updateActiveConflictLocal"]>, | |
| change: ConflictPromptChange, |
nbd but the current type kinda couples it closely to that method when the type is already an exported interface
| } else { | ||
| logs.push({ | ||
| level: "success", | ||
| message: `Synced ${pluralize(effect.totalCount, "file")} (${effect.updatedCount} updated, ${effect.unchangedCount} unchanged)`, |
There was a problem hiding this comment.
maybe too nitpicky but this is like 20+ lines of code for pushing a single success message to logs. A single logs.push({ level "succes", message: syncSuccessMessage(runtime, effect)}) could make the function a bit less distracting.
You can also emitLog directly or just call success() directly even instead of keeping an array
| await deleteFiles( | ||
| conflicts.filter(conflict => conflict.remoteContent === null).map(conflict => conflict.fileName), | ||
| ctx | ||
| ) |
There was a problem hiding this comment.
const filesToDelete = []
const fileToWrite = []
for (const conflict of conflicts) {
if (conflict.remoteContent === null) {
filesToDelete.push(conflict.fileName)
continue
}
fileToWrite.push({ name: conflict.fileName, content: conflict.remoteContent!, modifiedAt: conflict.remoteModifiedAt })
}
await Promise.all([writeFiles(filesToWrite), deleteFiles(filesToDelete)])
imo something like this would be a big readability win
| } | ||
| } | ||
|
|
||
| async function executeEffect(effect: Effect, ctx: ApplyCtx): Promise<SyncEvent[]> { |
There was a problem hiding this comment.
nit: all these callers can just call applyEffect directly right?
There was a problem hiding this comment.
oh truee thats much nicer thank ya sir
| getPersistedState(): Map<string, PersistedFileState> | ||
| } | ||
|
|
||
| export interface ReadonlyRuntime { |
There was a problem hiding this comment.
Is this used besides being re-exported?
|
|
||
| clearExpectedDeleteEcho(filePath: string): void { | ||
| this.memory.clearExpectedDeleteEcho(filePath) | ||
| } |
There was a problem hiding this comment.
This memory facade just pipes through memory methods directly which feels like its mostly plumbing?
It feels like calling runtime.memory.doXX in callers directly is fine since we're plumbing through almost all methods here. Unless you wanna avoid exposing some internal state?
| /** | ||
| * Plugin-side peer state: one map of normalized path → latest known content (snapshot + echo suppression). | ||
| */ | ||
| export class PluginBase { |
There was a problem hiding this comment.
nit imo this naming is a bit weird, its also only ever used within CodeFilesAPI, so i kinda feel like it should just be part of that class instead of introducing inheritance
There was a problem hiding this comment.
thats true I had a similar thought, will fixy
niekert
left a comment
niekert
left a comment
There was a problem hiding this comment.
LGTM. Clearly an improvement in architecture 👏 It was a lil difficult to fully trace the behavioral changes but I did quite a bit of QA too
Summary
This refactor makes Code Link sync easier to trace by flattening the core path instead of adding more layers:
SyncEvent -> transition -> Effect -> applyEffect -> SyncRuntime/SyncMemoryThe controller stays as the single orchestration surface, but the race-sensitive state now has clear homes:
SyncRuntimeowns lifecycle state: workspace paths, active connection, pending renames, active prompt sessions, disconnect UI, installer, and deferred prompt-sensitive state.SyncMemoryowns file sync truth: agreed metadata, inbound content echoes, delete tombstones, and normalized path handling.Effectremains the flat controller plan step. The old intermediate execution layers (EffectResult,RuntimeOp,SendIntent, prompt refresh flags) are gone.applyEffectnow performs each named side effect directly, so send/write/delete/prompt behavior can be read in one place.What changed
Race handling
The important sync commits are now success-driven and visible in the relevant
applyEffectcase:Plugin/UI updates
Testing
Added focused coverage using real collaborators where practical: real
SyncRuntime, realSyncMemory, temp directories, metadata cache, and the existing in-memory websocket shape.Covered scenarios include:
Verified:
yarn tsc --project packages/code-link-cli/tsconfig.json --noEmityarn tsc --project plugins/code-link/tsconfig.json --noEmityarn tsc --project packages/code-link-shared/tsconfig.json --noEmityarn biome check ...