Skip to content

feat(cli): remote folders and telemetry consent; fix spaces in remote folder names - #102

Merged
mthines merged 9 commits into
mainfrom
ccr-a6545f0b-13i3bj
Oct 9, 2026
Merged

mthines merged 9 commits into
mainfrom
ccr-a6545f0b-13i3bj

Conversation

@mthines

@mthines mthines commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

Three changes, plus the review loop's follow-up fixes.

fix(settings): keep spaces in remote folder names (87a4ee5, e07777e, 7ee2ce4)

Reported on Reddit: a remote folder named My Folder showed up as Folder in the profile editor's folder chooser. The chooser parsed rclone lsd output, which is a whitespace-padded column, by splitting on spaces. Everything before the last space was lost. This is not a regression; the bug has been there since the chooser was added (792bfe8).

  • One listing path: all three listing sites now use rclone lsf --dirs-only, which prints one name/ per line verbatim. They share folderListingArguments and parseDirectoryListing. The three sites are the setup wizard (RcloneConfigService.listFolders), the folder chooser (ProfileDetailView.loadRemoteFolders) and the "Sync Only These Folders" browser (RemoteFolderBrowserSheet). Leading, trailing and doubled spaces, and non-ASCII names, survive. The wizard's list is now sorted.
  • No more hang on large output: stdout and stderr are drained at the same time by one helper, RcloneConfigService.drainPipes. Reading one pipe to the end first deadlocks once the other fills its ~64 KB buffer. The CLI's own rclone runner now uses the same helper. stderr is kept apart from the listing, so an rclone warning can't show up as a folder.

feat(cli): synctray remote folders <remote>[:path] [--json] (fb68598)

Lists a remote's folders exactly as the app shows them, with the same arguments (including the remote's no_check_certificate) and the same parser. It needs no profile. --json makes leading and trailing spaces visible.

feat(cli): ask for usage-data consent once and share it with the app (1120507, 612b050, 0bf1782)

  • One-time question: the CLI asks once whether to share usage data, using the app banner's own copy (Share usage data? [y/N], default No). It asks only on an interactive run of a command that parses, and only while the question is unanswered. The question counts as answered once the user has answered it in the app's banner or setup wizard, or in the CLI, for the current consent version.
    • Known gap, unchanged by this PR: flipping the Settings toggle doesn't mark the question answered. Someone who only used that toggle to keep usage data off is asked once in the CLI.
  • Never blocks scripts or agents: stdin and stderr must both be a TTY and CI must be unset; help and telemetry never ask.
  • synctray telemetry [status|on|off] [--json]: the same telemetryEnabled setting the app uses. It also writes that one key to settings.json, so a running app applies the change live, and it never touches launchAtLogin.
  • Telemetry source: CLI telemetry carries the resource attribute synctray.process.kind=cli, and the app sends app. service.name stays synctray, so existing queries keep working.
  • Quieter stderr: exporter diagnostics are silent in CLI mode.

Not addressed here

The include-folder browser now shows folder names with leading or trailing spaces exactly as they are. SyncProfile.normalizedSyncIncludeFolders still trims them on save, though, so such a folder can't be selected for "Sync Only These Folders". The trimming predates this PR.

Test plan

  • SyncTray --self-test: 123 checks pass. Each check below was confirmed to fail against the broken code.
    • AC-RF1: parser checks, a real child process writing 300 KB to each pipe, and source pins on all four call sites.
    • AC-CLI12: remote folders.
    • AC-CLI13: when the question is asked (including a typo staying silent), default No, telemetry parse/output, and the settings.json key-only update.
  • Real rclone against folders named My Folder, My songs, trailing space and Café: listed verbatim as text and as --json.
  • Real binary: telemetry status/on/off; piped runs never ask; a bad argument exits 64; settings.json round-trips byte-identical.
  • CI green; scripts/check-schema-in-sync.sh passes.

claude and others added 3 commits October 9, 2026 04:35
The profile editor's remote folder dropdown split `rclone lsd` output on
whitespace and kept the last token, so "My Folder" was offered as
"Folder" (colliding with a real folder of that name) and "My songs" as
"songs". Present since the view was written, not a regression.

List folders with `rclone lsf --dirs-only` (one name per line, no
columns) through a shared RcloneConfigService.parseDirectoryListing used
by the chooser, the setup wizard and the Sync Only These Folders browser.
stderr is now read separately so rclone notices can't show up as folders.
Covered by ConfigSelfTest AC-RF1.

Claude-Session: https://claude.ai/code/session_01DXZWhCBFdN1o8jnSB8gd7t
…s the app sees them

Lists a remote's folders through the same rclone arguments and parser as
the profile editor's folder chooser and the setup wizard, so a folder-name
problem like the "My Folder" shown as "Folder" bug can be reproduced and
reported from a terminal. Needs no profile; --json makes leading and
trailing spaces visible.

The listing arguments now live in RcloneConfigService.folderListingArguments,
shared by the chooser, the wizard and the CLI. Covered by ConfigSelfTest
AC-CLI12.
The CLI now honours the same telemetry setting as the app instead of
silently following whatever the app last stored:

- First run asks once ("Share usage data? [y/N]") with the banner's own
  copy, default No. Only on an interactive terminal outside CI, never for
  help or telemetry itself, so scripts and agents are never blocked.
- `synctray telemetry [status|on|off] [--json]` reads/changes it, and
  writes settings.json (that key only) so a running app applies it live.
- CLI telemetry carries synctray.process.kind=cli (app sends app);
  service.name stays synctray so existing queries keep working.
- Exporter diagnostics are silent in CLI mode.

Covered by ConfigSelfTest AC-CLI13.
@dash0-dev

dash0-dev Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Darkplane auto-approval is enabled for this repository (mode: Auto-approve).

Trigger an evaluation for this pull request when it is ready:

Run auto-approval evaluation

@mthines

mthines commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner Author

✅ No issues found

No new commits since the last review at a1831f8; the edited description now covers the follow-up fixes, all three prior threads are resolved, and CI is green.

Progress: open review threads 1 → 2 → 0 → 0 across the last 4 reviews

Review details
Gate Status Details
Description vs. code ✅ Description now covers all three changes and the review-loop follow-ups (parse gate, drainPipes)
Prior review feedback ✅ Earlier review comments are resolved.
Documentation ✅ Description, CLAUDE.md and telemetry.md document the new commands, consent flow and drainPipes
Self-review signals ✅ No debug logs, leftover TODOs, or unreviewed stubs.
Code review ✅ The multi-lens review found no blocking issues.

Found

Quality — produced 0 → posted inline 0 · cleared 0 · carried forward 0 · deferred 0 · below-bar 0

Run

incremental · 0 lines in delta · tier deep · depth checkout
Memories — skipped (zero-delta fast path)

Nothing to report — standards (skipped), optimality (skipped), measurability (skipped), integrations (skipped), severity, 0 files skipped.

pr-reviewer · commit a1831f8 · no code changes since a1831f8, gate checks only · how these findings are produced · updated Oct 9, 2026 7:28am UTC

Comment thread SyncTray/CLI/SyncTrayCLI.swift
Comment thread SyncTray/Services/RcloneConfigService.swift Outdated
Comment thread SyncTray/Services/RcloneConfigService.swift
…ting folders

Reading stdout to EOF before stderr deadlocks once rclone fills the unread
stderr pipe's ~64 KB buffer. Add RcloneConfigService.drainPipes, which reads
stderr on a background queue while reading stdout, and use it at all three
folder-listing sites. AC-RF1 now runs a child writing 300 KB to both pipes
under a 10 s timeout and pins drainPipes at each site.

Addresses #102 (comment)
@mthines
mthines merged commit 4064cf4 into main Oct 9, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants