Add local CLI #144

2025-10-20T20:27:29+01:00

schmelczer commented

2025-10-20 20:27:29 +01:00

(Migrated from github.com)

Add client service for syncing a vault's files locally
Fix server race-condition

- Add client service for syncing a vault's files locally - Fix server race-condition

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2025-10-21 22:42:15 +01:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

This PR adds a new standalone CLI application for VaultLink that enables local filesystem synchronization with real-time bidirectional sync and file watching capabilities.

Key Changes

Created a new local-client-cli workspace package with full CLI implementation including argument parsing, file system operations, and file watching
Refactored database transaction management in the sync-server to centralize transaction commits within the insert_document_version method
Removed conditional telemetry/error tracking initialization from the Obsidian plugin to always enable these features

Reviewed Changes

Copilot reviewed 22 out of 24 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
sync-server/src/server/update_document.rs	Removed manual transaction commit, now handled by insert_document_version
sync-server/src/server/delete_document.rs	Removed manual transaction commit and anyhow Context import
sync-server/src/server/create_document.rs	Removed manual transaction commit and anyhow Context import
sync-server/src/app_state/database.rs	Centralized transaction commit logic within insert_document_version method
scripts/check.sh	Added recursive fix mode execution logic
frontend/test-client/package.json	Removed bufferutil dependency
frontend/package.json	Added local-client-cli workspace and updated lint script
frontend/obsidian-plugin/src/vault-link-plugin.ts	Removed debug build check to always enable telemetry and error tracking
frontend/local-client-cli/webpack.config.js	New webpack configuration for CLI bundling
frontend/local-client-cli/tsconfig.json	TypeScript configuration for CLI package
frontend/local-client-cli/src/node-filesystem.ts	Node.js filesystem operations implementation with cross-platform path handling
frontend/local-client-cli/src/node-filesystem.test.ts	Comprehensive test suite for filesystem operations
frontend/local-client-cli/src/logger-formatter.ts	Colorized log formatting utilities for CLI output
frontend/local-client-cli/src/file-watcher.ts	File system watcher implementation using fs.watch
frontend/local-client-cli/src/cli.ts	Main CLI entry point with sync client initialization and lifecycle management
frontend/local-client-cli/src/args.ts	Command-line argument parsing with validation
frontend/local-client-cli/src/args.test.ts	Test suite for argument parsing
frontend/local-client-cli/package.json	Package configuration with dependencies and scripts
frontend/local-client-cli/README.md	Comprehensive documentation for CLI usage and deployment
frontend/local-client-cli/Dockerfile	Multi-stage Docker build configuration
frontend/local-client-cli/.dockerignore	Docker build exclusions
.github/workflows/publish-cli-docker.yml	GitHub Actions workflow for Docker image publishing

Files not reviewed (1)

frontend/package-lock.json: Language not supported

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

## Pull Request Overview This PR adds a new standalone CLI application for VaultLink that enables local filesystem synchronization with real-time bidirectional sync and file watching capabilities. ### Key Changes - Created a new `local-client-cli` workspace package with full CLI implementation including argument parsing, file system operations, and file watching - Refactored database transaction management in the sync-server to centralize transaction commits within the `insert_document_version` method - Removed conditional telemetry/error tracking initialization from the Obsidian plugin to always enable these features ### Reviewed Changes Copilot reviewed 22 out of 24 changed files in this pull request and generated 4 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | sync-server/src/server/update_document.rs | Removed manual transaction commit, now handled by insert_document_version | | sync-server/src/server/delete_document.rs | Removed manual transaction commit and anyhow Context import | | sync-server/src/server/create_document.rs | Removed manual transaction commit and anyhow Context import | | sync-server/src/app_state/database.rs | Centralized transaction commit logic within insert_document_version method | | scripts/check.sh | Added recursive fix mode execution logic | | frontend/test-client/package.json | Removed bufferutil dependency | | frontend/package.json | Added local-client-cli workspace and updated lint script | | frontend/obsidian-plugin/src/vault-link-plugin.ts | Removed debug build check to always enable telemetry and error tracking | | frontend/local-client-cli/webpack.config.js | New webpack configuration for CLI bundling | | frontend/local-client-cli/tsconfig.json | TypeScript configuration for CLI package | | frontend/local-client-cli/src/node-filesystem.ts | Node.js filesystem operations implementation with cross-platform path handling | | frontend/local-client-cli/src/node-filesystem.test.ts | Comprehensive test suite for filesystem operations | | frontend/local-client-cli/src/logger-formatter.ts | Colorized log formatting utilities for CLI output | | frontend/local-client-cli/src/file-watcher.ts | File system watcher implementation using fs.watch | | frontend/local-client-cli/src/cli.ts | Main CLI entry point with sync client initialization and lifecycle management | | frontend/local-client-cli/src/args.ts | Command-line argument parsing with validation | | frontend/local-client-cli/src/args.test.ts | Test suite for argument parsing | | frontend/local-client-cli/package.json | Package configuration with dependencies and scripts | | frontend/local-client-cli/README.md | Comprehensive documentation for CLI usage and deployment | | frontend/local-client-cli/Dockerfile | Multi-stage Docker build configuration | | frontend/local-client-cli/.dockerignore | Docker build exclusions | | .github/workflows/publish-cli-docker.yml | GitHub Actions workflow for Docker image publishing | </details> <details> <summary>Files not reviewed (1)</summary> * **frontend/package-lock.json**: Language not supported </details> --- <sub>**Tip:** Customize your code reviews with copilot-instructions.md. <a href="/schmelczer/vault-link/new/main/.github?filename=copilot-instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Create the file</a> or <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">learn how to get started</a>.</sub>

frontend/local-client-cli/src/cli.ts

					
				@ -0,0 +1,207 @@

				import * as path from "path";

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-21 22:42:14 +01:00

This commented-out code creates a never-resolving promise that would block the program indefinitely. If this was intended to keep the process alive, it should either be removed or uncommented with a clear explanation. Currently, without this or any other blocking mechanism, the program may exit prematurely after starting the file watcher.

		// Block the main function to keep the process alive until interrupted.
		await new Promise<void>(() => {
			// This promise never resolves; process stays alive until SIGINT/SIGTERM.
		});

This commented-out code creates a never-resolving promise that would block the program indefinitely. If this was intended to keep the process alive, it should either be removed or uncommented with a clear explanation. Currently, without this or any other blocking mechanism, the program may exit prematurely after starting the file watcher. ```suggestion // Block the main function to keep the process alive until interrupted. await new Promise<void>(() => { // This promise never resolves; process stays alive until SIGINT/SIGTERM. }); ```

frontend/local-client-cli/src/cli.ts

					
				@ -0,0 +93,4 @@

								try {

									const content = await fs.readFile(dataFile, "utf-8");

									// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion

									database = JSON.parse(content) as Partial<StoredDatabase>;

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-21 22:42:14 +01:00

The type assertion bypasses type safety without validation. The parsed JSON should be validated to ensure it matches the expected StoredDatabase structure before casting, or handle potential type mismatches gracefully in case the data file is corrupted.

scripts/check.sh

					
				@ -42,0 +41,4 @@

				if [[ "$FIX_MODE" == true ]]; then

				    $0

				fi

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-21 22:42:14 +01:00

Recursive call to the script without arguments will lose the FIX_MODE=true state, causing infinite recursion or unexpected behavior. The script should pass the original arguments or at least preserve the FIX_MODE flag: $0 \"$@\" or FIX_MODE=true $0.

    $0 "$@"

Recursive call to the script without arguments will lose the FIX_MODE=true state, causing infinite recursion or unexpected behavior. The script should pass the original arguments or at least preserve the FIX_MODE flag: `$0 \"$@\"` or `FIX_MODE=true $0`. ```suggestion $0 "$@" ```

sync-server/src/app_state/database.rs

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-21 22:42:13 +01:00

The transaction ownership has changed from taking a mutable reference (&mut Transaction) to taking ownership (Transaction). This prevents the caller from handling commit failures or performing additional operations. Consider documenting this breaking change in the function comment or reverting to the reference-based approach if callers need transaction control.

The transaction ownership has changed from taking a mutable reference (`&mut Transaction`) to taking ownership (`Transaction`). This prevents the caller from handling commit failures or performing additional operations. Consider documenting this breaking change in the function comment or reverting to the reference-based approach if callers need transaction control.

Sign in to join this conversation.

No reviewers