Skip to content

perf(sync): cut sync-related startup latency — drop dead --hash, feedback, --offline#104

Merged
arimxyer merged 2 commits into
mainfrom
feat/96-sync-latency
Jun 26, 2026
Merged

perf(sync): cut sync-related startup latency — drop dead --hash, feedback, --offline#104
arimxyer merged 2 commits into
mainfrom
feat/96-sync-latency

Conversation

@arimxyer

Copy link
Copy Markdown
Collaborator

Closes #96.

Cuts the sync-related startup latency (and makes it visible), without touching the
change-detection model. Three low-risk pieces, plus review-driven fixes.

What changed

1. Drop the dead rclone lsjson --hash (internal/sync/sync.go)
CheckRemoteMetadata requested backend hashes that were never deserialized
(RemoteFileInfo has no hash field) and never used by any decision — the skip
check uses ModTime+Size and conflict detection uses a local sha256. On many
backends --hash forces extra server-side work, so removing it makes the
pre-unlock probe cheaper with zero behavior change. (Note: the backend hash is
the wrong tool to keep — see #102 for real content-hash change detection.)

2. Feedback by default (cmd/helpers.go)
The pre-unlock probe is a synchronous network round-trip; previously it was silent
unless --verbose. Now a transient Checking remote... indicator shows on
stderr (cleared with CR + ANSI erase, same as the existing Syncing...),
only when sync is enabled and not offline. stdout stays byte-clean for pipes.

3. --offline global flag (cmd/root.go)
Skips cloud sync for one command — run fully local. (--no-sync is taken by
init, so it's --offline.) It skips both the pre-unlock pull and the
post-command push: skipping only the pull would be unsafe, since SmartPush has
no independent remote-conflict check and could blind-overwrite a newer remote.
Offline changes simply sync on the next online run.

Review-driven fixes (second commit)

An adversarial review (3 lenses: sync-correctness, output-hygiene, scope/tests)
surfaced one medium issue and some polish:

  • TUI ignored --offline (medium): runTUI called SyncPull/SyncPush
    directly, so pass-cli --offline (the default TUI launch) and
    pass-cli tui --offline still hit the network. Both now route through the
    gated helpers, so the TUI honors --offline and matches CLI feedback. (The
    unreferenced cmd/tui/main.go:Run is dead code — left untouched.)
  • Docs: --offline added to the command-reference Global Options table and
    the sync guide's Offline Operation section.
  • Test: added TestSyncPushAfterCommand_FeedbackOnStderr locking the normal
    (sync-enabled, non-offline) push path.

Verification

  • gofmt -l, go vet, go build, golangci-lint (v2.5.0): clean
  • go test ./...: green
  • go test -race ./internal/sync/... ./cmd/...: green, no data races
  • go test -tags=integration ./test/integration/...: green (55s)
  • Manual smoke (built binary, sync-enabled config w/ a bogus remote): list
    hides usernames by default, list -q prints bare names, get --quiet returns
    only the value; online runs show the Checking remote... probe feedback;
    --offline skips the probe entirely and still returns the value.
  • TUI fix verified by wiring (the two live sync sites in runTUI now go
    through the offline-gated, unit-tested helpers) rather than a headless TUI run,
    since the TUI is interactive.

Known minor (deferred, not blocking)

  • The transient Checking remote... indicator clears cleanly on the happy path,
    but on the sync-failure path (rclone error / network down for a sync-enabled
    user
    ) the internal warning glues onto it on a single stderr line. Cosmetic,
    stderr-only, and the user is being told sync failed anyway. Clean fix (a
    follow-up) is in the sync layer: prefix SmartPull's warnings with the clear.
  • No isatty guard on the transient indicators (matches the pre-existing
    Syncing... pattern); offline: true in config disables sync persistently
    (exact parity with --verbose's viper binding).

Follow-ups (rebase on top of this)

🤖 Generated with Claude Code

arimxyer and others added 2 commits June 26, 2026 17:08
…+ --offline

Three low-risk pieces from issue #96:

1. CheckRemoteMetadata no longer passes "--hash" to rclone lsjson. The
   RemoteFileInfo struct has no hash field and no decision uses a remote hash
   (skip compares ModTime+Size; conflict detection uses a local sha256), so the
   flag only added backend hashing cost. Behavior-neutral.

2. syncPullBeforeUnlock now gives feedback by default. When sync is enabled and
   the command is not offline, the network probe always runs, so it shows a
   transient progress indicator on stderr (cleared with CR + ANSI erase, the
   same technique syncPushAfterCommand uses); verbose keeps the descriptive
   message. Nothing is ever written to stdout, and nothing at all when sync is
   disabled or --offline is set.

3. New persistent --offline flag (mirrors --verbose wiring) with an IsOffline()
   accessor. --offline skips BOTH directions — the pre-unlock pull and the
   post-command push. Skipping only one is unsafe: SmartPush has no independent
   remote-conflict check, its only safety is that SmartPull ran first, so an
   offline push could blind-overwrite a newer remote (silent cross-device data
   loss). --offline means fully local; changes sync on the next online run.

Out of scope (separate issues): concurrent-unlock (#103) and the
change-detection predicate (#102) are untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx
… push

Addresses the adversarial review of the #96 trio:

- TUI ignored --offline (medium): runTUI called SyncPull/SyncPush directly,
  bypassing the offline gate, so `pass-cli --offline` (default TUI launch) and
  `pass-cli tui --offline` still hit the network in both directions. Route both
  through syncPullBeforeUnlock / syncPushAfterCommand so the TUI honors --offline
  and matches CLI sync feedback. The TUI keeps its HasWriteOperations() gate on
  push; the helper supplies the offline + sync-enabled checks. (cmd/tui/main.go's
  Run() is dead — unreferenced — so it's left untouched.)
- Document --offline in the command reference Global Options table and the
  sync guide's Offline Operation section (skips both directions; why both).
- Add TestSyncPushAfterCommand_FeedbackOnStderr locking the normal (sync-enabled,
  non-offline) push path, mirroring the pull feedback test.

Verified: gofmt/vet/build/golangci-lint clean; go test ./... and
go test -race ./internal/sync/... ./cmd/... all green; manual smoke test
confirms feedback fires online, --offline skips the probe, and list/get
safety defaults hold.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx
@arimxyer arimxyer merged commit 67ca1d1 into main Jun 26, 2026
11 checks passed
@arimxyer arimxyer deleted the feat/96-sync-latency branch June 26, 2026 21:43
arimxyer added a commit that referenced this pull request Jun 27, 2026
…#111)

Two user-facing changes shipped without docs and are now covered ahead of
the 0.18.0 release:

- exec (#98): new command reference entry, and scripting-guide rewritten to
  lead with `pass-cli exec` as the recommended way to hand a credential to a
  child command (env injection, never touches files/clipboard/history). The
  old `$(pass-cli get ...)` substitution is kept as a documented fallback with
  an explicit shell-tracing/CI-log leak caveat. README gains exec examples.
- list safe-by-default (#95, #97): documented that the table now hides the
  Username column by default, plus the new --show-usernames and -q/--quiet
  flags, across command-reference, scripting-guide, and README.

Also:
- CHANGELOG: populate the long-stale [Unreleased] (top entry was 0.17.2) with
  everything since — exec, list-safe, sync content-hash markers (#102),
  unlock-latency overlap (#103), startup-latency cleanup (#104), tracked
  mise.toml (#100), CI-on-all-PRs (#105), agent skill (#101) — and head it as
  [0.18.0].
- CLAUDE.md: refresh the Sync Architecture notes for the #102 marker-based
  change detection and the #103 overlapped pre-unlock pull.

No code changed.


Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
arimxyer added a commit that referenced this pull request Jun 27, 2026
…ndings)

The blank-check verification caught two doc bugs shipped in #111:

- CHANGELOG #104 entry was inverted (the misleading squash subject "drop dead
  --hash, feedback, --offline" drove it). Verified against the #104 diff
  (67ca1d1): it removed the dead `rclone lsjson --hash` flag, ADDED pull-side
  sync feedback (not removed; still live at cmd/helpers.go:584), and ADDED a new
  global `--offline` flag. Corrected the entry and moved `--offline` to Added.
  Also added a [0.18.0] note for the arimxyer->reyamira install-source change.
- scripting-guide.md: fixed a dead `#configuration` anchor -> the configuration
  reference doc.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx
arimxyer added a commit that referenced this pull request Jun 27, 2026
* chore(org): complete arimxyer -> reyamira repo reference sweep

The repo transferred from the `arimxyer` org to `reyamira` (the old URLs now
redirect). This rewrites every repository/web/publish reference to reyamira so
0.18.0 — the first release since the transfer — publishes to the right place.

Changed:
- .goreleaser.yml: release.github.owner + brews/scoops repository.owner ->
  reyamira; homepage/install snippets/changelog URLs -> reyamira.
- homebrew/pass-cli.rb, scoop/pass-cli.json: homepage, release download URLs,
  autoupdate/checkver -> reyamira.
- cmd/doctor.go: runtime GitHubRepo update-check slug -> reyamira/pass-cli.
- cmd/root.go, cmd/keychain_status.go, internal/config/config.go: runtime
  help/doc URL strings -> reyamira (string literals/comments only).
- README, SECURITY, CONTRIBUTING, all docs/**, docsite/hugo.yaml, issue
  templates, docs-validation workflow, winget/brew/scoop manifests,
  .doc-manager: badges, Pages URLs, repo links -> reyamira.

Deliberately kept as arimxyer (identity / module path, not repo references):
- Go module path `github.com/arimxyer/pass-cli` in go.mod + docsite/go.mod, all
  .go import statements, and the .goreleaser.yml ldflags that inject into it.
- CHANGELOG history of the prior ari1110 -> arimxyer rename.
- winget Publisher / Author / Copyright / PackageIdentifier (arimxyer.pass-cli)
  and PublisherUrl (publisher's GitHub profile).
- issue-management.yml `assignees: ['arimxyer']` (GitHub username).

Supersedes #92 (the long-open sweep, now stale). Verified: go build, go vet,
valid .goreleaser.yml, residual-grep clean (only intentional keeps remain).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx

* docs: correct #104 changelog entry and a dead anchor (verification findings)

The blank-check verification caught two doc bugs shipped in #111:

- CHANGELOG #104 entry was inverted (the misleading squash subject "drop dead
  --hash, feedback, --offline" drove it). Verified against the #104 diff
  (67ca1d1): it removed the dead `rclone lsjson --hash` flag, ADDED pull-side
  sync feedback (not removed; still live at cmd/helpers.go:584), and ADDED a new
  global `--offline` flag. Corrected the entry and moved `--offline` to Added.
  Also added a [0.18.0] note for the arimxyer->reyamira install-source change.
- scripting-guide.md: fixed a dead `#configuration` anchor -> the configuration
  reference doc.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

Reduce (and surface) pre-unlock sync latency: SmartPull does a silent network round trip on every command

1 participant