andras/vault-link
1
0
Fork 0
Code Issues Pull requests 18 Projects Releases 55 Packages Wiki Activity Actions 1

Improve settings #168

Merged
schmelczer merged 14 commits from asch/explore into main 2025-11-19 19:53:10 +00:00
Conversation 1 Commits 14 Files changed 19 +302 -128
schmelczer commented 2025-11-18 22:18:58 +00:00 (Migrated from github.com)
Copy link

This pull request introduces Docker healthcheck support for the local client CLI and removes telemetry and error reporting dependencies from the Obsidian plugin. It also adds an option to enable telemetry in the CLI and plugin settings, improves connection status reporting, and updates some package dependencies.

Docker healthcheck and CLI improvements:

  • Added a healthcheck.js script and Docker healthcheck configuration to the CLI Dockerfile, allowing containers to report healthy status based on sync connection. The CLI now writes connection status to a health file for the healthcheck to read. (frontend/local-client-cli/Dockerfile, frontend/local-client-cli/src/cli.ts, frontend/local-client-cli/src/healthcheck.ts, frontend/local-client-cli/webpack.config.js) [1] [2] [3] [4] [5]
  • Added a new --health CLI argument and health file handling in argument parsing and main logic. (frontend/local-client-cli/src/args.ts, frontend/local-client-cli/src/cli.ts) [1] [2] [3] [4] [5]
  • Added an --enable-telemetry CLI option and passed it into client settings. (frontend/local-client-cli/src/args.ts, frontend/local-client-cli/src/cli.ts) [1] [2] [3] [4]

Obsidian plugin telemetry and error reporting:

  • Removed Plausible Analytics and Sentry error reporting from the plugin code and dependencies. (frontend/obsidian-plugin/package.json, frontend/obsidian-plugin/src/vault-link-plugin.ts, frontend/package-lock.json) [1] [2] [3] [4] [5]
  • Added a "Enable telemetry" toggle to the plugin settings UI, allowing users to opt in/out of anonymous usage data and error reports. (frontend/obsidian-plugin/src/views/settings/settings-tab.ts) [1] [2]

Dependency updates and code improvements:

  • Updated reconcile-text dependency version and removed unused dependencies (virtual-scroller, Plausible) from package files. (frontend/obsidian-plugin/package.json, frontend/package-lock.json) [1] [2] [3] [4] [5] [6]
  • Improved connection status and error reporting in CLI and plugin UI, including more informative log messages and notices. (frontend/local-client-cli/src/cli.ts, frontend/obsidian-plugin/src/views/settings/settings-tab.ts) [1] [2]

Let me know if you want more details about any specific change!

This pull request introduces Docker healthcheck support for the local client CLI and removes telemetry and error reporting dependencies from the Obsidian plugin. It also adds an option to enable telemetry in the CLI and plugin settings, improves connection status reporting, and updates some package dependencies. **Docker healthcheck and CLI improvements:** * Added a `healthcheck.js` script and Docker healthcheck configuration to the CLI Dockerfile, allowing containers to report healthy status based on sync connection. The CLI now writes connection status to a health file for the healthcheck to read. (`frontend/local-client-cli/Dockerfile`, `frontend/local-client-cli/src/cli.ts`, `frontend/local-client-cli/src/healthcheck.ts`, `frontend/local-client-cli/webpack.config.js`) [[1]](diffhunk://#diff-9dcc1e76e154116995b05c39398f08e9d5d9760ce183a8e2ddd8b6fc6959491bR19-R28) [[2]](diffhunk://#diff-a58d57dcc663a78c363605a90b2c00d90a695210ab87e1f8abca48d5a96fd5e8R1-R59) [[3]](diffhunk://#diff-599f6d860cb661d93e4acdd06125d60672a07017c559c3534193d870b6dc32d5R18-R30) [[4]](diffhunk://#diff-f7ce0a70d3334185538cc48b162e5018f7163b7af1dfc92871288a62abcdd2afL5-R8) [[5]](diffhunk://#diff-f7ce0a70d3334185538cc48b162e5018f7163b7af1dfc92871288a62abcdd2afL24-R27) * Added a new `--health` CLI argument and health file handling in argument parsing and main logic. (`frontend/local-client-cli/src/args.ts`, `frontend/local-client-cli/src/cli.ts`) [[1]](diffhunk://#diff-2db140fccff3b9f2755eb1f9f8ac6aab04d33293f5562de95754c7240e7aadf0R15-R16) [[2]](diffhunk://#diff-2db140fccff3b9f2755eb1f9f8ac6aab04d33293f5562de95754c7240e7aadf0R56-R63) [[3]](diffhunk://#diff-2db140fccff3b9f2755eb1f9f8ac6aab04d33293f5562de95754c7240e7aadf0R91-R92) [[4]](diffhunk://#diff-2db140fccff3b9f2755eb1f9f8ac6aab04d33293f5562de95754c7240e7aadf0L120-R134) [[5]](diffhunk://#diff-599f6d860cb661d93e4acdd06125d60672a07017c559c3534193d870b6dc32d5R139-R147) * Added an `--enable-telemetry` CLI option and passed it into client settings. (`frontend/local-client-cli/src/args.ts`, `frontend/local-client-cli/src/cli.ts`) [[1]](diffhunk://#diff-2db140fccff3b9f2755eb1f9f8ac6aab04d33293f5562de95754c7240e7aadf0R56-R63) [[2]](diffhunk://#diff-2db140fccff3b9f2755eb1f9f8ac6aab04d33293f5562de95754c7240e7aadf0R91-R92) [[3]](diffhunk://#diff-2db140fccff3b9f2755eb1f9f8ac6aab04d33293f5562de95754c7240e7aadf0L120-R134) [[4]](diffhunk://#diff-599f6d860cb661d93e4acdd06125d60672a07017c559c3534193d870b6dc32d5R96-R102) **Obsidian plugin telemetry and error reporting:** * Removed Plausible Analytics and Sentry error reporting from the plugin code and dependencies. (`frontend/obsidian-plugin/package.json`, `frontend/obsidian-plugin/src/vault-link-plugin.ts`, `frontend/package-lock.json`) [[1]](diffhunk://#diff-bd7c6081b2a92d46cae09ae33371031674691cc21ea3df329332e3a5f74e4edcL16-R23) [[2]](diffhunk://#diff-aa8964e93750f3c65fd44540d16ac4656a6f09017cdd25e08b7f04832b30306aL14-L15) [[3]](diffhunk://#diff-aa8964e93750f3c65fd44540d16ac4656a6f09017cdd25e08b7f04832b30306aL53-L91) [[4]](diffhunk://#diff-4a2d9aa3e849b134993936ca81b83fb139edd2b0218077ab0f403b8c4803c62aL874-L880) [[5]](diffhunk://#diff-4a2d9aa3e849b134993936ca81b83fb139edd2b0218077ab0f403b8c4803c62aL4664-R4650) * Added a "Enable telemetry" toggle to the plugin settings UI, allowing users to opt in/out of anonymous usage data and error reports. (`frontend/obsidian-plugin/src/views/settings/settings-tab.ts`) [[1]](diffhunk://#diff-5e760ad7f6ac8556b7ec264010df8bfeb062d1acaf3115194296842f86d46055R75) [[2]](diffhunk://#diff-5e760ad7f6ac8556b7ec264010df8bfeb062d1acaf3115194296842f86d46055R333-R352) **Dependency updates and code improvements:** * Updated `reconcile-text` dependency version and removed unused dependencies (`virtual-scroller`, Plausible) from package files. (`frontend/obsidian-plugin/package.json`, `frontend/package-lock.json`) [[1]](diffhunk://#diff-bd7c6081b2a92d46cae09ae33371031674691cc21ea3df329332e3a5f74e4edcL16-R23) [[2]](diffhunk://#diff-bd7c6081b2a92d46cae09ae33371031674691cc21ea3df329332e3a5f74e4edcL36) [[3]](diffhunk://#diff-4a2d9aa3e849b134993936ca81b83fb139edd2b0218077ab0f403b8c4803c62aL3491-L3506) [[4]](diffhunk://#diff-4a2d9aa3e849b134993936ca81b83fb139edd2b0218077ab0f403b8c4803c62aL4332-L4339) [[5]](diffhunk://#diff-4a2d9aa3e849b134993936ca81b83fb139edd2b0218077ab0f403b8c4803c62aL4664-R4650) [[6]](diffhunk://#diff-4a2d9aa3e849b134993936ca81b83fb139edd2b0218077ab0f403b8c4803c62aL4684) * Improved connection status and error reporting in CLI and plugin UI, including more informative log messages and notices. (`frontend/local-client-cli/src/cli.ts`, `frontend/obsidian-plugin/src/views/settings/settings-tab.ts`) [[1]](diffhunk://#diff-599f6d860cb661d93e4acdd06125d60672a07017c559c3534193d870b6dc32d5L135-R164) [[2]](diffhunk://#diff-5e760ad7f6ac8556b7ec264010df8bfeb062d1acaf3115194296842f86d46055L196-L226) Let me know if you want more details about any specific change!
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2025-11-18 22:19:55 +00:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment
Copy link

Pull Request Overview

This pull request adds Docker healthcheck support to the CLI and introduces opt-in telemetry for both the CLI and Obsidian plugin, while removing hardcoded telemetry from the plugin. It also includes cache management improvements and dependency updates.

Key changes:

  • Docker healthcheck implementation with periodic connection status reporting
  • Opt-in telemetry toggle in both CLI and plugin settings (disabled by default)
  • Cache resize capability and improved lifecycle management

Reviewed Changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
sync-server/src/main.rs Changed logging output from stdout to stderr
frontend/sync-client/src/utils/set-up-telemetry.ts Added new telemetry setup utility with Sentry integration
frontend/sync-client/src/utils/fix-sized-cache.ts Added resize and clear methods, refactored constructor
frontend/sync-client/src/utils/fix-sized-cache.test.ts Added tests for new cache resize functionality
frontend/sync-client/src/sync-operations/syncer.ts Removed contentCache dependency from constructor
frontend/sync-client/src/sync-client.ts Moved cache management to SyncClient, added telemetry lifecycle and settings listeners
frontend/sync-client/src/persistence/settings.ts Added diffCacheSizeMB and enableTelemetry settings
frontend/sync-client/package.json Added @sentry/browser dependency
frontend/obsidian-plugin/src/views/settings/settings-tab.ts Added telemetry toggle, simplified connection test UI
frontend/obsidian-plugin/src/views/cursors/remote-cursors-plugin.ts Made app property optional
frontend/obsidian-plugin/src/vault-link-plugin.ts Removed hardcoded Plausible and Sentry initialization
frontend/obsidian-plugin/package.json Removed @plausible-analytics/tracker, @sentry/browser, virtual-scroller; updated reconcile-text
frontend/local-client-cli/webpack.config.js Added healthcheck entry point with dynamic filename output
frontend/local-client-cli/src/healthcheck.ts Added new healthcheck script for Docker container monitoring
frontend/local-client-cli/src/cli.ts Added health file writing, telemetry support, improved logging
frontend/local-client-cli/src/args.ts Added --health and --enable-telemetry CLI arguments
frontend/local-client-cli/Dockerfile Added healthcheck configuration and health file path
Files not reviewed (1)
  • frontend/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull Request Overview This pull request adds Docker healthcheck support to the CLI and introduces opt-in telemetry for both the CLI and Obsidian plugin, while removing hardcoded telemetry from the plugin. It also includes cache management improvements and dependency updates. Key changes: - Docker healthcheck implementation with periodic connection status reporting - Opt-in telemetry toggle in both CLI and plugin settings (disabled by default) - Cache resize capability and improved lifecycle management ### Reviewed Changes Copilot reviewed 17 out of 19 changed files in this pull request and generated 3 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | sync-server/src/main.rs | Changed logging output from stdout to stderr | | frontend/sync-client/src/utils/set-up-telemetry.ts | Added new telemetry setup utility with Sentry integration | | frontend/sync-client/src/utils/fix-sized-cache.ts | Added resize and clear methods, refactored constructor | | frontend/sync-client/src/utils/fix-sized-cache.test.ts | Added tests for new cache resize functionality | | frontend/sync-client/src/sync-operations/syncer.ts | Removed contentCache dependency from constructor | | frontend/sync-client/src/sync-client.ts | Moved cache management to SyncClient, added telemetry lifecycle and settings listeners | | frontend/sync-client/src/persistence/settings.ts | Added diffCacheSizeMB and enableTelemetry settings | | frontend/sync-client/package.json | Added @sentry/browser dependency | | frontend/obsidian-plugin/src/views/settings/settings-tab.ts | Added telemetry toggle, simplified connection test UI | | frontend/obsidian-plugin/src/views/cursors/remote-cursors-plugin.ts | Made app property optional | | frontend/obsidian-plugin/src/vault-link-plugin.ts | Removed hardcoded Plausible and Sentry initialization | | frontend/obsidian-plugin/package.json | Removed @plausible-analytics/tracker, @sentry/browser, virtual-scroller; updated reconcile-text | | frontend/local-client-cli/webpack.config.js | Added healthcheck entry point with dynamic filename output | | frontend/local-client-cli/src/healthcheck.ts | Added new healthcheck script for Docker container monitoring | | frontend/local-client-cli/src/cli.ts | Added health file writing, telemetry support, improved logging | | frontend/local-client-cli/src/args.ts | Added --health and --enable-telemetry CLI arguments | | frontend/local-client-cli/Dockerfile | Added healthcheck configuration and health file path | </details> <details> <summary>Files not reviewed (1)</summary> * **frontend/package-lock.json**: Language not supported </details> --- 💡 <a href="/schmelczer/vault-link/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <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>.
frontend/local-client-cli/src/cli.ts
@ -119,6 +136,21 @@ async function main(): Promise<void> {
nativeLineEndings: process.platform === "win32" ? "\r\n" : "\n"
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-11-18 22:19:54 +00:00
Copy link

The setInterval creates an untracked timer that will continue running indefinitely. Consider storing the timer reference and clearing it during cleanup or process termination to prevent potential resource leaks.

		const healthInterval = setInterval(() => {
			void client.checkConnection().then((status) => {
				writeHealthStatus(healthFile, status);
			});
		}, 30 * 1000); // every 30 seconds
		const clearHealthInterval = () => clearInterval(healthInterval);
		process.on("SIGINT", clearHealthInterval);
		process.on("SIGTERM", clearHealthInterval);
		process.on("exit", clearHealthInterval);
The setInterval creates an untracked timer that will continue running indefinitely. Consider storing the timer reference and clearing it during cleanup or process termination to prevent potential resource leaks. ```suggestion const healthInterval = setInterval(() => { void client.checkConnection().then((status) => { writeHealthStatus(healthFile, status); }); }, 30 * 1000); // every 30 seconds const clearHealthInterval = () => clearInterval(healthInterval); process.on("SIGINT", clearHealthInterval); process.on("SIGTERM", clearHealthInterval); process.on("exit", clearHealthInterval); ```
frontend/local-client-cli/src/healthcheck.ts
@ -0,0 +1,66 @@
#!/usr/bin/env node
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-11-18 22:19:54 +00:00
Copy link

The type guard always returns true for any non-null object, failing to validate the actual structure of NetworkConnectionStatus. This will pass invalid objects that lack required properties like isSuccessful, isWebSocketConnected, or serverMessage, leading to runtime errors when those properties are accessed on line 44.

	const obj = value as Record<string, unknown>;
	return (
		typeof obj.isSuccessful === "boolean" &&
		typeof obj.isWebSocketConnected === "boolean" &&
		typeof obj.serverMessage === "string"
	);
The type guard always returns true for any non-null object, failing to validate the actual structure of NetworkConnectionStatus. This will pass invalid objects that lack required properties like isSuccessful, isWebSocketConnected, or serverMessage, leading to runtime errors when those properties are accessed on line 44. ```suggestion const obj = value as Record<string, unknown>; return ( typeof obj.isSuccessful === "boolean" && typeof obj.isWebSocketConnected === "boolean" && typeof obj.serverMessage === "string" ); ```
frontend/obsidian-plugin/src/views/settings/settings-tab.ts
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-11-18 22:19:54 +00:00
Copy link

The tooltip repeats the exact same text as the description (setDesc). Tooltips should provide additional context or clarification, not duplicate the description verbatim. Either remove the tooltip or provide supplementary information.

				"Only basic usage statistics and error reports are collected. No personal or note content is ever sent."
The tooltip repeats the exact same text as the description (setDesc). Tooltips should provide additional context or clarification, not duplicate the description verbatim. Either remove the tooltip or provide supplementary information. ```suggestion "Only basic usage statistics and error reports are collected. No personal or note content is ever sent." ```
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

Something isn't working

  
dependencies

Pull requests that update a dependency file

  
docker

Pull requests that update docker code

  
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
enhancement

New feature or request

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
javascript

Pull requests that update javascript code

  
question

Further information is requested

  
rust

Pull requests that update rust code

  
wontfix

This will not be worked on

No labels
bug
dependencies
docker
documentation
duplicate
enhancement
good first issue
help wanted
invalid
javascript
question
rust
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: andras/vault-link#168
No description provided.
Delete branch "asch/explore"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?