diff --git a/.golangci.yml b/.golangci.yml index 34cc652..53868b5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -10,6 +10,7 @@ linters: - noctx - unparam - misspell + - depguard disable: - errcheck @@ -19,6 +20,15 @@ linters: disable: - shadow - fieldalignment + depguard: + rules: + logging-seam: + files: + - $all + - '!**/internal/log/**' + deny: + - pkg: go.uber.org/zap + desc: 'import internal/log instead of go.uber.org/zap directly' exclusions: rules: diff --git a/.goreleaser.yaml b/.goreleaser.yaml index ec0008f..ec49bcc 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -253,7 +253,7 @@ release: ## kcd v{{ .Version }} Headless KDE Connect daemon for Linux. - + footer: | ### Installation **Arch Linux:** @@ -289,5 +289,5 @@ release: curl -sSL https://github.com/bethropolis/kcd/releases/download/v{{ .Version }}/kcd_{{ .Version }}_x86_64.tar.gz | tar -xz sudo mv kcd /usr/local/bin/ ``` - footer: | + **Full Changelog:** https://github.com/bethropolis/kcd/compare/{{ .PreviousTag }}...v{{ .Version }} diff --git a/AGENTS.md b/AGENTS.md index 0e0ac4f..5f2d890 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,7 +24,7 @@ Never violate these. If a change would break one, stop and reconsider the approa 7. **deviceId is permanent.** Generated once via `config.EnsureDeviceID`, stored in `kcd.toml`. Never regenerate. It is the stable identity used for cert fingerprint pairing. 8. **Self-signed TLS, `InsecureSkipVerify: true`.** Authentication happens via the SHA-256 fingerprint stored in `devices.json` after pairing — not via CA chain. 9. **`Plugin.Handle()` must return immediately.** Any D-Bus call, subprocess (`exec.Command`), or disk I/O must be spawned in a goroutine *inside* the plugin. Blocking `Handle()` stalls the entire TCP read loop for that device. -10. **No deprecated packages.** No `ioutil` (use `os`/`io`). No `log` (use `go.uber.org/zap`). No `cobra` (use `urfave/cli/v2`). +10. **No deprecated packages.** No `ioutil` (use `os`/`io`). No `log` and no direct `go.uber.org/zap` imports (use `internal/log` — enforced by depguard). No `cobra` (use `urfave/cli/v2`). 11. **Keep `Body` as `json.RawMessage` in the router.** Plugins unmarshal their own body types. The packet router never touches body content. 12. **One goroutine per connection.** Dispatch is sequential per device — one packet handled at a time. This is intentional; it removes the need for per-plugin locks. 13. **Cap incoming payload size with `io.LimitReader`.** Never trust the `payloadSize` field from the remote device without a cap. @@ -39,7 +39,7 @@ These structural constraints must hold at all times: | Invariant | Why | |---|---| | `internal/protocol/` has **zero external imports** (stdlib only) | Protocol types are used everywhere; external deps would create cycles | -| `internal/config/` only imports `github.com/BurntSushi/toml` | Config must stay lean and cycle-free | +| `internal/config/` only imports `github.com/BurntSushi/toml` and `internal/protocol` | Config must stay lean and cycle-free (`protocol` is stdlib-only, so no cycle is possible) | | `internal/plugin/plugin.go` only imports `internal/protocol` and `internal/device` | Plugins never import each other | | Plugins are registered in `daemon.go`, never in their own `init()` | Explicit, ordered, conditional on config | | `pkg/client/` only imports `internal/device`, `internal/events`, `internal/ipc`, and `internal/plugins/contacts` from the `internal/` tree | Public client API must not depend on internals beyond the IPC protocol and the types it surfaces | @@ -67,7 +67,7 @@ These structural constraints must hold at all times: `daemon.Run()` wires everything in this exact sequence. Preserve the order when modifying startup: -1. Build `zap.Logger` from config log level +1. Build `log.Logger` via `log.New(cfg.LogLevel)` 2. Load or generate TLS certificate (`cert.LoadOrGenerate`) 3. Create event bus (`events.NewBus`) 4. Create device registry (`device.NewRegistry`) and load persisted state from `devices.json` @@ -86,7 +86,7 @@ These structural constraints must hold at all times: ``` ⚠ CONSTRUCTORS: Every plugin now requires bus *events.Bus and -logger *zap.Logger. Never use struct literals (&battery.BatteryPlugin{}) +logger log.Logger. Never use struct literals (&battery.BatteryPlugin{}) — always call the constructor. The compiler will catch this but the error message may be confusing. ``` @@ -95,16 +95,16 @@ error message may be confusing. | Plugin | Correct constructor signature | |---|---| -| Battery | `battery.NewBatteryPlugin(cfg config.BatteryConfig, bus *events.Bus, logger *zap.Logger) *BatteryPlugin` | -| Notification | `notification.NewNotificationPlugin(cfg config.NotificationPluginConfig, bus *events.Bus, tlsConfig *tls.Config, logger *zap.Logger) *NotificationPlugin` | -| Share | `share.NewSharePlugin(downloadDir string, cfg config.ShareConfig, tlsConfig *tls.Config, bus *events.Bus, logger *zap.Logger) *SharePlugin` | -| SFTP | `sftp.NewSftpPlugin(cfg config.SFTPConfig, bus *events.Bus, logger *zap.Logger) *SftpPlugin` | -| Ping | `ping.NewPingPlugin(cfg config.PingConfig, bus *events.Bus, logger *zap.Logger) *PingPlugin` | -| Pair | `pair.NewPairPlugin(devices *device.Registry, localCert *x509.Certificate, cfg config.PairingConfig, onStateChanged func(), bus *events.Bus, logger *zap.Logger) *PairPlugin` | -| Mousepad | `mousepad.NewMousepadPlugin(cfg config.MousepadConfig, logger *zap.Logger) *MousepadPlugin` | -| SystemVolume | `systemvolume.NewSystemVolumePlugin(bus *events.Bus, logger *zap.Logger) *SystemVolumePlugin` | -| SMS | `sms.NewSMSPlugin(cfg config.SMSConfig, bus *events.Bus, tlsConfig *tls.Config, logger *zap.Logger) *SMSPlugin` | -| Contacts | `contacts.NewContactsPlugin(bus *events.Bus, logger *zap.Logger) *ContactsPlugin` | +| Battery | `battery.NewBatteryPlugin(cfg config.BatteryConfig, bus *events.Bus, logger log.Logger) *BatteryPlugin` | +| Notification | `notification.NewNotificationPlugin(cfg config.NotificationPluginConfig, bus *events.Bus, tlsConfig *tls.Config, logger log.Logger) *NotificationPlugin` | +| Share | `share.NewSharePlugin(downloadDir string, cfg config.ShareConfig, tlsConfig *tls.Config, bus *events.Bus, logger log.Logger) *SharePlugin` | +| SFTP | `sftp.NewSftpPlugin(cfg config.SFTPConfig, bus *events.Bus, logger log.Logger) *SftpPlugin` | +| Ping | `ping.NewPingPlugin(cfg config.PingConfig, bus *events.Bus, logger log.Logger) *PingPlugin` | +| Pair | `pair.NewPairPlugin(devices *device.Registry, localCert *x509.Certificate, cfg config.PairingConfig, onStateChanged func(), bus *events.Bus, logger log.Logger) *PairPlugin` | +| Mousepad | `mousepad.NewMousepadPlugin(cfg config.MousepadConfig, logger log.Logger) *MousepadPlugin` | +| SystemVolume | `systemvolume.NewSystemVolumePlugin(bus *events.Bus, logger log.Logger) *SystemVolumePlugin` | +| SMS | `sms.NewSMSPlugin(cfg config.SMSConfig, bus *events.Bus, tlsConfig *tls.Config, logger log.Logger) *SMSPlugin` | +| Contacts | `contacts.NewContactsPlugin(bus *events.Bus, logger log.Logger) *ContactsPlugin` | ### Interface @@ -274,7 +274,7 @@ bus.Publish(events.TypeBatteryUpdate, dev.ID(), map[string]any{ ``` Rules: -- Subscriber channels have capacity 64. If a slow subscriber fills its channel, events are **dropped** (with a `zap.Warn`), never blocked. +- Subscriber channels have capacity 64. If a slow subscriber fills its channel, events are **dropped** (with a `log.Warn`), never blocked. - Filters: `bus.Subscribe(events.TypeBatteryUpdate, events.TypeNotification)` — empty filter = all events. - Always call `sub.Close()` when done to avoid goroutine leaks. diff --git a/README.md b/README.md index 5fa1c02..475891a 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,7 @@ | **Find My Phone** | Ring the phone to locate it | | **Telephony** | Get call and SMS notifications on the desktop | | **SMS** | Send SMS messages via the phone | +| **Contacts** | Sync and browse the phone address book | | **SFTP** | Browse the phone's filesystem | | **Lock / Unlock** | Lock and unlock the desktop session | | **Ping** | Simple connectivity check | @@ -47,7 +48,6 @@ Install from the AUR using your preferred helper: ```bash yay -S kcd-bin - systemctl --user enable --now kcd.socket ``` @@ -60,7 +60,7 @@ cd kcd ### Binary releases -Download the latest pre-built binary from [GitHub Releases](https://github.com/bethropolis/kcd/releases). +Every [GitHub Release](https://github.com/bethropolis/kcd/releases) ships pre-built artifacts for Linux (amd64, arm64, armv7): `.deb` (Debian/Ubuntu), `.rpm` (Fedora/RHEL), `.tar.gz` archives, and a Homebrew cask — see the release notes for per-format install commands. --- @@ -376,8 +376,10 @@ kcd connect 192.168.1.100 | Document | Description | |---|---| -| [`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md) | System architecture, plugin system, event bus, IPC protocol | +| [`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md) | System architecture, plugin system, event bus | | [`docs/CLI.md`](docs/CLI.md) | Full CLI reference and sub-commands | +| [`docs/IPC_PROTOCOL.md`](docs/IPC_PROTOCOL.md) | Daemon socket protocol: commands, events, packet reference | +| [`docs/CLIENT_GUIDE.md`](docs/CLIENT_GUIDE.md) | Building clients over the IPC socket (Python examples) | | [`docs/CONTAINER.md`](docs/CONTAINER.md) | Running kcd in Docker / Podman | | [`packaging/kcd.example.toml`](packaging/kcd.example.toml) | Annotated configuration reference | diff --git a/cmd/kcd/cli_contacts.go b/cmd/kcd/cli_contacts.go index 8d810b2..f5cda92 100644 --- a/cmd/kcd/cli_contacts.go +++ b/cmd/kcd/cli_contacts.go @@ -69,5 +69,29 @@ var contactsCmd = &cli.Command{ return nil }, }, + { + Name: "clear", + Usage: "Delete cached contacts for a device (re-sync restores them)", + ArgsUsage: "", + Action: func(c *cli.Context) error { + if c.NArg() < 1 { + return fmt.Errorf("missing device ID") + } + cl, err := getClient(c) + if err != nil { + return err + } + id := c.Args().Get(0) + list, err := cl.ContactsList(id) + if err != nil { + return err + } + if err := cl.ContactsClear(id); err != nil { + return err + } + fmt.Printf("Cleared %d cached contact(s) for %s.\n", len(list), id) + return nil + }, + }, }, } diff --git a/cmd/testid/main.go b/cmd/testid/main.go index 198659f..b75911e 100644 --- a/cmd/testid/main.go +++ b/cmd/testid/main.go @@ -13,7 +13,7 @@ func main() { "test_device_id_with_underscores", "TestDevice", "desktop", - 1716, + protocol.DefaultTCPPort, []string{"kdeconnect.ping"}, []string{"kdeconnect.ping"}, ) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 19026dd..22339f8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -188,6 +188,32 @@ type Plugin interface { Plugin execution is wrapped in a context with the plugin's declared `Timeout()` deadline. +### Subprocess policy + +All subprocess spawns go through `internal/plugin/exec.go` unless a site +needs something the seam cannot express: + +| Helper | Use when | +|---|---| +| `RunCommandAsync(logger, name, args...)` | Fire-and-forget, output only matters on failure (10s bound, warns once). The helper owns the goroutine — do not wrap it in another. | +| `RunCommandSync(ctx, name, args...)` | Caller needs the exit status or combined output; caller owns the timeout via ctx. | +| `RunCommandOutput(ctx, name, args...)` | Caller parses stdout; stderr is discarded so it cannot corrupt the parse. | + +Hand-rolled `exec.CommandContext` stays only where the seam is +inexpressive — do not "migrate" these without replacing the missing +capability: + +- **stdin piping**: share clipboard `wl-copy`/`xclip`, sftp `sshfs` (password on stdin). +- **`Start()`-without-`Wait` detach**: sftp auto-open of the mount (waiting would block on the file manager). +- **custom `Env` / `WaitDelay`**: clipboard runners (Wayland env injection, anti-`wl-copy`-fork hang, `/dev/null` fds so `Wait()` isn't pinned). +- **injectable constructor**: notification `newExec` field (tests stub `notify-send`). +- **stdout purity + stored IDs**: notification send/close (`--print-id` output becomes the next `-r` replace ID). +- **long-lived streaming child**: `wl-paste --watch` supervisor loop in the CLI. + +Every `exec.CommandContext` outside `internal/plugin/exec.go` must carry +an explicit timeout — `context.Background()` with no deadline is a +goroutine leak when the child wedges. + ### Implemented plugins | Package | Types handled | Notes | diff --git a/docs/CLI.md b/docs/CLI.md index 1db81cb..f3db347 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -876,6 +876,15 @@ unknown). kcd contacts list [--json] ``` +### contacts clear + +Delete a device's cached contacts. Works offline (the cache is local +state); re-sync restores everything from the phone. + +``` +kcd contacts clear +``` + --- ## volume diff --git a/docs/CLIENT_GUIDE.md b/docs/CLIENT_GUIDE.md index c2249cf..9f01f3c 100644 --- a/docs/CLIENT_GUIDE.md +++ b/docs/CLIENT_GUIDE.md @@ -384,6 +384,9 @@ if resp["ok"]: for c in resp["data"]: print(c["name"], c.get("phones", [])) # empty list = never synced (unknown, not zero contacts) + +# delete the cached address book (offline-capable; re-sync restores it) +ipc_request(sock, "contacts_clear", {"deviceId": dev_id}) ``` ### 5.7 Lock/Unlock diff --git a/docs/IPC_PROTOCOL.md b/docs/IPC_PROTOCOL.md index d92191d..ec4088b 100644 --- a/docs/IPC_PROTOCOL.md +++ b/docs/IPC_PROTOCOL.md @@ -390,6 +390,19 @@ Empty when never synced — absent means unknown. Example: [{"uid": "1", "name": "Ada Lovelace", "phones": ["+1-555-0100"], "timestamp": 973486597}] ``` +#### `contacts_clear` + +Delete a device's cached contacts. Offline-capable (the cache is local +state); re-sync restores everything from the phone. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** none. + #### `call_mute` Mute an incoming phone call. diff --git a/flake.nix b/flake.nix index ab1e40e..cb1b055 100644 --- a/flake.nix +++ b/flake.nix @@ -19,7 +19,7 @@ src = ./.; # Update when go.sum changes: nix build 2>&1 | grep 'got:' | awk '{print $2}' - vendorHash = "sha256-rnI60JzB8vtFC4iIVoHGi9um7f0mV7jbIJTNVu8ytVY="; + vendorHash = "sha256-6zwzWlboTQeZcBiiHU7Jt+vDn2FYCrQ8CzGgCntKRGo="; subPackages = [ "cmd/kcd" ]; diff --git a/internal/config/config.go b/internal/config/config.go index 42212fb..dcc3641 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,5 +1,6 @@ // Package config handles loading and validating the kcd daemon configuration. -// It has zero external imports except github.com/BurntSushi/toml. +// It imports only github.com/BurntSushi/toml and internal/protocol +// (stdlib-only, so no import cycle is possible). package config import ( @@ -8,6 +9,7 @@ import ( "path/filepath" "github.com/BurntSushi/toml" + "github.com/bethropolis/kcd/internal/protocol" ) // Config holds all daemon configuration. @@ -63,7 +65,7 @@ func Defaults() *Config { c.KeyFile = configPath("key.pem", false) c.SocketPath = DefaultSocketPath() c.DownloadDir = filepath.Join(home, "Downloads", "kcd") - c.TCPPort = 1716 + c.TCPPort = protocol.DefaultTCPPort c.LogLevel = "info" c.Network = NetworkConfig{DialTimeout: "5s", HandshakeTimeout: "10s", SidechannelTimeout: "15s", TransferIdleTimeout: "60s"} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 3139633..6ea8e03 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/BurntSushi/toml" + "github.com/bethropolis/kcd/internal/protocol" ) func TestDefaults(t *testing.T) { @@ -28,6 +29,14 @@ func TestDefaults(t *testing.T) { if cfg.Pairing.IntentTTL != "5m" || cfg.Pairing.ListenTimeout != "60s" || cfg.Pairing.TimeoutSecs != 30 { t.Errorf("pairing defaults: %+v", cfg.Pairing) } + if cfg.TCPPort != protocol.DefaultTCPPort { + t.Errorf("tcp_port default = %d, want protocol.DefaultTCPPort (%d)", cfg.TCPPort, protocol.DefaultTCPPort) + } + if cfg.Share.PortMin != protocol.DefaultSidechannelPortMin || cfg.Share.PortMax != protocol.DefaultSidechannelPortMax { + t.Errorf("share port range default = %d-%d, want %d-%d", + cfg.Share.PortMin, cfg.Share.PortMax, + protocol.DefaultSidechannelPortMin, protocol.DefaultSidechannelPortMax) + } if cfg.Cache != (CacheConfig{}) || cfg.Ping.AppName != "" || cfg.Notifications.AppName() != "KDE Connect" { t.Fatal("default cache paths or notification inheritance changed") } diff --git a/internal/config/plugins.go b/internal/config/plugins.go index 71cc8cc..4f87180 100644 --- a/internal/config/plugins.go +++ b/internal/config/plugins.go @@ -3,6 +3,8 @@ package config import ( "os" "path/filepath" + + "github.com/bethropolis/kcd/internal/protocol" ) type PluginConfig struct { @@ -150,8 +152,8 @@ func (c *ClipboardConfig) Defaults() { } func (c *ShareConfig) Defaults() { - c.PortMin = 1739 - c.PortMax = 1764 + c.PortMin = protocol.DefaultSidechannelPortMin + c.PortMax = protocol.DefaultSidechannelPortMax c.AcceptTimeoutSecs = 120 c.OpenCommand = "xdg-open" } diff --git a/internal/daemon/configurability_test.go b/internal/daemon/configurability_test.go index 75da598..66d817f 100644 --- a/internal/daemon/configurability_test.go +++ b/internal/daemon/configurability_test.go @@ -5,15 +5,15 @@ import ( "encoding/json" "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" "net" "testing" ) func TestReconnectConfiguredPort(t *testing.T) { - dev := device.NewDevice("peer", "Peer", "phone", zap.NewNop()) + dev := device.NewDevice("peer", "Peer", "phone", log.Nop()) if got := reconnectPort(dev, 1816); got != 1816 { t.Fatalf("fallback = %d", got) } @@ -41,6 +41,6 @@ func TestDialConfiguredPortCanceled(t *testing.T) { } ctx, cancel := context.WithCancel(context.Background()) cancel() - logger := zap.NewNop() + logger := log.Nop() DialDevice(ctx, net.IPv4(127, 0, 0, 1), cfg.TCPPort, "peer", protocol.ProtocolVersion, pkt, nil, device.NewRegistry(nil), plugin.NewRegistry(logger), "local", logger, true, cfg) } diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 234c64b..a895822 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -17,12 +17,11 @@ import ( "github.com/bethropolis/kcd/internal/discovery" "github.com/bethropolis/kcd/internal/events" "github.com/bethropolis/kcd/internal/ipc" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/plugins/notification" "github.com/bethropolis/kcd/internal/plugins/runcommand" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" ) // Version is set from main's ldflags-provided version at startup. @@ -46,36 +45,16 @@ func syncReconnectBroadcast(ctx context.Context, devices *device.Registry, bc *d func Run(ctx context.Context, cfg *config.Config) error { startedAt := time.Now() - atomicLevel := zap.NewAtomicLevel() - switch cfg.LogLevel { - case "debug": - atomicLevel.SetLevel(zapcore.DebugLevel) - case "warn": - atomicLevel.SetLevel(zapcore.WarnLevel) - case "error", "quiet": - atomicLevel.SetLevel(zapcore.ErrorLevel) - default: - atomicLevel.SetLevel(zapcore.InfoLevel) - } - - var zapCfg zap.Config - if cfg.LogLevel == "debug" { - zapCfg = zap.NewDevelopmentConfig() - } else { - zapCfg = zap.NewProductionConfig() - } - zapCfg.Level = atomicLevel - - logger, err := zapCfg.Build() + logger, err := log.New(cfg.LogLevel) if err != nil { return err } defer logger.Sync() //nolint:errcheck - logger.Info("kcd daemon initializing", zap.String("device_id", cfg.DeviceID)) + logger.Info("kcd daemon initializing", log.String("device_id", cfg.DeviceID)) if err := cfg.Validate(); err != nil { - logger.Fatal("invalid configuration", zap.Error(err)) + logger.Fatal("invalid configuration", log.Error(err)) } // 1. TLS Certs (CN must match device ID for KDE Connect verification) @@ -114,12 +93,12 @@ func Run(ctx context.Context, cfg *config.Config) error { devices.Add(dev) } } else { - logger.Warn("failed to load devices", zap.Error(err)) + logger.Warn("failed to load devices", log.Error(err)) } pruneThreshold, _ := time.ParseDuration(cfg.PruneStaleThreshold) if pruned := devices.Prune(pruneThreshold); pruned > 0 { - logger.Info("pruned stale devices on startup", zap.Int("count", pruned)) + logger.Info("pruned stale devices on startup", log.Int("count", pruned)) } // Helper to save device state @@ -164,7 +143,7 @@ func Run(ctx context.Context, cfg *config.Config) error { return err } - bc := discovery.NewBroadcasterController(identity, config.Duration(cfg.Discovery.BroadcastInterval), logger, devices.AllPairedDevicesConnected, config.Duration(cfg.Discovery.BroadcastIdleInterval)) + bc := discovery.NewBroadcasterController(identity, cfg.TCPPort, config.Duration(cfg.Discovery.BroadcastInterval), logger, devices.AllPairedDevicesConnected, config.Duration(cfg.Discovery.BroadcastIdleInterval)) // mDNS advertisement is always on: unlike UDP broadcast it is // responder-only (zero idle timers), so phones keep a standing @@ -213,12 +192,12 @@ func Run(ctx context.Context, cfg *config.Config) error { return fmt.Errorf("device address unknown yet, wait for discovery and retry") } if port == 0 { - port = 1716 + port = cfg.TCPPort } go func() { DialDevice(ctx, ip, port, deviceID, protocol.ProtocolVersion, identity, tlsCfg, devices, plugins, cfg.DeviceID, logger, true, cfg) if !dev.IsConnected() { - logger.Warn("on-demand pair dial failed", zap.String("device_id", deviceID)) + logger.Warn("on-demand pair dial failed", log.String("device_id", deviceID)) return } // The immediate dial succeeded: consume the one-shot trigger so @@ -229,11 +208,11 @@ func Run(ctx context.Context, cfg *config.Config) error { dev.ConsumePairDial() if dev.State() == device.StatePairRequestedByPeer { if err := pairPlugin.AcceptPairing(dev); err != nil { - logger.Warn("auto-accept on pair dial failed", zap.Error(err)) + logger.Warn("auto-accept on pair dial failed", log.Error(err)) } } else if dev.State() != device.StatePaired { if err := pairPlugin.RequestPairing(dev); err != nil { - logger.Warn("pair request on dial failed", zap.Error(err)) + logger.Warn("pair request on dial failed", log.Error(err)) } } }() @@ -245,7 +224,7 @@ func Run(ctx context.Context, cfg *config.Config) error { // Start IPC in background go func() { if err := ipcServer.Listen(ctx); err != nil { - logger.Error("ipc server error", zap.Error(err)) + logger.Error("ipc server error", log.Error(err)) } }() @@ -276,7 +255,7 @@ func Run(ctx context.Context, cfg *config.Config) error { logger.Info("SIGHUP received, reloading config") newCfg, err := config.Load(cfg.ConfigPath) if err != nil { - logger.Error("config reload failed", zap.Error(err)) + logger.Error("config reload failed", log.Error(err)) continue } @@ -288,8 +267,8 @@ func Run(ctx context.Context, cfg *config.Config) error { rc.CommandsPerDevice = newCfg.CommandsPerDevice rc.Mu.Unlock() logger.Info("reloaded commands", - zap.Int("global", len(newCfg.Commands)), - zap.Int("per_device", len(newCfg.CommandsPerDevice)), + log.Int("global", len(newCfg.Commands)), + log.Int("per_device", len(newCfg.CommandsPerDevice)), ) } @@ -300,7 +279,7 @@ func Run(ctx context.Context, cfg *config.Config) error { } // Reload log level. - setLogLevel(atomicLevel, newCfg.LogLevel) + logger.SetLevel(newCfg.LogLevel) } } }() @@ -312,27 +291,13 @@ func Run(ctx context.Context, cfg *config.Config) error { acquires, misses := protocol.PoolStats() hits := acquires - misses logger.Debug("packet pool stats", - zap.Int64("acquires", acquires), - zap.Int64("misses", misses), - zap.Int64("hits", hits), - zap.String("hit_rate", fmt.Sprintf("%.1f%%", + log.Int64("acquires", acquires), + log.Int64("misses", misses), + log.Int64("hits", hits), + log.String("hit_rate", fmt.Sprintf("%.1f%%", float64(hits)/max(float64(acquires), 1)*100, )), ) return nil } - -// setLogLevel updates the atomic log level without restarting the daemon. -func setLogLevel(al zap.AtomicLevel, level string) { - switch level { - case "debug": - al.SetLevel(zapcore.DebugLevel) - case "warn": - al.SetLevel(zapcore.WarnLevel) - case "error", "quiet": - al.SetLevel(zapcore.ErrorLevel) - default: - al.SetLevel(zapcore.InfoLevel) - } -} diff --git a/internal/daemon/ipc_route_helpers.go b/internal/daemon/ipc_route_helpers.go new file mode 100644 index 0000000..91acc51 --- /dev/null +++ b/internal/daemon/ipc_route_helpers.go @@ -0,0 +1,59 @@ +package daemon + +import ( + "encoding/json" + "strings" + + "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/ipc" + "github.com/bethropolis/kcd/internal/plugin" +) + +// hasDeviceID is implemented by IPC payloads that address a device. +// The methods live on the ipc payload types (see ipc/payload.go). +type hasDeviceID interface { + GetDeviceID() string +} + +// deviceRoute handles the standard decode → plugin → device preamble +// shared by most IPC handlers. An empty pluginName skips the plugin +// lookup (pl is nil) for device-only routes. Error strings match +// docs/IPC_PROTOCOL.md byte-for-byte; the disabled-plugin message is +// derived from the registry name, which keeps every route consistent. +func deviceRoute[P hasDeviceID](req ipc.Request, payload P, devices *device.Registry, plugins *plugin.Registry, pluginName string, action func(dev *device.Device, pl plugin.Plugin) ipc.Response) ipc.Response { + if err := json.Unmarshal(req.Payload, payload); err != nil { + return ipc.Response{OK: false, Error: "invalid payload"} + } + var pl plugin.Plugin + if pluginName != "" { + var ok bool + if pl, ok = plugins.GetByName(pluginName); !ok { + return ipc.Response{OK: false, Error: strings.ToLower(pluginName) + " plugin not enabled"} + } + } + dev, ok := devices.Get(payload.GetDeviceID()) + if !ok { + return ipc.Response{OK: false, Error: "device not found"} + } + return action(dev, pl) +} + +// pluginRoute handles the decode → plugin preamble for routes that do +// not address a single device (listings, global status). +func pluginRoute[P any](req ipc.Request, payload P, plugins *plugin.Registry, pluginName string, action func(pl plugin.Plugin) ipc.Response) ipc.Response { + if err := json.Unmarshal(req.Payload, payload); err != nil { + return ipc.Response{OK: false, Error: "invalid payload"} + } + pl, ok := plugins.GetByName(pluginName) + if !ok { + return ipc.Response{OK: false, Error: strings.ToLower(pluginName) + " plugin not enabled"} + } + return action(pl) +} + +// jsonOK marshals v into a successful IPC response. Marshal failures +// are ignored, matching the previous inline behavior at every site. +func jsonOK(v any) ipc.Response { + data, _ := json.Marshal(v) + return ipc.Response{OK: true, Data: data} +} diff --git a/internal/daemon/ipc_route_helpers_test.go b/internal/daemon/ipc_route_helpers_test.go new file mode 100644 index 0000000..2e0da0a --- /dev/null +++ b/internal/daemon/ipc_route_helpers_test.go @@ -0,0 +1,109 @@ +package daemon + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/ipc" + "github.com/bethropolis/kcd/internal/log" + "github.com/bethropolis/kcd/internal/plugin" + "github.com/bethropolis/kcd/internal/protocol" +) + +type stubRoutePlugin struct{ name string } + +func (s *stubRoutePlugin) Name() string { return s.name } +func (s *stubRoutePlugin) IncomingTypes() []string { return nil } +func (s *stubRoutePlugin) OutgoingTypes() []string { return nil } +func (s *stubRoutePlugin) Timeout() time.Duration { return 0 } +func (s *stubRoutePlugin) OnConnect(device.Sender) {} +func (s *stubRoutePlugin) OnDisconnect(device.Sender) {} +func (s *stubRoutePlugin) Handle(context.Context, device.Sender, *protocol.Packet) error { + return nil +} + +func routeTestRegistries(t *testing.T) (*device.Registry, *plugin.Registry) { + t.Helper() + bus := events.NewBus(log.Nop()) + devices := device.NewRegistry(bus) + devices.Add(device.NewDevice("dev1", "Test", "phone", log.Nop())) + plugins := plugin.NewRegistry(log.Nop()) + plugins.Register(&stubRoutePlugin{name: "SMS"}) + return devices, plugins +} + +func TestDeviceRoutePreamble(t *testing.T) { + devices, plugins := routeTestRegistries(t) + okAction := func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if dev.ID() != "dev1" { + t.Errorf("action got device %q, want dev1", dev.ID()) + } + if pl == nil || pl.Name() != "SMS" { + t.Errorf("action got plugin %v, want SMS", pl) + } + return ipc.Response{OK: true} + } + + payload := json.RawMessage(`{"deviceId":"dev1"}`) + if resp := deviceRoute(ipc.Request{Payload: payload}, &ipc.DevicePayload{}, devices, plugins, "SMS", okAction); !resp.OK { + t.Errorf("happy path failed: %q", resp.Error) + } + + if resp := deviceRoute(ipc.Request{Payload: json.RawMessage(`{bad`)}, &ipc.DevicePayload{}, devices, plugins, "SMS", okAction); resp.Error != "invalid payload" { + t.Errorf("malformed payload error = %q, want invalid payload", resp.Error) + } + + if resp := deviceRoute(ipc.Request{Payload: payload}, &ipc.DevicePayload{}, devices, plugins, "Missing", okAction); resp.Error != "missing plugin not enabled" { + t.Errorf("unknown plugin error = %q, want missing plugin not enabled", resp.Error) + } + + missing := json.RawMessage(`{"deviceId":"nope"}`) + if resp := deviceRoute(ipc.Request{Payload: missing}, &ipc.DevicePayload{}, devices, plugins, "SMS", okAction); resp.Error != "device not found" { + t.Errorf("unknown device error = %q, want device not found", resp.Error) + } + + // Empty plugin name skips the lookup for device-only routes. + seenNil := false + if resp := deviceRoute(ipc.Request{Payload: payload}, &ipc.DevicePayload{}, devices, plugins, "", func(_ *device.Device, pl plugin.Plugin) ipc.Response { + seenNil = pl == nil + return ipc.Response{OK: true} + }); !resp.OK || !seenNil { + t.Errorf("empty plugin name: ok=%v seenNil=%v, want ok with nil plugin", resp.OK, seenNil) + } +} + +func TestPluginRoutePreamble(t *testing.T) { + _, plugins := routeTestRegistries(t) + payload := json.RawMessage(`{"deviceId":"dev1"}`) + okAction := func(pl plugin.Plugin) ipc.Response { + if pl.Name() != "SMS" { + t.Errorf("action got plugin %v, want SMS", pl) + } + return ipc.Response{OK: true} + } + + if resp := pluginRoute(ipc.Request{Payload: payload}, &ipc.DevicePayload{}, plugins, "SMS", okAction); !resp.OK { + t.Errorf("happy path failed: %q", resp.Error) + } + if resp := pluginRoute(ipc.Request{Payload: json.RawMessage(`{bad`)}, &ipc.DevicePayload{}, plugins, "SMS", okAction); resp.Error != "invalid payload" { + t.Errorf("malformed payload error = %q, want invalid payload", resp.Error) + } + if resp := pluginRoute(ipc.Request{Payload: payload}, &ipc.DevicePayload{}, plugins, "Missing", okAction); resp.Error != "missing plugin not enabled" { + t.Errorf("unknown plugin error = %q, want missing plugin not enabled", resp.Error) + } +} + +func TestJsonOK(t *testing.T) { + resp := jsonOK(map[string]int{"charge": 85}) + if !resp.OK { + t.Fatal("jsonOK must succeed") + } + var got map[string]int + if err := json.Unmarshal(resp.Data, &got); err != nil || got["charge"] != 85 { + t.Errorf("jsonOK round-trip = %v, %v; want charge 85", got, err) + } +} diff --git a/internal/daemon/ipc_routes.go b/internal/daemon/ipc_routes.go index 5b6632e..beb18e6 100644 --- a/internal/daemon/ipc_routes.go +++ b/internal/daemon/ipc_routes.go @@ -12,14 +12,14 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/discovery" "github.com/bethropolis/kcd/internal/ipc" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/plugins/mpris" "github.com/bethropolis/kcd/internal/plugins/notification" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) -func registerIPCRoutes(handler *ipc.Handler, cfg *config.Config, devices *device.Registry, plugins *plugin.Registry, bc *discovery.BroadcasterController, ctx context.Context, tlsCfg *tls.Config, logger *zap.Logger, startedAt time.Time) { +func registerIPCRoutes(handler *ipc.Handler, cfg *config.Config, devices *device.Registry, plugins *plugin.Registry, bc *discovery.BroadcasterController, ctx context.Context, tlsCfg *tls.Config, logger log.Logger, startedAt time.Time) { if cfg.Plugins.Notification { if notifPl, ok := plugins.GetByName("Notification"); ok { notifPl.(*notification.NotificationPlugin).SetFilters(cfg.Notifications.Filters()) @@ -27,7 +27,7 @@ func registerIPCRoutes(handler *ipc.Handler, cfg *config.Config, devices *device } if cfg.Plugins.Battery { - registerBatteryRoutes(handler, devices) + registerBatteryRoutes(handler, devices, plugins) } if cfg.Plugins.Connectivity { registerConnectivityRoutes(handler, devices, plugins) @@ -39,7 +39,7 @@ func registerIPCRoutes(handler *ipc.Handler, cfg *config.Config, devices *device registerContactsRoutes(handler, devices, plugins) } if cfg.Plugins.RunCommand { - registerRunCommandRoutes(handler, devices) + registerRunCommandRoutes(handler, devices, plugins) } if cfg.Plugins.Share { registerShareRoutes(handler, devices, plugins) @@ -96,7 +96,7 @@ func registerIPCRoutes(handler *ipc.Handler, cfg *config.Config, devices *device continue } logger.Debug("broadcast stopped, dropping unpaired discovery connection", - zap.String("device_id", dev.ID())) + log.String("device_id", dev.ID())) dev.Disconnect() } return ipc.Response{OK: true} @@ -171,7 +171,6 @@ func registerIPCRoutes(handler *ipc.Handler, cfg *config.Config, devices *device return ipc.Response{OK: false, Error: "mpris plugin not enabled"} } status := pl.(*mpris.MPRISPlugin).DebugStatus() - data, _ := json.Marshal(status) - return ipc.Response{OK: true, Data: data} + return jsonOK(status) }) } diff --git a/internal/daemon/ipc_routes_battery.go b/internal/daemon/ipc_routes_battery.go index dab0fc1..09613c7 100644 --- a/internal/daemon/ipc_routes_battery.go +++ b/internal/daemon/ipc_routes_battery.go @@ -1,33 +1,26 @@ package daemon import ( - "encoding/json" - "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/ipc" + "github.com/bethropolis/kcd/internal/plugin" ) -func registerBatteryRoutes(handler *ipc.Handler, devices *device.Registry) { +func registerBatteryRoutes(handler *ipc.Handler, devices *device.Registry, plugins *plugin.Registry) { handler.Register(ipc.CmdBattery, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - // Fail closed when no packet was ever received: returning the - // zero values would be indistinguishable from a real 0% reading. - if !dev.HasBattery() { - return ipc.Response{OK: false, Error: "no battery reading yet"} - } - charge, charging := dev.GetBattery() - data, _ := json.Marshal(map[string]interface{}{ - "charge": charge, - "charging": charging, - "batteryAgeMs": dev.BatteryAge().Milliseconds(), + return deviceRoute(req, &p, devices, plugins, "", func(dev *device.Device, _ plugin.Plugin) ipc.Response { + // Fail closed when no packet was ever received: returning the + // zero values would be indistinguishable from a real 0% reading. + if !dev.HasBattery() { + return ipc.Response{OK: false, Error: "no battery reading yet"} + } + charge, charging := dev.GetBattery() + return jsonOK(map[string]interface{}{ + "charge": charge, + "charging": charging, + "batteryAgeMs": dev.BatteryAge().Milliseconds(), + }) }) - return ipc.Response{OK: true, Data: data} }) } diff --git a/internal/daemon/ipc_routes_clipboard.go b/internal/daemon/ipc_routes_clipboard.go index bf5f9a8..afc76ac 100644 --- a/internal/daemon/ipc_routes_clipboard.go +++ b/internal/daemon/ipc_routes_clipboard.go @@ -2,7 +2,6 @@ package daemon import ( "context" - "encoding/json" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/ipc" @@ -13,20 +12,11 @@ import ( func registerClipboardRoutes(handler *ipc.Handler, devices *device.Registry, plugins *plugin.Registry) { handler.Register(ipc.CmdClipboardPush, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - pl, ok := plugins.GetByName("Clipboard") - if !ok { - return ipc.Response{OK: false, Error: "clipboard plugin not enabled"} - } - if err := clipboard.Push(context.Background(), dev, pl.(*clipboard.ClipboardPlugin)); err != nil { - return ipc.Response{OK: false, Error: "clipboard push failed: " + err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "Clipboard", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := clipboard.Push(context.Background(), dev, pl.(*clipboard.ClipboardPlugin)); err != nil { + return ipc.Response{OK: false, Error: "clipboard push failed: " + err.Error()} + } + return ipc.Response{OK: true} + }) }) } diff --git a/internal/daemon/ipc_routes_comm.go b/internal/daemon/ipc_routes_comm.go index 83f4a79..d9481ab 100644 --- a/internal/daemon/ipc_routes_comm.go +++ b/internal/daemon/ipc_routes_comm.go @@ -1,8 +1,6 @@ package daemon import ( - "encoding/json" - "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/ipc" @@ -16,136 +14,73 @@ func registerCommRoutes(handler *ipc.Handler, cfg *config.Config, devices *devic if cfg.Plugins.SMS { handler.Register(ipc.CmdSendSMS, func(req ipc.Request) ipc.Response { var p ipc.SMSPayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("SMS") - if !ok { - return ipc.Response{OK: false, Error: "sms plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*sms.SMSPlugin).SendSMS(dev, p.PhoneNumber, p.Message); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "SMS", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*sms.SMSPlugin).SendSMS(dev, p.PhoneNumber, p.Message); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) handler.Register(ipc.CmdSmsRequestConvs, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("SMS") - if !ok { - return ipc.Response{OK: false, Error: "sms plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*sms.SMSPlugin).RequestConversations(dev); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "SMS", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*sms.SMSPlugin).RequestConversations(dev); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) handler.Register(ipc.CmdSmsRequestConv, func(req ipc.Request) ipc.Response { var p ipc.SMSConvPayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("SMS") - if !ok { - return ipc.Response{OK: false, Error: "sms plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*sms.SMSPlugin).RequestConversation(dev, p.ThreadID, p.RangeStartTimestamp, p.NumberToRequest); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "SMS", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*sms.SMSPlugin).RequestConversation(dev, p.ThreadID, p.RangeStartTimestamp, p.NumberToRequest); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) handler.Register(ipc.CmdSmsRequestAttachment, func(req ipc.Request) ipc.Response { var p ipc.SMSAttachmentPayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("SMS") - if !ok { - return ipc.Response{OK: false, Error: "sms plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*sms.SMSPlugin).RequestAttachment(dev, p.PartID, p.UniqueIdentifier); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "SMS", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*sms.SMSPlugin).RequestAttachment(dev, p.PartID, p.UniqueIdentifier); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) } if cfg.Plugins.Telephony { handler.Register(ipc.CmdCallMute, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("Telephony") - if !ok { - return ipc.Response{OK: false, Error: "telephony plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*telephony.TelephonyPlugin).Mute(dev); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "Telephony", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*telephony.TelephonyPlugin).Mute(dev); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) } if cfg.Plugins.Notification { handler.Register(ipc.CmdNotifyReply, func(req ipc.Request) ipc.Response { var p ipc.NotifyReplyPayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("Notification") - if !ok { - return ipc.Response{OK: false, Error: "notification plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*notification.NotificationPlugin).RequestReply(dev, p.ReplyID, p.Message); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "Notification", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*notification.NotificationPlugin).RequestReply(dev, p.ReplyID, p.Message); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) handler.Register(ipc.CmdNotifyDismiss, func(req ipc.Request) ipc.Response { var p ipc.NotifyDismissPayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("Notification") - if !ok { - return ipc.Response{OK: false, Error: "notification plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*notification.NotificationPlugin).Dismiss(dev, p.NotificationID); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "Notification", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*notification.NotificationPlugin).Dismiss(dev, p.NotificationID); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) } } diff --git a/internal/daemon/ipc_routes_connectivity.go b/internal/daemon/ipc_routes_connectivity.go index ccfde31..b4afdd9 100644 --- a/internal/daemon/ipc_routes_connectivity.go +++ b/internal/daemon/ipc_routes_connectivity.go @@ -1,8 +1,6 @@ package daemon import ( - "encoding/json" - "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/ipc" "github.com/bethropolis/kcd/internal/plugin" @@ -12,22 +10,12 @@ import ( func registerConnectivityRoutes(handler *ipc.Handler, devices *device.Registry, plugins *plugin.Registry) { handler.Register(ipc.CmdConnectivity, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - pl, ok := plugins.GetByName("Connectivity") - if !ok { - return ipc.Response{OK: false, Error: "connectivity plugin not enabled"} - } - report, ok := pl.(*connectivity.ConnectivityPlugin).Report(dev.ID()) - if !ok { - return ipc.Response{OK: false, Error: "no connectivity data (device offline or never reported)"} - } - data, _ := json.Marshal(report) - return ipc.Response{OK: true, Data: data} + return deviceRoute(req, &p, devices, plugins, "Connectivity", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + report, ok := pl.(*connectivity.ConnectivityPlugin).Report(dev.ID()) + if !ok { + return ipc.Response{OK: false, Error: "no connectivity data (device offline or never reported)"} + } + return jsonOK(report) + }) }) } diff --git a/internal/daemon/ipc_routes_contacts.go b/internal/daemon/ipc_routes_contacts.go index 0834aba..8be65cb 100644 --- a/internal/daemon/ipc_routes_contacts.go +++ b/internal/daemon/ipc_routes_contacts.go @@ -1,8 +1,6 @@ package daemon import ( - "encoding/json" - "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/ipc" "github.com/bethropolis/kcd/internal/plugin" @@ -12,40 +10,35 @@ import ( func registerContactsRoutes(handler *ipc.Handler, devices *device.Registry, plugins *plugin.Registry) { handler.Register(ipc.CmdContactsSync, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("Contacts") - if !ok { - return ipc.Response{OK: false, Error: "contacts plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if !dev.IsConnected() { - return ipc.Response{OK: false, Error: "device not connected"} - } - if err := pl.(*contacts.ContactsPlugin).RequestSync(dev); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "Contacts", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if !dev.IsConnected() { + return ipc.Response{OK: false, Error: "device not connected"} + } + if err := pl.(*contacts.ContactsPlugin).RequestSync(dev); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) handler.Register(ipc.CmdContactsList, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("Contacts") - if !ok { - return ipc.Response{OK: false, Error: "contacts plugin not enabled"} - } - list := pl.(*contacts.ContactsPlugin).List(p.DeviceID) - if list == nil { - list = []contacts.ContactSummary{} - } - data, _ := json.Marshal(list) - return ipc.Response{OK: true, Data: data} + return pluginRoute(req, &p, plugins, "Contacts", func(pl plugin.Plugin) ipc.Response { + list := pl.(*contacts.ContactsPlugin).List(p.DeviceID) + if list == nil { + list = []contacts.ContactSummary{} + } + return jsonOK(list) + }) + }) + + handler.Register(ipc.CmdContactsClear, func(req ipc.Request) ipc.Response { + var p ipc.DevicePayload + return pluginRoute(req, &p, plugins, "Contacts", func(pl plugin.Plugin) ipc.Response { + if err := pl.(*contacts.ContactsPlugin).ForgetDevice(p.DeviceID); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) } diff --git a/internal/daemon/ipc_routes_contacts_test.go b/internal/daemon/ipc_routes_contacts_test.go new file mode 100644 index 0000000..8688283 --- /dev/null +++ b/internal/daemon/ipc_routes_contacts_test.go @@ -0,0 +1,59 @@ +package daemon + +import ( + "encoding/json" + "testing" + + "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/ipc" + "github.com/bethropolis/kcd/internal/log" + "github.com/bethropolis/kcd/internal/plugin" + "github.com/bethropolis/kcd/internal/plugins/contacts" +) + +func contactsTestHandler(t *testing.T, withPlugin bool) *ipc.Handler { + t.Helper() + t.Setenv("XDG_DATA_HOME", t.TempDir()) + bus := events.NewBus(log.Nop()) + devices := device.NewRegistry(bus) + plugins := plugin.NewRegistry(log.Nop()) + if withPlugin { + plugins.Register(contacts.NewContactsPlugin(nil, log.Nop())) + } + h := ipc.NewHandler(devices, plugins, nil, "", bus, 0) + registerContactsRoutes(h, devices, plugins) + return h +} + +func contactsRequest(cmd, deviceID string) ipc.Request { + payload, _ := json.Marshal(ipc.DevicePayload{DeviceID: deviceID}) + return ipc.Request{Command: cmd, Payload: payload} +} + +// Clear is idempotent and offline-capable: unknown device, no connection. +func TestContactsClearRoute(t *testing.T) { + h := contactsTestHandler(t, true) + resp := h.HandleRequest(contactsRequest(ipc.CmdContactsClear, "ghost")) + if !resp.OK { + t.Fatalf("clear unknown device failed: %q", resp.Error) + } +} + +// Disabled plugin surfaces the registry-derived error. +func TestContactsClearPluginDisabled(t *testing.T) { + h := contactsTestHandler(t, false) + resp := h.HandleRequest(contactsRequest(ipc.CmdContactsClear, "ghost")) + if resp.Error != "contacts plugin not enabled" { + t.Fatalf("error = %q, want contacts plugin not enabled", resp.Error) + } +} + +// Malformed payload is rejected before touching the plugin. +func TestContactsClearInvalidPayload(t *testing.T) { + h := contactsTestHandler(t, true) + resp := h.HandleRequest(ipc.Request{Command: ipc.CmdContactsClear, Payload: json.RawMessage(`{bad`)}) + if resp.Error != "invalid payload" { + t.Fatalf("error = %q, want invalid payload", resp.Error) + } +} diff --git a/internal/daemon/ipc_routes_device.go b/internal/daemon/ipc_routes_device.go index cc8505b..21774f9 100644 --- a/internal/daemon/ipc_routes_device.go +++ b/internal/daemon/ipc_routes_device.go @@ -1,8 +1,6 @@ package daemon import ( - "encoding/json" - "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/ipc" @@ -15,59 +13,32 @@ func registerDeviceRoutes(handler *ipc.Handler, cfg *config.Config, devices *dev if cfg.Plugins.FindMyPhone { handler.Register(ipc.CmdFindMyPhone, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("FindMyPhone") - if !ok { - return ipc.Response{OK: false, Error: "findmyphone plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*findmyphone.FindMyPhonePlugin).Ring(dev); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "FindMyPhone", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*findmyphone.FindMyPhonePlugin).Ring(dev); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) } if cfg.Plugins.LockDevice { handler.Register(ipc.CmdLock, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("LockDevice") - if !ok { - return ipc.Response{OK: false, Error: "lockdevice plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*lockdevice.LockDevicePlugin).Lock(dev); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "LockDevice", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*lockdevice.LockDevicePlugin).Lock(dev); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) handler.Register(ipc.CmdUnlock, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("LockDevice") - if !ok { - return ipc.Response{OK: false, Error: "lockdevice plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*lockdevice.LockDevicePlugin).Unlock(dev); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "LockDevice", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*lockdevice.LockDevicePlugin).Unlock(dev); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) } } diff --git a/internal/daemon/ipc_routes_mpris.go b/internal/daemon/ipc_routes_mpris.go index ee360e7..eb05697 100644 --- a/internal/daemon/ipc_routes_mpris.go +++ b/internal/daemon/ipc_routes_mpris.go @@ -1,7 +1,6 @@ package daemon import ( - "encoding/json" "fmt" "github.com/bethropolis/kcd/internal/device" @@ -13,60 +12,54 @@ import ( func registerMprisRoutes(handler *ipc.Handler, devices *device.Registry, plugins *plugin.Registry) { handler.Register(ipc.CmdMprisAction, func(req ipc.Request) ipc.Response { var p ipc.MprisActionPayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - - pl, ok := plugins.GetByName("MPRIS") - if !ok { - return ipc.Response{OK: false, Error: "mpris plugin not enabled"} - } - mprisPl := pl.(*mpris.MPRISPlugin) + return pluginRoute(req, &p, plugins, "MPRIS", func(pl plugin.Plugin) ipc.Response { + mprisPl := pl.(*mpris.MPRISPlugin) - targetDeviceID := p.DeviceID - if targetDeviceID == "" { - // Pick the first connected device that has cached MPRIS state, - // falling back to any connected device. - devs := devices.List() - for _, d := range devs { - if d.IsConnected() && mprisPl.RemoteState(d.ID()) != nil { - targetDeviceID = d.ID() - break - } - } + targetDeviceID := p.DeviceID if targetDeviceID == "" { + // Pick the first connected device that has cached MPRIS state, + // falling back to any connected device. + devs := devices.List() for _, d := range devs { - if d.IsConnected() { + if d.IsConnected() && mprisPl.RemoteState(d.ID()) != nil { targetDeviceID = d.ID() break } } + if targetDeviceID == "" { + for _, d := range devs { + if d.IsConnected() { + targetDeviceID = d.ID() + break + } + } + } + } + if targetDeviceID == "" { + return ipc.Response{OK: false, Error: "no connected device found"} } - } - if targetDeviceID == "" { - return ipc.Response{OK: false, Error: "no connected device found"} - } - dev, ok := devices.Get(targetDeviceID) - if !ok { - return ipc.Response{OK: false, Error: fmt.Sprintf("device %s not found", targetDeviceID)} - } + dev, ok := devices.Get(targetDeviceID) + if !ok { + return ipc.Response{OK: false, Error: fmt.Sprintf("device %s not found", targetDeviceID)} + } - // Auto-detect player from cached remote state if not specified - player := p.Player - if player == "" { - if state := mprisPl.RemoteState(targetDeviceID); state != nil { - player = state.Player + // Auto-detect player from cached remote state if not specified + player := p.Player + if player == "" { + if state := mprisPl.RemoteState(targetDeviceID); state != nil { + player = state.Player + } + } + if player == "" { + return ipc.Response{OK: false, Error: "no player known for this device; specify --player"} } - } - if player == "" { - return ipc.Response{OK: false, Error: "no player known for this device; specify --player"} - } - if err := mprisPl.SendAction(dev, player, p.Action, p.Seek, p.Volume); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + if err := mprisPl.SendAction(dev, player, p.Action, p.Seek, p.Volume); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) handler.Register(ipc.CmdMprisRemote, func(req ipc.Request) ipc.Response { @@ -76,6 +69,10 @@ func registerMprisRoutes(handler *ipc.Handler, devices *device.Registry, plugins } mprisPl := pl.(*mpris.MPRISPlugin) + // User-initiated query: heal any signal drift first so the + // listing below reflects current bus truth. + mprisPl.RequestReconcile() + // Request fresh state from all connected devices (fires requests asynchronously) for _, d := range devices.List() { if d.IsConnected() { @@ -111,7 +108,6 @@ func registerMprisRoutes(handler *ipc.Handler, devices *device.Registry, plugins }) } - data, _ := json.Marshal(ipc.MprisRemoteResponse{Players: players}) - return ipc.Response{OK: true, Data: data} + return jsonOK(ipc.MprisRemoteResponse{Players: players}) }) } diff --git a/internal/daemon/ipc_routes_remotevolume.go b/internal/daemon/ipc_routes_remotevolume.go index 24c991a..9af3439 100644 --- a/internal/daemon/ipc_routes_remotevolume.go +++ b/internal/daemon/ipc_routes_remotevolume.go @@ -1,8 +1,6 @@ package daemon import ( - "encoding/json" - "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/ipc" "github.com/bethropolis/kcd/internal/plugin" @@ -12,69 +10,32 @@ import ( func registerRemoteVolumeRoutes(handler *ipc.Handler, devices *device.Registry, plugins *plugin.Registry) { handler.Register(ipc.CmdRemoteVolumeList, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("RemoteSystemVolume") - if !ok { - return ipc.Response{OK: false, Error: "remotesystemvolume plugin not enabled"} - } - sinks := pl.(*remotesystemvolume.RemoteSystemVolumePlugin).ListSinks(p.DeviceID) - if sinks == nil { - return ipc.Response{OK: true, Data: mustJSON([]remotesystemvolume.SinkInfo{})} - } - data, _ := json.Marshal(sinks) - return ipc.Response{OK: true, Data: data} + return pluginRoute(req, &p, plugins, "RemoteSystemVolume", func(pl plugin.Plugin) ipc.Response { + sinks := pl.(*remotesystemvolume.RemoteSystemVolumePlugin).ListSinks(p.DeviceID) + if sinks == nil { + return jsonOK([]remotesystemvolume.SinkInfo{}) + } + return jsonOK(sinks) + }) }) handler.Register(ipc.CmdRemoteVolumeSet, func(req ipc.Request) ipc.Response { - var p struct { - DeviceID string `json:"deviceId"` - Name string `json:"name"` - Volume int `json:"volume"` - } - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("RemoteSystemVolume") - if !ok { - return ipc.Response{OK: false, Error: "remotesystemvolume plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*remotesystemvolume.RemoteSystemVolumePlugin).SetVolume(dev, p.Name, p.Volume); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + var p ipc.RemoteVolumeSetPayload + return deviceRoute(req, &p, devices, plugins, "RemoteSystemVolume", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*remotesystemvolume.RemoteSystemVolumePlugin).SetVolume(dev, p.Name, p.Volume); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) handler.Register(ipc.CmdRemoteVolumeMute, func(req ipc.Request) ipc.Response { - var p struct { - DeviceID string `json:"deviceId"` - Name string `json:"name"` - Muted bool `json:"muted"` - } - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("RemoteSystemVolume") - if !ok { - return ipc.Response{OK: false, Error: "remotesystemvolume plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*remotesystemvolume.RemoteSystemVolumePlugin).SetMuted(dev, p.Name, p.Muted); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + var p ipc.RemoteVolumeMutePayload + return deviceRoute(req, &p, devices, plugins, "RemoteSystemVolume", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*remotesystemvolume.RemoteSystemVolumePlugin).SetMuted(dev, p.Name, p.Muted); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) } - -func mustJSON(v any) []byte { - b, _ := json.Marshal(v) - return b -} diff --git a/internal/daemon/ipc_routes_runcommand.go b/internal/daemon/ipc_routes_runcommand.go index 9710f17..88aa04c 100644 --- a/internal/daemon/ipc_routes_runcommand.go +++ b/internal/daemon/ipc_routes_runcommand.go @@ -1,42 +1,31 @@ package daemon import ( - "encoding/json" - "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/ipc" + "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" ) -func registerRunCommandRoutes(handler *ipc.Handler, devices *device.Registry) { +func registerRunCommandRoutes(handler *ipc.Handler, devices *device.Registry, plugins *plugin.Registry) { handler.Register(ipc.CmdRunList, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - pkt, _ := protocol.NewPacket("kdeconnect.runcommand.request", map[string]bool{"requestCommandList": true}) - if err := dev.Send(pkt); err != nil { - return ipc.Response{OK: false, Error: "failed to send runcommand list request"} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "", func(dev *device.Device, _ plugin.Plugin) ipc.Response { + pkt, _ := protocol.NewPacket("kdeconnect.runcommand.request", map[string]bool{"requestCommandList": true}) + if err := dev.Send(pkt); err != nil { + return ipc.Response{OK: false, Error: "failed to send runcommand list request"} + } + return ipc.Response{OK: true} + }) }) handler.Register(ipc.CmdRunExec, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - pkt, _ := protocol.NewPacket("kdeconnect.runcommand.request", map[string]string{"key": p.Key}) - if err := dev.Send(pkt); err != nil { - return ipc.Response{OK: false, Error: "failed to send runcommand exec request"} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "", func(dev *device.Device, _ plugin.Plugin) ipc.Response { + pkt, _ := protocol.NewPacket("kdeconnect.runcommand.request", map[string]string{"key": p.Key}) + if err := dev.Send(pkt); err != nil { + return ipc.Response{OK: false, Error: "failed to send runcommand exec request"} + } + return ipc.Response{OK: true} + }) }) } diff --git a/internal/daemon/ipc_routes_sftp.go b/internal/daemon/ipc_routes_sftp.go index 8f7011a..b6b7292 100644 --- a/internal/daemon/ipc_routes_sftp.go +++ b/internal/daemon/ipc_routes_sftp.go @@ -2,7 +2,6 @@ package daemon import ( "context" - "encoding/json" "strconv" "strings" @@ -51,134 +50,87 @@ func resolveVolume(arg string, volumes []ipc.StorageVolumeResponse) string { func registerSftpRoutes(handler *ipc.Handler, devices *device.Registry, plugins *plugin.Registry) { handler.Register(ipc.CmdSftpInfo, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("SFTP") - if !ok { - return ipc.Response{OK: false, Error: "sftp plugin not enabled"} - } - info := pl.(*sftp.SftpPlugin).Info(p.DeviceID) - if info == nil { - return ipc.Response{OK: false, Error: "no SFTP credentials cached for this device — use 'kcd sftp request' first"} - } - data, _ := json.Marshal(info) - return ipc.Response{OK: true, Data: data} + return pluginRoute(req, &p, plugins, "SFTP", func(pl plugin.Plugin) ipc.Response { + info := pl.(*sftp.SftpPlugin).Info(p.DeviceID) + if info == nil { + return ipc.Response{OK: false, Error: "no SFTP credentials cached for this device — use 'kcd sftp request' first"} + } + return jsonOK(info) + }) }) handler.Register(ipc.CmdSftpVolumes, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("SFTP") - if !ok { - return ipc.Response{OK: false, Error: "sftp plugin not enabled"} - } - volumes := pl.(*sftp.SftpPlugin).Volumes(p.DeviceID) - if volumes == nil { - return ipc.Response{OK: false, Error: "no volumes available — use 'kcd sftp request' first"} - } - data, _ := json.Marshal(volumes) - return ipc.Response{OK: true, Data: data} + return pluginRoute(req, &p, plugins, "SFTP", func(pl plugin.Plugin) ipc.Response { + volumes := pl.(*sftp.SftpPlugin).Volumes(p.DeviceID) + if volumes == nil { + return ipc.Response{OK: false, Error: "no volumes available — use 'kcd sftp request' first"} + } + return jsonOK(volumes) + }) }) handler.Register(ipc.CmdSftpMount, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("SFTP") - if !ok { - return ipc.Response{OK: false, Error: "sftp plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - if err := pl.(*sftp.SftpPlugin).RequestMount(dev); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "SFTP", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + if err := pl.(*sftp.SftpPlugin).RequestMount(dev); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) handler.Register(ipc.CmdSftpMountLocal, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("SFTP") - if !ok { - return ipc.Response{OK: false, Error: "sftp plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - browsePath, err := pl.(*sftp.SftpPlugin).RequestAndMount(context.Background(), dev) - if err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - data, _ := json.Marshal(map[string]string{"path": browsePath}) - return ipc.Response{OK: true, Data: data} + return deviceRoute(req, &p, devices, plugins, "SFTP", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + browsePath, err := pl.(*sftp.SftpPlugin).RequestAndMount(context.Background(), dev) + if err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return jsonOK(map[string]string{"path": browsePath}) + }) }) handler.Register(ipc.CmdSftpUnmount, func(req ipc.Request) ipc.Response { var p ipc.DevicePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("SFTP") - if !ok { - return ipc.Response{OK: false, Error: "sftp plugin not enabled"} - } - if err := pl.(*sftp.SftpPlugin).Unmount(p.DeviceID); err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } - return ipc.Response{OK: true} + return pluginRoute(req, &p, plugins, "SFTP", func(pl plugin.Plugin) ipc.Response { + if err := pl.(*sftp.SftpPlugin).Unmount(p.DeviceID); err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } + return ipc.Response{OK: true} + }) }) handler.Register(ipc.CmdSftpBrowse, func(req ipc.Request) ipc.Response { var p ipc.SftpBrowsePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - pl, ok := plugins.GetByName("SFTP") - if !ok { - return ipc.Response{OK: false, Error: "sftp plugin not enabled"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - sftpPl := pl.(*sftp.SftpPlugin) + return deviceRoute(req, &p, devices, plugins, "SFTP", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + sftpPl := pl.(*sftp.SftpPlugin) - volumePath := p.Volume + volumePath := p.Volume - // If a volume was specified, try to resolve it to a path. - if volumePath != "" { - vols := sftpPl.Volumes(p.DeviceID) - if len(vols) > 0 { - sv := make([]ipc.StorageVolumeResponse, len(vols)) - for i, v := range vols { - sv[i] = ipc.StorageVolumeResponse{Name: v.Name, Path: v.Path} - } - if resolved := resolveVolume(volumePath, sv); resolved != "" { - volumePath = resolved + // If a volume was specified, try to resolve it to a path. + if volumePath != "" { + vols := sftpPl.Volumes(p.DeviceID) + if len(vols) > 0 { + sv := make([]ipc.StorageVolumeResponse, len(vols)) + for i, v := range vols { + sv[i] = ipc.StorageVolumeResponse{Name: v.Name, Path: v.Path} + } + if resolved := resolveVolume(volumePath, sv); resolved != "" { + volumePath = resolved + } } } - } - mountPath, volumes, err := sftpPl.RequestAndMountVolume(context.Background(), dev, volumePath) - if err != nil { - return ipc.Response{OK: false, Error: err.Error()} - } + mountPath, volumes, err := sftpPl.RequestAndMountVolume(context.Background(), dev, volumePath) + if err != nil { + return ipc.Response{OK: false, Error: err.Error()} + } - vols := make([]ipc.StorageVolumeResponse, len(volumes)) - for i, v := range volumes { - vols[i] = ipc.StorageVolumeResponse{Name: v.Name, Path: v.Path} - } + vols := make([]ipc.StorageVolumeResponse, len(volumes)) + for i, v := range volumes { + vols[i] = ipc.StorageVolumeResponse{Name: v.Name, Path: v.Path} + } - resp := ipc.SftpBrowseResponse{ - Path: mountPath, - Volumes: vols, - } - data, _ := json.Marshal(resp) - return ipc.Response{OK: true, Data: data} + return jsonOK(ipc.SftpBrowseResponse{ + Path: mountPath, + Volumes: vols, + }) + }) }) } diff --git a/internal/daemon/ipc_routes_share.go b/internal/daemon/ipc_routes_share.go index f72dec7..f94badb 100644 --- a/internal/daemon/ipc_routes_share.go +++ b/internal/daemon/ipc_routes_share.go @@ -2,7 +2,6 @@ package daemon import ( "context" - "encoding/json" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/ipc" @@ -13,24 +12,15 @@ import ( func registerShareRoutes(handler *ipc.Handler, devices *device.Registry, plugins *plugin.Registry) { handler.Register(ipc.CmdShare, func(req ipc.Request) ipc.Response { var p ipc.SharePayload - if err := json.Unmarshal(req.Payload, &p); err != nil { - return ipc.Response{OK: false, Error: "invalid payload"} - } - dev, ok := devices.Get(p.DeviceID) - if !ok { - return ipc.Response{OK: false, Error: "device not found"} - } - pl, ok := plugins.GetByName("Share") - if !ok { - return ipc.Response{OK: false, Error: "share plugin not enabled"} - } - sharePl, ok := pl.(*share.SharePlugin) - if !ok { - return ipc.Response{OK: false, Error: "invalid share plugin type"} - } - if err := sharePl.SendFile(context.Background(), dev, p.FilePath); err != nil { - return ipc.Response{OK: false, Error: "share failed: " + err.Error()} - } - return ipc.Response{OK: true} + return deviceRoute(req, &p, devices, plugins, "Share", func(dev *device.Device, pl plugin.Plugin) ipc.Response { + sharePl, ok := pl.(*share.SharePlugin) + if !ok { + return ipc.Response{OK: false, Error: "invalid share plugin type"} + } + if err := sharePl.SendFile(context.Background(), dev, p.FilePath); err != nil { + return ipc.Response{OK: false, Error: "share failed: " + err.Error()} + } + return ipc.Response{OK: true} + }) }) } diff --git a/internal/daemon/plugins.go b/internal/daemon/plugins.go index e375ebc..358c09e 100644 --- a/internal/daemon/plugins.go +++ b/internal/daemon/plugins.go @@ -7,6 +7,7 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/plugins/battery" "github.com/bethropolis/kcd/internal/plugins/clipboard" @@ -29,10 +30,9 @@ import ( "github.com/bethropolis/kcd/internal/plugins/systemvolume" "github.com/bethropolis/kcd/internal/plugins/telephony" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) -func setupPlugins(cfg *config.Config, bus *events.Bus, tlsCfg *tls.Config, logger *zap.Logger, devices *device.Registry, localCert *x509.Certificate, saveDevices func(), plugins *plugin.Registry) *pair.PairPlugin { +func setupPlugins(cfg *config.Config, bus *events.Bus, tlsCfg *tls.Config, logger log.Logger, devices *device.Registry, localCert *x509.Certificate, saveDevices func(), plugins *plugin.Registry) *pair.PairPlugin { sidechannel := transport.SidechannelOptions{ Timeout: config.Duration(cfg.Network.SidechannelTimeout), IdleTimeout: config.Duration(cfg.Network.TransferIdleTimeout), diff --git a/internal/daemon/transport.go b/internal/daemon/transport.go index fa42f37..5cb25e4 100644 --- a/internal/daemon/transport.go +++ b/internal/daemon/transport.go @@ -12,10 +12,10 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/discovery" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) // shouldEphemeralClose reports whether an active connection that exists @@ -41,11 +41,17 @@ func shouldEphemeralClose(dev *device.Device, pairingMode bool) bool { return true } -func runTransport(ctx context.Context, cfg *tls.Config, bc *discovery.BroadcasterController, identity *protocol.Packet, devices *device.Registry, plugins *plugin.Registry, localDeviceID string, logger *zap.Logger, opts *config.Config) { +// discoveryDialMinInterval rate-limits ephemeral dials triggered by UDP +// sightings of unknown devices: at most one dial per device per interval. +// Without it a chatty announcer would spawn a dial storm. +const discoveryDialMinInterval = 2 * time.Second + +func runTransport(ctx context.Context, cfg *tls.Config, bc *discovery.BroadcasterController, identity *protocol.Packet, devices *device.Registry, plugins *plugin.Registry, localDeviceID string, logger log.Logger, opts *config.Config) { + // TCP Listener tcpListener, err := transport.Listen(ctx, fmt.Sprintf(":%d", opts.TCPPort)) if err != nil { - logger.Error("failed to start TCP listener", zap.Error(err)) + logger.Error("failed to start TCP listener", log.Error(err)) return } defer tcpListener.Close() @@ -105,8 +111,8 @@ func runTransport(ctx context.Context, cfg *tls.Config, bc *discovery.Broadcaste // Never dial garbage ports from unauthenticated announcements. if !validDialPort(tcpPort) { logger.Debug("ignoring discovery with invalid tcpPort", - zap.String("device_id", body.DeviceID), - zap.Int("port", tcpPort)) + log.String("device_id", body.DeviceID), + log.Int("port", tcpPort)) return } @@ -118,7 +124,7 @@ func runTransport(ctx context.Context, cfg *tls.Config, bc *discovery.Broadcaste // exists only for the discovery handshake. if shouldEphemeralClose(dev, pairingMode) { logger.Debug("closing ephemeral discovery connection", - zap.String("device_id", body.DeviceID)) + log.String("device_id", body.DeviceID)) dev.Disconnect() return } @@ -152,7 +158,7 @@ func runTransport(ctx context.Context, cfg *tls.Config, bc *discovery.Broadcaste // "not reachable"). A sighting at a new address is a genuine // roam the backoff can't reach — dial it now. if lastIP := dev.LastIP(); lastIP == nil || !lastIP.Equal(ip) { - if dev.ShouldDiscoveryDial(2 * time.Second) { + if dev.ShouldDiscoveryDial(discoveryDialMinInterval) { // Prefer the peer's last authenticated listening port // over the unauthenticated sighted one. port := tcpPort @@ -165,7 +171,7 @@ func runTransport(ctx context.Context, cfg *tls.Config, bc *discovery.Broadcaste if currIP := dev.RemoteIP(); currIP == nil || !currIP.Equal(ip) { // Live socket is bound elsewhere: the peer roamed but // kept its address assertion — replace the zombie. - if dev.ShouldDiscoveryDial(2 * time.Second) { + if dev.ShouldDiscoveryDial(discoveryDialMinInterval) { port := tcpPort if p := dev.LastPort(); validDialPort(p) { port = p @@ -210,7 +216,7 @@ func runTransport(ctx context.Context, cfg *tls.Config, bc *discovery.Broadcaste // announcement. Pairing mode and explicit `kcd pair ` bypass. } - udpListener := discovery.NewListener(1716, localDeviceID, onDeviceFound, logger) + udpListener := discovery.NewListener(opts.TCPPort, localDeviceID, onDeviceFound, logger) go udpListener.Run(ctx) // Accept loop @@ -221,7 +227,7 @@ func runTransport(ctx context.Context, cfg *tls.Config, bc *discovery.Broadcaste if ctx.Err() != nil { return } - logger.Error("accept error", zap.Error(err)) + logger.Error("accept error", log.Error(err)) continue } @@ -248,13 +254,13 @@ func runTransport(ctx context.Context, cfg *tls.Config, bc *discovery.Broadcaste // our reinstall minted a new device ID) must not kill // an otherwise legitimate inbound. logger.Debug("inbound addressed to another device", - zap.String("device_id", preBody.DeviceID), - zap.String("target_device_id", preBody.TargetDeviceID)) + log.String("device_id", preBody.DeviceID), + log.String("target_device_id", preBody.TargetDeviceID)) } if dev, ok := devices.Get(preBody.DeviceID); ok && dev.InCooldown() && (bc == nil || !bc.IsRunning()) { logger.Debug("refusing inbound inside reconnect cooldown", - zap.String("device_id", preBody.DeviceID)) + log.String("device_id", preBody.DeviceID)) protocol.ReleasePacket(preTlsPkt) return } @@ -272,7 +278,7 @@ func runTransport(ctx context.Context, cfg *tls.Config, bc *discovery.Broadcaste c = nil // Prevent defer from closing the active connection if err := handleNewConnection(ctx, transConn, identity, devices, plugins, localDeviceID, cfg, logger, opts); err != nil { - logger.Debug("new connection setup failed", zap.Error(err)) + logger.Debug("new connection setup failed", log.Error(err)) transConn.Close() } }(conn) diff --git a/internal/daemon/transport_dial.go b/internal/daemon/transport_dial.go index c7f38b8..595dad5 100644 --- a/internal/daemon/transport_dial.go +++ b/internal/daemon/transport_dial.go @@ -6,14 +6,13 @@ import ( "encoding/json" "fmt" "net" - "time" "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) // validDialPort reports whether a discovery-advertised TCP port is usable. @@ -27,44 +26,34 @@ func validDialPort(port int) bool { // inside reconnectCooldown are skipped so post-roam bursts can't complete // near-simultaneously and churn the peer's duplicate resolution. // Explicit user actions (pair intent, manual connect) pass force=true. -func DialDevice(ctx context.Context, targetIP net.IP, targetPort int, targetID string, targetProto int, identity *protocol.Packet, cfg *tls.Config, devices *device.Registry, plugins *plugin.Registry, localDeviceID string, logger *zap.Logger, force bool, opts *config.Config) { +func DialDevice(ctx context.Context, targetIP net.IP, targetPort int, targetID string, targetProto int, identity *protocol.Packet, cfg *tls.Config, devices *device.Registry, plugins *plugin.Registry, localDeviceID string, logger log.Logger, force bool, opts *config.Config) { if targetIP == nil || !validDialPort(targetPort) { logger.Debug("refusing to dial invalid target", - zap.String("device_id", targetID), - zap.String("ip", targetIP.String()), - zap.Int("port", targetPort)) + log.String("device_id", targetID), + log.String("ip", targetIP.String()), + log.Int("port", targetPort)) return } if !force { if dev, ok := devices.Get(targetID); ok && dev.InCooldown() { logger.Debug("skipping dial inside reconnect cooldown", - zap.String("device_id", targetID), - zap.String("ip", targetIP.String())) + log.String("device_id", targetID), + log.String("ip", targetIP.String())) return } } addr := fmt.Sprintf("%s:%d", targetIP, targetPort) - logger.Debug("dialing discovered device", zap.String("device_id", targetID), zap.String("addr", addr)) + logger.Debug("dialing discovered device", log.String("device_id", targetID), log.String("addr", addr)) dialer := &net.Dialer{ Timeout: config.Duration(opts.Network.DialTimeout), } conn, err := dialer.DialContext(ctx, "tcp", addr) if err != nil { - logger.Debug("failed to dial peer", zap.Error(err)) + logger.Debug("failed to dial peer", log.Error(err)) return } - if tcpConn, ok := conn.(*net.TCPConn); ok { - if err := tcpConn.SetKeepAliveConfig(net.KeepAliveConfig{ - Enable: true, - Idle: 30 * time.Second, - Interval: 10 * time.Second, - Count: 3, - }); err != nil { - _ = tcpConn.SetKeepAlive(true) - _ = tcpConn.SetKeepAlivePeriod(30 * time.Second) - } - } + transport.SetTCPKeepAlive(conn) var myID protocol.IdentityBody json.Unmarshal(identity.Body, &myID) @@ -98,14 +87,14 @@ func DialDevice(ctx context.Context, targetIP net.IP, targetPort int, targetID s defer cancel() if err := tlsConn.HandshakeContext(handshakeCtx); err != nil { tlsConn.Close() - logger.Debug("tls handshake failed", zap.Error(err)) + logger.Debug("tls handshake failed", log.Error(err)) return } transConn := transport.NewConn(tlsConn) // Ensure the connection is closed if handleNewConnection fails mid-setup if err := handleNewConnection(ctx, transConn, identity, devices, plugins, localDeviceID, cfg, logger, opts); err != nil { - logger.Debug("new connection setup failed", zap.Error(err)) + logger.Debug("new connection setup failed", log.Error(err)) transConn.Close() } } diff --git a/internal/daemon/transport_handshake.go b/internal/daemon/transport_handshake.go index 340ece1..407b349 100644 --- a/internal/daemon/transport_handshake.go +++ b/internal/daemon/transport_handshake.go @@ -10,14 +10,14 @@ import ( "github.com/bethropolis/kcd/internal/cert" "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) // Returning an error ensures the caller can close the connection if it fails mid-setup. -func handleNewConnection(ctx context.Context, conn *transport.Conn, identity *protocol.Packet, devices *device.Registry, plugins *plugin.Registry, localDeviceID string, cfg *tls.Config, logger *zap.Logger, opts *config.Config) error { +func handleNewConnection(ctx context.Context, conn *transport.Conn, identity *protocol.Packet, devices *device.Registry, plugins *plugin.Registry, localDeviceID string, cfg *tls.Config, logger log.Logger, opts *config.Config) error { if err := conn.WritePacket(identity); err != nil { return fmt.Errorf("failed to send identity: %w", err) } @@ -77,9 +77,9 @@ func handleNewConnection(ctx context.Context, conn *transport.Conn, identity *pr dev.OutgoingCaps = peerBody.OutgoingCapabilities logger.Debug("device connected", - zap.String("device_id", peerBody.DeviceID), - zap.String("device_name", safeDeviceName), - zap.Int("protocol_version", peerBody.ProtocolVersion)) + log.String("device_id", peerBody.DeviceID), + log.String("device_name", safeDeviceName), + log.Int("protocol_version", peerBody.ProtocolVersion)) dispatch := func(ctx context.Context, sender *device.Device, pkt *protocol.Packet) bool { return plugins.Dispatch(ctx, sender, pkt) @@ -111,7 +111,7 @@ func handleNewConnection(ctx context.Context, conn *transport.Conn, identity *pr // Prevent multiple concurrent reconnect goroutines for the same device. if !sender.TryReconnect() { logger.Debug("auto-reconnect: already reconnecting, skipping", - zap.String("device_id", sender.ID())) + log.String("device_id", sender.ID())) return } go reconnectWithBackoff(ctx, sender, lastIP, identity, cfg, devices, plugins, localDeviceID, logger, opts) diff --git a/internal/daemon/transport_reconnect.go b/internal/daemon/transport_reconnect.go index 5ace804..c1751ec 100644 --- a/internal/daemon/transport_reconnect.go +++ b/internal/daemon/transport_reconnect.go @@ -8,9 +8,9 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // reconnectWithBackoff dials a paired device after it disconnects, using @@ -30,7 +30,7 @@ func reconnectWithBackoff( devices *device.Registry, plugins *plugin.Registry, localDeviceID string, - logger *zap.Logger, + logger log.Logger, opts *config.Config, ) { maxBackoff := config.Duration(opts.Reconnect.MaxBackoff) @@ -39,9 +39,9 @@ func reconnectWithBackoff( defer dev.ReconnectDone() logger.Info("starting auto-reconnect", - zap.String("device_id", dev.ID()), - zap.String("device_name", dev.Name()), - zap.String("ip", ip.String()), + log.String("device_id", dev.ID()), + log.String("device_name", dev.Name()), + log.String("ip", ip.String()), ) for { @@ -53,22 +53,22 @@ func reconnectWithBackoff( // Stop if the device was unpaired while we were waiting. if dev.State() != device.StatePaired { logger.Debug("auto-reconnect: device no longer paired, stopping", - zap.String("device_id", dev.ID())) + log.String("device_id", dev.ID())) return } // Stop if the device already reconnected (inbound connection from phone). if dev.IsConnected() { logger.Debug("auto-reconnect: device already connected, stopping", - zap.String("device_id", dev.ID())) + log.String("device_id", dev.ID())) return } backoff := device.ReconnectBackoff(attempt, maxBackoff, config.Duration(opts.Reconnect.InitialBackoff)) logger.Debug("auto-reconnect: waiting before next attempt", - zap.String("device_id", dev.ID()), - zap.Int("attempt", attempt+1), - zap.Duration("backoff", backoff), + log.String("device_id", dev.ID()), + log.Int("attempt", attempt+1), + log.Duration("backoff", backoff), ) select { @@ -83,9 +83,9 @@ func reconnectWithBackoff( } logger.Info("auto-reconnect: dialling", - zap.String("device_id", dev.ID()), - zap.String("ip", ip.String()), - zap.Int("attempt", attempt+1), + log.String("device_id", dev.ID()), + log.String("ip", ip.String()), + log.Int("attempt", attempt+1), ) // Prefer the peer's last advertised listening port over the @@ -96,8 +96,8 @@ func reconnectWithBackoff( if dev.IsConnected() { logger.Info("auto-reconnect: succeeded", - zap.String("device_id", dev.ID()), - zap.Int("attempts", attempt+1), + log.String("device_id", dev.ID()), + log.Int("attempts", attempt+1), ) // Persist the counter before returning: if this connection flaps, // the next reconnect cycle continues backing off rather than diff --git a/internal/daemon/transport_test.go b/internal/daemon/transport_test.go index b7aa924..8cd402e 100644 --- a/internal/daemon/transport_test.go +++ b/internal/daemon/transport_test.go @@ -11,14 +11,14 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/discovery" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) func ephemeralDevice(state device.PairingState, markEphemeral, pairIntent bool) *device.Device { - dev := device.NewDevice("test-id", "Test", "phone", zap.NewNop()) + dev := device.NewDevice("test-id", "Test", "phone", log.Nop()) dev.SetState(state) if markEphemeral { dev.MarkEphemeralDialed() @@ -54,7 +54,7 @@ func TestShouldEphemeralClose(t *testing.T) { } func TestEphemeralMarkerReset(t *testing.T) { - dev := device.NewDevice("test-id", "Test", "phone", zap.NewNop()) + dev := device.NewDevice("test-id", "Test", "phone", log.Nop()) if dev.EphemeralDialed() { t.Fatal("new device must not be ephemeral-marked") } @@ -82,7 +82,7 @@ func TestEphemeralMarkerReset(t *testing.T) { } func TestPairDialIntentLifetime(t *testing.T) { - dev := device.NewDevice("test-id", "Test", "phone", zap.NewNop()) + dev := device.NewDevice("test-id", "Test", "phone", log.Nop()) dev.MarkEphemeralDialed() if dev.PairDialActive() { @@ -117,7 +117,7 @@ func TestPairDialIntentLifetime(t *testing.T) { } func TestLastPortRoundTrip(t *testing.T) { - dev := device.NewDevice("test-id", "Test", "phone", zap.NewNop()) + dev := device.NewDevice("test-id", "Test", "phone", log.Nop()) if dev.LastPort() != 0 { t.Fatal("new device must have unknown port") } @@ -128,7 +128,7 @@ func TestLastPortRoundTrip(t *testing.T) { } func TestShouldDiscoveryDialThrottle(t *testing.T) { - dev := device.NewDevice("test-id", "Test", "phone", zap.NewNop()) + dev := device.NewDevice("test-id", "Test", "phone", log.Nop()) if !dev.ShouldDiscoveryDial(10 * time.Second) { t.Fatal("first dial must be allowed") } @@ -141,18 +141,18 @@ func TestShouldDiscoveryDialThrottle(t *testing.T) { } func newTestIdentity() (*protocol.Packet, error) { - return protocol.NewIdentityPacket("test-id", "Test", "desktop", 1716, nil, nil) + return protocol.NewIdentityPacket("test-id", "Test", "desktop", protocol.DefaultTCPPort, nil, nil) } func TestSyncReconnectBroadcast(t *testing.T) { - logger := zap.NewNop() + logger := log.Nop() ctx := context.Background() identity, err := newTestIdentity() if err != nil { t.Fatalf("identity: %v", err) } newBC := func() *discovery.BroadcasterController { - return discovery.NewBroadcasterController(identity, time.Hour, logger, nil) + return discovery.NewBroadcasterController(identity, protocol.DefaultTCPPort, time.Hour, logger, nil) } // Offline paired device starts the reconnect broadcast. @@ -236,7 +236,7 @@ func TestDialPreTLSIdentityTarget(t *testing.T) { if err != nil { t.Fatal(err) } - logger := zap.NewNop() + logger := log.Nop() addr := ln.Addr().(*net.TCPAddr) DialDevice(context.Background(), addr.IP, addr.Port, tc.targetID, protocol.ProtocolVersion, pkt, &tls.Config{}, device.NewRegistry(nil), plugin.NewRegistry(logger), "local", logger, true, cfg) select { diff --git a/internal/device/configurability_test.go b/internal/device/configurability_test.go index b05fba3..0b23079 100644 --- a/internal/device/configurability_test.go +++ b/internal/device/configurability_test.go @@ -1,7 +1,7 @@ package device import ( - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" "testing" "time" ) @@ -24,7 +24,7 @@ func TestConfiguredReconnectBackoff(t *testing.T) { } func TestConfiguredPairIntentTTL(t *testing.T) { - dev := NewDevice("peer", "Peer", "phone", zap.NewNop()) + dev := NewDevice("peer", "Peer", "phone", log.Nop()) before := time.Now() dev.RequestPairDial(time.Hour) deadline := time.Unix(0, dev.pairIntentUntil.Load()) diff --git a/internal/device/device_core.go b/internal/device/device_core.go index 603e102..4963445 100644 --- a/internal/device/device_core.go +++ b/internal/device/device_core.go @@ -8,9 +8,9 @@ import ( "time" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) // Device represents an active KDE Connect remote device. @@ -30,7 +30,8 @@ type Device struct { // lastPort is the tcpPort the peer last advertised over the authenticated // (post-TLS) identity exchange. Used with lastIP as the dial target for // paired devices so unauthenticated discovery packets can never redirect - // a paired auto-dial. Zero means unknown (fall back to 1716). + // a paired auto-dial. Zero means unknown (fall back to the configured + // tcp_port, protocol.DefaultTCPPort by default). lastPort int // discoveryIP/discoveryPort remember where a device was last seen @@ -112,13 +113,13 @@ type Device struct { onConnect func(dev *Device) onDisconnect func(dev *Device) - logger *zap.Logger + logger log.Logger bus *events.Bus } // NewDevice creates a new disconnected device instance. // New devices start as Unpaired (not Unknown) so listings are unambiguous. -func NewDevice(id, name, dtype string, logger *zap.Logger) *Device { +func NewDevice(id, name, dtype string, logger log.Logger) *Device { return &Device{ id: id, name: name, @@ -126,7 +127,7 @@ func NewDevice(id, name, dtype string, logger *zap.Logger) *Device { state: StateUnpaired, sendChan: make(chan *protocol.Packet, 32), done: make(chan struct{}), - logger: logger.With(zap.String("device_id", id)), + logger: logger.With(log.String("device_id", id)), } } diff --git a/internal/device/device_session.go b/internal/device/device_session.go index a3af372..9d1a497 100644 --- a/internal/device/device_session.go +++ b/internal/device/device_session.go @@ -7,9 +7,9 @@ import ( "time" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) // reconnectCooldown refuses new handshakes this long after a completed @@ -58,11 +58,11 @@ func (d *Device) Connect(ctx context.Context, conn *transport.Conn, dispatch fun if oldConn != nil { _ = oldConn.Close() d.logger.Debug("replacing superseded connection", - zap.String("old_addr", oldAddr), - zap.String("new_addr", newAddr)) + log.String("old_addr", oldAddr), + log.String("new_addr", newAddr)) } - d.logger.Info("device connected", zap.String("remote_addr", conn.RemoteAddr().String())) + d.logger.Info("device connected", log.String("remote_addr", conn.RemoteAddr().String())) if bus != nil { bus.Publish(events.TypeDeviceConnected, d.id, map[string]interface{}{ "name": d.name, diff --git a/internal/device/device_test.go b/internal/device/device_test.go index a381a56..18f1cad 100644 --- a/internal/device/device_test.go +++ b/internal/device/device_test.go @@ -8,14 +8,14 @@ import ( "testing" "time" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap/zaptest" ) func TestRegistry_Deduplicate(t *testing.T) { reg := NewRegistry(nil) - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) d1 := NewDevice("123", "Phone 1", "phone", logger) d2 := NewDevice("123", "Phone 2", "phone", logger) @@ -56,7 +56,7 @@ func TestReconnectBackoff(t *testing.T) { } func TestDevice_ReconnectAttempt(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) d := NewDevice("123", "Phone", "phone", logger) if got := d.ReconnectAttempt(); got != 0 { @@ -75,7 +75,7 @@ func TestDevice_ReconnectAttempt(t *testing.T) { } func TestDevice_ConnectionAge(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) d := NewDevice("123", "Phone", "phone", logger) if got := d.ConnectionAge(); got != 0 { @@ -110,7 +110,7 @@ func TestDevice_ConnectionAge(t *testing.T) { } func TestDevice_BatterySeen(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) d := NewDevice("bat-seen", "Phone", "phone", logger) if d.HasBattery() { @@ -260,7 +260,7 @@ func readAny(t *testing.T, peer net.Conn) { } func TestDevice_ReplaceOnNewAuth(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) d := NewDevice("dup", "Phone", "phone", logger) var connects, disconnects atomic.Int32 @@ -312,7 +312,7 @@ func TestDevice_ReplaceOnNewAuth(t *testing.T) { } func TestDevice_SupersededDisconnectSilent(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) d := NewDevice("sup", "Phone", "phone", logger) var disconnects atomic.Int32 @@ -342,7 +342,7 @@ func TestDevice_SupersededDisconnectSilent(t *testing.T) { } func TestDevice_ReplaceClosesOld(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) d := NewDevice("rep", "Phone", "phone", logger) ctx, cancel := context.WithCancel(context.Background()) @@ -380,7 +380,7 @@ func TestDevice_ReplaceClosesOld(t *testing.T) { } func TestDevice_CooldownWindow(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) d := NewDevice("cd", "Phone", "phone", logger) if d.InCooldown() { @@ -409,7 +409,7 @@ func TestDevice_CooldownWindow(t *testing.T) { } func TestDevice_NoteSightingRoamConfirm(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) d := NewDevice("roam", "Phone", "phone", logger) d.SetLastIP(net.ParseIP("192.168.1.10")) diff --git a/internal/device/io.go b/internal/device/io.go index c5392b8..bab7257 100644 --- a/internal/device/io.go +++ b/internal/device/io.go @@ -7,9 +7,9 @@ import ( "strings" "time" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) // writeTimeout bounds one WritePacket call. LAN writes complete in @@ -38,7 +38,7 @@ func (d *Device) Send(p *protocol.Packet) error { case sendChan <- p: return nil default: - d.logger.Warn("send channel full, dropping packet", zap.String("type", p.Type)) + d.logger.Warn("send channel full, dropping packet", log.String("type", p.Type)) protocol.ReleasePacket(p) return nil } @@ -63,16 +63,16 @@ func (d *Device) readLoop(ctx context.Context, conn *transport.Conn) { pkt, err := conn.ReadPacket() if err != nil { if strings.Contains(err.Error(), "protocol: unmarshal:") { - d.logger.Warn("dropping malformed packet, keeping connection", zap.Error(err)) + d.logger.Warn("dropping malformed packet, keeping connection", log.Error(err)) continue } - d.logger.Debug("read packet error (disconnecting)", zap.Error(err)) + d.logger.Debug("read packet error (disconnecting)", log.Error(err)) return } if dispatch != nil { if d.State() != StatePaired && pkt.Type != protocol.TypeIdentity && pkt.Type != protocol.TypePair { - d.logger.Debug("dropping packet from unpaired device", zap.String("type", pkt.Type)) + d.logger.Debug("dropping packet from unpaired device", log.String("type", pkt.Type)) protocol.ReleasePacket(pkt) } else if dispatch(ctx, d, pkt) { // Plugin completed within timeout — safe to recycle. @@ -115,12 +115,12 @@ func (d *Device) writerLoop(ctx context.Context) { protocol.ReleasePacket(pkt) continue } - d.logger.Debug("sending packet", zap.String("type", pkt.Type)) + d.logger.Debug("sending packet", log.String("type", pkt.Type)) _ = conn.SetWriteDeadline(time.Now().Add(writeTimeout)) err := conn.WritePacket(pkt) _ = conn.SetWriteDeadline(time.Time{}) if err != nil { - d.logger.Debug("write packet error", zap.Error(err)) + d.logger.Debug("write packet error", log.Error(err)) var netErr net.Error if errors.As(err, &netErr) && netErr.Timeout() { // Stuck socket: fail the session now instead of diff --git a/internal/device/registry.go b/internal/device/registry.go index 1992648..096fe7f 100644 --- a/internal/device/registry.go +++ b/internal/device/registry.go @@ -113,10 +113,14 @@ func (r *Registry) Prune(threshold time.Duration) int { return pruned } +// reconnectBackoffBase is the first retry delay for reconnect backoff; +// callers may override it via the initial parameter (tests do). +const reconnectBackoffBase = 2 * time.Second + // ReconnectBackoff calculates exponential backoff duration based on attempts. // Caps out at maxDuration. func ReconnectBackoff(attempt int, maxDuration time.Duration, initial ...time.Duration) time.Duration { - base := 2 * time.Second + base := reconnectBackoffBase if len(initial) > 0 && initial[0] > 0 { base = initial[0] } diff --git a/internal/discovery/broadcaster.go b/internal/discovery/broadcaster.go index 4685731..578d692 100644 --- a/internal/discovery/broadcaster.go +++ b/internal/discovery/broadcaster.go @@ -6,28 +6,30 @@ import ( "net" "time" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // Broadcaster sends identity packets over UDP to advertise the local device. type Broadcaster struct { identityPacket *protocol.Packet + port int interval time.Duration idleInterval time.Duration - logger *zap.Logger + logger log.Logger } -// NewBroadcaster creates a UDP discovery broadcaster. -func NewBroadcaster(identity *protocol.Packet, interval time.Duration, logger *zap.Logger) *Broadcaster { +// NewBroadcaster creates a UDP discovery broadcaster targeting port. +func NewBroadcaster(identity *protocol.Packet, port int, interval time.Duration, logger log.Logger) *Broadcaster { return &Broadcaster{ identityPacket: identity, + port: port, interval: interval, - logger: logger.With(zap.String("component", "broadcaster")), + logger: logger.With(log.String("component", "broadcaster")), } } -// Run periodically sends the identity packet to 255.255.255.255:1716. +// Run periodically sends the identity packet to 255.255.255.255:. // If shouldReduce is provided and returns true, the broadcast frequency // is reduced to 60 seconds to save CPU and network resources while idle. // (mDNS advertisement is no longer tied to this loop — see AdvertiseMDNS.) @@ -35,19 +37,19 @@ func (b *Broadcaster) Run(ctx context.Context, shouldReduce func() bool) { normalInterval := b.interval reducedInterval := b.idleInterval if reducedInterval <= 0 { - reducedInterval = 60 * time.Second + reducedInterval = defaultIdleInterval } conn, err := net.ListenUDP("udp4", nil) if err != nil { - b.logger.Error("failed to listen for udp broadcast", zap.Error(err)) + b.logger.Error("failed to listen for udp broadcast", log.Error(err)) return } defer conn.Close() data, err := json.Marshal(b.identityPacket) if err != nil { - b.logger.Error("failed to marshal identity packet", zap.Error(err)) + b.logger.Error("failed to marshal identity packet", log.Error(err)) return } data = append(data, '\n') @@ -61,7 +63,7 @@ func (b *Broadcaster) Run(ctx context.Context, shouldReduce func() bool) { return case <-timer.C: // 1. Attempt global broadcast - globalAddr := &net.UDPAddr{IP: net.IPv4bcast, Port: 1716} + globalAddr := &net.UDPAddr{IP: net.IPv4bcast, Port: b.port} conn.WriteToUDP(data, globalAddr) // 2. Attempt per-interface directed broadcast for multi-homed reliability @@ -84,7 +86,7 @@ func (b *Broadcaster) Run(ctx context.Context, shouldReduce func() bool) { for i := 0; i < 4; i++ { bcast[i] = ip4[i] | ^mask[i] } - conn.WriteToUDP(data, &net.UDPAddr{IP: bcast, Port: 1716}) + conn.WriteToUDP(data, &net.UDPAddr{IP: bcast, Port: b.port}) } } } diff --git a/internal/discovery/broadcaster_controller.go b/internal/discovery/broadcaster_controller.go index 8128e96..e9131cf 100644 --- a/internal/discovery/broadcaster_controller.go +++ b/internal/discovery/broadcaster_controller.go @@ -5,8 +5,8 @@ import ( "sync" "time" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // Broadcast ownership: pairing mode (`kcd pair`) and the reconnect @@ -21,10 +21,11 @@ const ( // Starts in stopped state. Broadcast is only active while Start() is in effect. type BroadcasterController struct { identityPacket *protocol.Packet + port int interval time.Duration idleInterval time.Duration shouldReduce func() bool - logger *zap.Logger + logger log.Logger mu sync.Mutex running bool @@ -32,18 +33,25 @@ type BroadcasterController struct { owners map[string]struct{} } +// defaultIdleInterval is the broadcast period while idle (all pairs +// connected, no pairing owner): quiet enough to sip battery, frequent +// enough that a roamed phone finds us back within a minute. +const defaultIdleInterval = 60 * time.Second + // NewBroadcasterController creates a controller that starts in stopped state. -func NewBroadcasterController(identity *protocol.Packet, interval time.Duration, logger *zap.Logger, shouldReduce func() bool, idleInterval ...time.Duration) *BroadcasterController { - idle := 60 * time.Second +// port is the UDP discovery port broadcasts target (normally cfg.TCPPort). +func NewBroadcasterController(identity *protocol.Packet, port int, interval time.Duration, logger log.Logger, shouldReduce func() bool, idleInterval ...time.Duration) *BroadcasterController { + idle := defaultIdleInterval if len(idleInterval) > 0 && idleInterval[0] > 0 { idle = idleInterval[0] } return &BroadcasterController{ identityPacket: identity, + port: port, interval: interval, idleInterval: idle, shouldReduce: shouldReduce, - logger: logger.With(zap.String("component", "broadcaster")), + logger: logger.With(log.String("component", "broadcaster")), owners: make(map[string]struct{}), } } @@ -76,6 +84,7 @@ func (bc *BroadcasterController) StartOwned(parentCtx context.Context, owner str b := &Broadcaster{ identityPacket: bc.identityPacket, + port: bc.port, interval: bc.interval, idleInterval: bc.idleInterval, logger: bc.logger, diff --git a/internal/discovery/discovery_test.go b/internal/discovery/discovery_test.go index 75eee92..74cc29f 100644 --- a/internal/discovery/discovery_test.go +++ b/internal/discovery/discovery_test.go @@ -5,13 +5,13 @@ import ( "testing" "time" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) func testIdentity(t *testing.T) *protocol.Packet { t.Helper() - pkt, err := protocol.NewIdentityPacket("test-id", "Test", "desktop", 1716, nil, nil) + pkt, err := protocol.NewIdentityPacket("test-id", "Test", "desktop", protocol.DefaultTCPPort, nil, nil) if err != nil { t.Fatalf("NewIdentityPacket failed: %v", err) } @@ -19,7 +19,7 @@ func testIdentity(t *testing.T) *protocol.Packet { } func TestConfiguredBroadcastIntervals(t *testing.T) { - bc := NewBroadcasterController(testIdentity(t), 7*time.Second, zap.NewNop(), nil, 19*time.Second) + bc := NewBroadcasterController(testIdentity(t), protocol.DefaultTCPPort, 7*time.Second, log.Nop(), nil, 19*time.Second) if bc.interval != 7*time.Second || bc.idleInterval != 19*time.Second { t.Fatal("configured intervals not stored") } @@ -32,8 +32,8 @@ func TestConfiguredBroadcastIntervals(t *testing.T) { // other: withdrawing one owner leaves the loop up while the other holds it, // and the loop stops only when the last owner withdraws. func TestBroadcasterOwners(t *testing.T) { - logger := zap.NewNop() - bc := NewBroadcasterController(testIdentity(t), time.Hour, logger, nil) + logger := log.Nop() + bc := NewBroadcasterController(testIdentity(t), protocol.DefaultTCPPort, time.Hour, logger, nil) ctx := context.Background() if bc.IsRunning() { @@ -60,3 +60,16 @@ func TestBroadcasterOwners(t *testing.T) { t.Error("loop must stop when no owners remain") } } + +// A non-default tcp_port must reach the broadcaster: the controller stores +// it and hands it to every Broadcaster it spawns. +func TestBroadcasterControllerStoresPort(t *testing.T) { + bc := NewBroadcasterController(testIdentity(t), 1816, time.Hour, log.Nop(), nil) + if bc.port != 1816 { + t.Fatalf("controller port = %d, want 1816", bc.port) + } + b := NewBroadcaster(testIdentity(t), 1816, time.Hour, log.Nop()) + if b.port != 1816 { + t.Fatalf("broadcaster port = %d, want 1816", b.port) + } +} diff --git a/internal/discovery/listener.go b/internal/discovery/listener.go index feea722..55fbec6 100644 --- a/internal/discovery/listener.go +++ b/internal/discovery/listener.go @@ -5,8 +5,8 @@ import ( "encoding/json" "net" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // Listener listens for UDP identity packets from other devices. @@ -14,16 +14,16 @@ type Listener struct { port int localDeviceID string onDeviceFound func(ip net.IP, tcpPort int, identity *protocol.Packet) - logger *zap.Logger + logger log.Logger } // NewListener creates a UDP discovery listener. -func NewListener(port int, localDeviceID string, callback func(ip net.IP, tcpPort int, identity *protocol.Packet), logger *zap.Logger) *Listener { +func NewListener(port int, localDeviceID string, callback func(ip net.IP, tcpPort int, identity *protocol.Packet), logger log.Logger) *Listener { return &Listener{ port: port, localDeviceID: localDeviceID, onDeviceFound: callback, - logger: logger.With(zap.String("component", "udp-listener")), + logger: logger.With(log.String("component", "udp-listener")), } } @@ -35,7 +35,7 @@ func (l *Listener) Run(ctx context.Context) { addr := &net.UDPAddr{Port: l.port} conn, err := net.ListenUDP("udp", addr) if err != nil { - l.logger.Error("failed to listen on udp", zap.Int("port", l.port), zap.Error(err)) + l.logger.Error("failed to listen on udp", log.Int("port", l.port), log.Error(err)) return } defer conn.Close() @@ -54,7 +54,7 @@ func (l *Listener) Run(ctx context.Context) { if ctx.Err() != nil { return // clean exit on context cancel } - l.logger.Debug("read udp error", zap.Error(err)) + l.logger.Debug("read udp error", log.Error(err)) continue } diff --git a/internal/discovery/listener_mdns.go b/internal/discovery/listener_mdns.go index c5a699f..de1f224 100644 --- a/internal/discovery/listener_mdns.go +++ b/internal/discovery/listener_mdns.go @@ -6,9 +6,9 @@ import ( "strconv" "strings" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" "github.com/libp2p/zeroconf/v2" - "go.uber.org/zap" ) // AdvertiseMDNS registers the local identity as _kdeconnect._udp until @@ -16,10 +16,10 @@ import ( // incoming queries), so it stays up for the daemon lifetime at negligible // idle cost and gives phones a standing discovery path even while UDP // broadcast is stopped. -func AdvertiseMDNS(ctx context.Context, identityPacket *protocol.Packet, logger *zap.Logger) { +func AdvertiseMDNS(ctx context.Context, identityPacket *protocol.Packet, logger log.Logger) { var idBody protocol.IdentityBody if err := json.Unmarshal(identityPacket.Body, &idBody); err != nil { - logger.Warn("failed to parse identity for mDNS", zap.Error(err)) + logger.Warn("failed to parse identity for mDNS", log.Error(err)) return } server, err := zeroconf.Register( @@ -36,7 +36,7 @@ func AdvertiseMDNS(ctx context.Context, identityPacket *protocol.Packet, logger nil, ) if err != nil { - logger.Warn("failed to register mDNS service", zap.Error(err)) + logger.Warn("failed to register mDNS service", log.Error(err)) return } go func() { @@ -100,6 +100,6 @@ func (l *Listener) runMdnsDiscovery(ctx context.Context) { }(entries) if err := zeroconf.Browse(ctx, "_kdeconnect._udp", "local.", entries); err != nil { - l.logger.Warn("failed to browse mDNS", zap.Error(err)) + l.logger.Warn("failed to browse mDNS", log.Error(err)) } } diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index cae7e4d..a9b9ab9 100644 --- a/internal/doctor/doctor.go +++ b/internal/doctor/doctor.go @@ -3,12 +3,14 @@ package doctor import ( "context" + "fmt" "net" "os" "os/exec" "time" "github.com/bethropolis/kcd/internal/config" + "github.com/bethropolis/kcd/internal/protocol" ) // Check is the result of a single diagnostic check. @@ -47,11 +49,12 @@ func Run() []Check { checks = append(checks, checkBin("wtype", "wtype", "install wtype for Wayland keyboard input")) } - // port 1716/udp open - checks = append(checks, checkUDPPort(daemonCheck.Pass)) + // port /udp open (configured tcp_port, 1716 by default) + tcpPort := configuredTCPPort() + checks = append(checks, checkUDPPort(daemonCheck.Pass, tcpPort)) - // port 1716/tcp open - checks = append(checks, checkTCPPort(daemonCheck.Pass)) + // port /tcp open + checks = append(checks, checkTCPPort(daemonCheck.Pass, tcpPort)) // config file readable checks = append(checks, checkConfigFile()) @@ -62,10 +65,26 @@ func Run() []Check { return checks } +// configuredTCPPort returns tcp_port from the on-disk config, falling back +// to the protocol default when the config is missing or unset. +func configuredTCPPort() int { + cfg, err := config.Load(config.DefaultConfigPath()) + if err != nil || cfg.TCPPort == 0 { + return protocol.DefaultTCPPort + } + return cfg.TCPPort +} + +// Doctor probe budgets: the socket dial gets 1s, the whole probe 2s. +const ( + doctorDialTimeout = 1 * time.Second + doctorProbeTimeout = 2 * time.Second +) + func checkDaemon() Check { socketPath := config.DefaultSocketPath() - dialer := net.Dialer{Timeout: 1 * time.Second} - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + dialer := net.Dialer{Timeout: doctorDialTimeout} + ctx, cancel := context.WithTimeout(context.Background(), doctorProbeTimeout) defer cancel() conn, err := dialer.DialContext(ctx, "unix", socketPath) if err == nil { @@ -91,30 +110,32 @@ func checkAny(name, hint string, bins ...string) Check { return Check{Name: name, Detail: hint, Pass: false} } -func checkUDPPort(daemonRunning bool) Check { +func checkUDPPort(daemonRunning bool, port int) Check { + name := fmt.Sprintf("port %d/udp open", port) if daemonRunning { - return Check{Name: "port 1716/udp open", Pass: true} + return Check{Name: name, Pass: true} } var lc net.ListenConfig - l, err := lc.ListenPacket(context.Background(), "udp", ":1716") + l, err := lc.ListenPacket(context.Background(), "udp", fmt.Sprintf(":%d", port)) if err != nil { - return Check{Name: "port 1716/udp open", Detail: "port is blocked or in use: " + err.Error(), Pass: false} + return Check{Name: name, Detail: "port is blocked or in use: " + err.Error(), Pass: false} } l.Close() - return Check{Name: "port 1716/udp open", Pass: true} + return Check{Name: name, Pass: true} } -func checkTCPPort(daemonRunning bool) Check { +func checkTCPPort(daemonRunning bool, port int) Check { + name := fmt.Sprintf("port %d/tcp open", port) if daemonRunning { - return Check{Name: "port 1716/tcp open", Pass: true} + return Check{Name: name, Pass: true} } var lc net.ListenConfig - l, err := lc.Listen(context.Background(), "tcp", ":1716") + l, err := lc.Listen(context.Background(), "tcp", fmt.Sprintf(":%d", port)) if err != nil { - return Check{Name: "port 1716/tcp open", Detail: "port is blocked or in use: " + err.Error(), Pass: false} + return Check{Name: name, Detail: "port is blocked or in use: " + err.Error(), Pass: false} } l.Close() - return Check{Name: "port 1716/tcp open", Pass: true} + return Check{Name: name, Pass: true} } func checkConfigFile() Check { diff --git a/internal/doctor/doctor_test.go b/internal/doctor/doctor_test.go new file mode 100644 index 0000000..69011eb --- /dev/null +++ b/internal/doctor/doctor_test.go @@ -0,0 +1,19 @@ +package doctor + +import ( + "fmt" + "testing" +) + +// Port checks must report the configured tcp_port, not a hardcoded 1716. +func TestPortChecksUseConfiguredPort(t *testing.T) { + const port = 1816 + udp := checkUDPPort(true, port) + if udp.Name != fmt.Sprintf("port %d/udp open", port) || !udp.Pass { + t.Errorf("udp check = %+v, want name port %d/udp open passing", udp, port) + } + tcp := checkTCPPort(true, port) + if tcp.Name != fmt.Sprintf("port %d/tcp open", port) || !tcp.Pass { + t.Errorf("tcp check = %+v, want name port %d/tcp open passing", tcp, port) + } +} diff --git a/internal/events/bus.go b/internal/events/bus.go index 448cbf2..eee0029 100644 --- a/internal/events/bus.go +++ b/internal/events/bus.go @@ -4,7 +4,7 @@ import ( "sync" "time" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) // EventType defines the kind of event being broadcast. @@ -92,14 +92,14 @@ type Bus struct { mu sync.RWMutex subscribers map[uint64]*Subscriber nextID uint64 - logger *zap.Logger + logger log.Logger } // NewBus creates a new event bus. -func NewBus(logger *zap.Logger) *Bus { +func NewBus(logger log.Logger) *Bus { return &Bus{ subscribers: make(map[uint64]*Subscriber), - logger: logger.With(zap.String("component", "events")), + logger: logger.With(log.String("component", "events")), } } @@ -126,7 +126,7 @@ func (b *Bus) Subscribe(capacity int, filters ...EventType) *Subscriber { } b.subscribers[id] = sub - b.logger.Debug("new subscriber", zap.Uint64("id", id), zap.Int("filters", len(filters))) + b.logger.Debug("new subscriber", log.Uint64("id", id), log.Int("filters", len(filters))) return sub } @@ -138,7 +138,7 @@ func (b *Bus) unsubscribe(id uint64) { if sub, ok := b.subscribers[id]; ok { close(sub.ch) delete(b.subscribers, id) - b.logger.Debug("subscriber removed", zap.Uint64("id", id)) + b.logger.Debug("subscriber removed", log.Uint64("id", id)) } } @@ -161,9 +161,9 @@ func (b *Bus) Publish(typ EventType, deviceID string, payload any) { default: // Drop event if channel is full b.logger.Warn("subscriber channel full, dropping event", - zap.Uint64("id", sub.id), - zap.String("type", string(typ)), - zap.String("device_id", deviceID)) + log.Uint64("id", sub.id), + log.String("type", string(typ)), + log.String("device_id", deviceID)) } } } diff --git a/internal/ipc/configurability_test.go b/internal/ipc/configurability_test.go index 07c2f8b..4a2dfb3 100644 --- a/internal/ipc/configurability_test.go +++ b/internal/ipc/configurability_test.go @@ -4,15 +4,15 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugins/pair" - "go.uber.org/zap" "strings" "testing" "time" ) func TestConfiguredPairListenTimeout(t *testing.T) { - logger := zap.NewNop() + logger := log.Nop() bus := events.NewBus(logger) devices := device.NewRegistry(bus) cfg := config.Defaults() diff --git a/internal/ipc/handler.go b/internal/ipc/handler.go index 78fc722..b3f817e 100644 --- a/internal/ipc/handler.go +++ b/internal/ipc/handler.go @@ -13,6 +13,10 @@ import ( "github.com/bethropolis/kcd/internal/protocol" ) +// defaultPairListenTimeout bounds `kcd pair` listen mode when the daemon +// config leaves the pairing listen timeout unset. +const defaultPairListenTimeout = 60 * time.Second + // Handler handles incoming IPC requests. type Handler struct { devices *device.Registry @@ -225,7 +229,7 @@ func (h *Handler) SetPairListenTimeout(timeout time.Duration) { func (h *Handler) handlePairListen() Response { timeout := h.pairListenTimeout if timeout <= 0 { - timeout = 60 * time.Second + timeout = defaultPairListenTimeout } // Report any device already in StatePairRequestedByPeer WITHOUT // accepting it. The caller (CLI / GUI / script) inspects the candidate diff --git a/internal/ipc/ipc_test.go b/internal/ipc/ipc_test.go index 082d2cb..594a0a0 100644 --- a/internal/ipc/ipc_test.go +++ b/internal/ipc/ipc_test.go @@ -8,16 +8,16 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/ipc" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/pkg/client" - "go.uber.org/zap/zaptest" ) func TestIPCRoundTrip(t *testing.T) { // Setup dir := t.TempDir() sockPath := filepath.Join(dir, "test.sock") - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) devReg := device.NewRegistry(nil) pluginReg := plugin.NewRegistry(logger) diff --git a/internal/ipc/payload.go b/internal/ipc/payload.go new file mode 100644 index 0000000..63990e0 --- /dev/null +++ b/internal/ipc/payload.go @@ -0,0 +1,34 @@ +package ipc + +// GetDeviceID methods let route helpers treat every device-addressed +// payload uniformly. Each method is a one-line accessor over the +// payload's deviceId field; the JSON shapes are unchanged. +func (p DevicePayload) GetDeviceID() string { return p.DeviceID } +func (p SharePayload) GetDeviceID() string { return p.DeviceID } +func (p NotifyReplyPayload) GetDeviceID() string { return p.DeviceID } +func (p NotifyDismissPayload) GetDeviceID() string { return p.DeviceID } +func (p SMSPayload) GetDeviceID() string { return p.DeviceID } +func (p SMSConvPayload) GetDeviceID() string { return p.DeviceID } +func (p SMSAttachmentPayload) GetDeviceID() string { return p.DeviceID } +func (p SftpBrowsePayload) GetDeviceID() string { return p.DeviceID } +func (p MprisActionPayload) GetDeviceID() string { return p.DeviceID } + +// RemoteVolumeSetPayload is used for CmdRemoteVolumeSet. +type RemoteVolumeSetPayload struct { + DeviceID string `json:"deviceId"` + Name string `json:"name"` + Volume int `json:"volume"` +} + +// GetDeviceID returns the addressed device. +func (p RemoteVolumeSetPayload) GetDeviceID() string { return p.DeviceID } + +// RemoteVolumeMutePayload is used for CmdRemoteVolumeMute. +type RemoteVolumeMutePayload struct { + DeviceID string `json:"deviceId"` + Name string `json:"name"` + Muted bool `json:"muted"` +} + +// GetDeviceID returns the addressed device. +func (p RemoteVolumeMutePayload) GetDeviceID() string { return p.DeviceID } diff --git a/internal/ipc/proto.go b/internal/ipc/proto.go index cbbe19b..c5de5eb 100644 --- a/internal/ipc/proto.go +++ b/internal/ipc/proto.go @@ -36,6 +36,7 @@ const ( CmdSmsRequestAttachment = "sms_request_attachment" CmdContactsSync = "contacts_sync" CmdContactsList = "contacts_list" + CmdContactsClear = "contacts_clear" CmdSftpMountLocal = "sftp_mount_local" CmdSftpUnmount = "sftp_unmount" CmdStatus = "status" diff --git a/internal/ipc/server.go b/internal/ipc/server.go index 9e511f7..86d6e14 100644 --- a/internal/ipc/server.go +++ b/internal/ipc/server.go @@ -11,22 +11,22 @@ import ( "strconv" "github.com/bethropolis/kcd/internal/config" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) // Server handles listening for JSON IPC requests over a Unix socket. type Server struct { path string handler *Handler - logger *zap.Logger + logger log.Logger } // NewServer creates a new IPC server. -func NewServer(path string, handler *Handler, logger *zap.Logger) *Server { +func NewServer(path string, handler *Handler, logger log.Logger) *Server { return &Server{ path: path, handler: handler, - logger: logger.With(zap.String("component", "ipc")), + logger: logger.With(log.String("component", "ipc")), } } @@ -99,8 +99,8 @@ func (s *Server) serve(ctx context.Context, l net.Listener, activated bool) erro }() s.logger.Info("ipc server started", - zap.String("path", s.path), - zap.Bool("socket_activated", activated), + log.String("path", s.path), + log.Bool("socket_activated", activated), ) for { @@ -109,7 +109,7 @@ func (s *Server) serve(ctx context.Context, l net.Listener, activated bool) erro if ctx.Err() != nil { return nil } - s.logger.Error("ipc socket accept error", zap.Error(err)) + s.logger.Error("ipc socket accept error", log.Error(err)) continue } diff --git a/internal/ipc/snapshot_test.go b/internal/ipc/snapshot_test.go index 05bc9a3..a4a72d9 100644 --- a/internal/ipc/snapshot_test.go +++ b/internal/ipc/snapshot_test.go @@ -10,13 +10,13 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/ipc" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap/zaptest" ) func TestBuildSnapshotCoversOfflineDevices(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) devReg := device.NewRegistry(nil) pluginReg := plugin.NewRegistry(logger) @@ -86,7 +86,7 @@ func TestBuildSnapshotCoversOfflineDevices(t *testing.T) { // (unknown, not 0%); one packet makes the key appear with the sent values; // a real 0% packet is preserved as a present zero, not collapsed to unknown. func TestSummarizeDeviceBatteryOmission(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) pluginReg := plugin.NewRegistry(logger) fresh := device.NewDevice("dev-fresh", "Fresh", "phone", logger) @@ -144,7 +144,7 @@ func TestSummarizeDeviceBatteryOmission(t *testing.T) { // stamp goes stale for the whole session. The summary must report now for // connected devices and preserve the stored stamp for offline ones. func TestSummarizeDeviceConnectedMeansSeenNow(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) stale := time.Now().Add(-53 * time.Minute) offline := device.NewDevice("dev-off", "Offline Phone", "phone", logger) diff --git a/internal/log/fields.go b/internal/log/fields.go new file mode 100644 index 0000000..c3f9a15 --- /dev/null +++ b/internal/log/fields.go @@ -0,0 +1,41 @@ +package log + +import ( + "time" + + "go.uber.org/zap" +) + +// Field is a structured log field. A direct alias of zap.Field, so +// there is zero conversion cost at call sites. +type Field = zap.Field + +// String adds a string field. +func String(key, val string) Field { return zap.String(key, val) } + +// Strings adds a string-slice field. +func Strings(key string, val []string) Field { return zap.Strings(key, val) } + +// Int adds an int field. +func Int(key string, val int) Field { return zap.Int(key, val) } + +// Int64 adds an int64 field. +func Int64(key string, val int64) Field { return zap.Int64(key, val) } + +// Uint64 adds a uint64 field. +func Uint64(key string, val uint64) Field { return zap.Uint64(key, val) } + +// Bool adds a bool field. +func Bool(key string, val bool) Field { return zap.Bool(key, val) } + +// Float64 adds a float64 field. +func Float64(key string, val float64) Field { return zap.Float64(key, val) } + +// Duration adds a time.Duration field. +func Duration(key string, val time.Duration) Field { return zap.Duration(key, val) } + +// Any adds a field using reflection for the value. +func Any(key string, val any) Field { return zap.Any(key, val) } + +// Error adds an error field under the "error" key. A nil error is skipped. +func Error(err error) Field { return zap.Error(err) } diff --git a/internal/log/level.go b/internal/log/level.go new file mode 100644 index 0000000..82de98a --- /dev/null +++ b/internal/log/level.go @@ -0,0 +1,46 @@ +package log + +import ( + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +// New builds the daemon logger for a config level name: "debug", +// "info", "warn", "error" (or "quiet"). Unknown names fall back to +// info. Debug selects the development encoder; anything else uses +// production. The returned Logger carries an atomic level so SetLevel +// can adjust verbosity at runtime without restarting. +// +// AddCallerSkip keeps the "caller" field pointing at the real call +// site instead of this wrapper; With and Named inherit it. +func New(level string) (Logger, error) { + atomic := zap.NewAtomicLevel() + setAtomicLevel(atomic, level) + + var cfg zap.Config + if level == "debug" { + cfg = zap.NewDevelopmentConfig() + } else { + cfg = zap.NewProductionConfig() + } + cfg.Level = atomic + + zl, err := cfg.Build(zap.AddCallerSkip(1)) + if err != nil { + return Logger{}, err + } + return Logger{zap: zl, level: atomic}, nil +} + +func setAtomicLevel(al zap.AtomicLevel, level string) { + switch level { + case "debug": + al.SetLevel(zapcore.DebugLevel) + case "warn": + al.SetLevel(zapcore.WarnLevel) + case "error", "quiet": + al.SetLevel(zapcore.ErrorLevel) + default: + al.SetLevel(zapcore.InfoLevel) + } +} diff --git a/internal/log/log.go b/internal/log/log.go new file mode 100644 index 0000000..ba909a1 --- /dev/null +++ b/internal/log/log.go @@ -0,0 +1,56 @@ +// Package log is kcd's single logging seam. +// +// Every package logs through this wrapper instead of importing +// go.uber.org/zap directly (enforced by depguard). zap remains the +// backend; the wrapper exists so backend-wide concerns — caller +// attribution, level names, dev/prod encoding, test helpers — live in +// exactly one place and a future backend swap touches one package. +package log + +import "go.uber.org/zap" + +// Logger is a structured logger. The zero value is not usable; +// construct via New, Nop, NewDevelopment, or NewTest. +type Logger struct { + zap *zap.Logger + level zap.AtomicLevel +} + +// Debug logs at debug level. +func (l Logger) Debug(msg string, fields ...Field) { l.zap.Debug(msg, fields...) } + +// Info logs at info level. +func (l Logger) Info(msg string, fields ...Field) { l.zap.Info(msg, fields...) } + +// Warn logs at warn level. +func (l Logger) Warn(msg string, fields ...Field) { l.zap.Warn(msg, fields...) } + +// Error logs at error level. +func (l Logger) Error(msg string, fields ...Field) { l.zap.Error(msg, fields...) } + +// Fatal logs at fatal level, then exits. Retained for parity with the +// daemon's startup path; plugins must never call it. +func (l Logger) Fatal(msg string, fields ...Field) { l.zap.Fatal(msg, fields...) } + +// With returns a Logger carrying additional context fields. +func (l Logger) With(fields ...Field) Logger { + return Logger{zap: l.zap.With(fields...), level: l.level} +} + +// Named adds a logger name segment (surfaced as "logger" in output). +func (l Logger) Named(name string) Logger { + return Logger{zap: l.zap.Named(name), level: l.level} +} + +// SetLevel adjusts verbosity at runtime using the same names as New. +// See level.go. +func (l Logger) SetLevel(level string) { setAtomicLevel(l.level, level) } + +// Sync flushes buffered output. Whoever owns the root logger defers it. +func (l Logger) Sync() error { return l.zap.Sync() } + +// Nop returns a Logger that discards everything. Used in tests and as +// a fallback where no logger was provided. +func Nop() Logger { + return Logger{zap: zap.NewNop(), level: zap.NewAtomicLevel()} +} diff --git a/internal/log/log_test.go b/internal/log/log_test.go new file mode 100644 index 0000000..c2548db --- /dev/null +++ b/internal/log/log_test.go @@ -0,0 +1,79 @@ +package log + +import ( + "strings" + "testing" + "time" + + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" +) + +func TestNewLevelNames(t *testing.T) { + for _, level := range []string{"debug", "info", "warn", "error", "quiet", "bogus"} { + l, err := New(level) + if err != nil { + t.Fatalf("New(%q) returned error: %v", level, err) + } + _ = l.Sync() + } +} + +func TestNopDiscards(t *testing.T) { + l := Nop() + l.Debug("d", String("k", "v")) + l.Info("i", Int("n", 1)) + l.Warn("w", Bool("b", true)) + l.Error("e", Error(nil)) + _ = l.With(String("k", "v")).Named("n") + l.SetLevel("debug") + _ = l.Sync() + // Fatal intentionally not exercised: it exits the process. +} + +func newObservedLogger() (Logger, *observer.ObservedLogs) { + // Mirror production wiring: the Logger's atomic level gates the core. + atomic := zap.NewAtomicLevel() + atomic.SetLevel(zapcore.DebugLevel) + core, logged := observer.New(atomic) + l := Logger{zap: zap.New(core, zap.AddCaller(), zap.AddCallerSkip(1)), level: atomic} + return l, logged +} + +func TestCallerPointsAtCallSite(t *testing.T) { + l, logged := newObservedLogger() + + l.Info("direct", String("k", "v")) + l.With(String("a", "b")).Warn("scoped") + l.Named("test").Error("named", Int64("n", 1), Duration("d", time.Second)) + + entries := logged.All() + if len(entries) != 3 { + t.Fatalf("got %d entries, want 3", len(entries)) + } + for _, e := range entries { + if !e.Caller.Defined { + t.Error("entry has no caller information") + continue + } + if !strings.HasSuffix(e.Caller.File, "log_test.go") { + t.Errorf("caller = %s, want this test file (wrapper leaked into caller attribution)", e.Caller.File) + } + } +} + +func TestSetLevelFilters(t *testing.T) { + l, logged := newObservedLogger() + + l.SetLevel("error") + l.Info("dropped") + l.Error("kept") + + if logged.Len() != 1 { + t.Fatalf("got %d entries after SetLevel(error), want 1", logged.Len()) + } + if got := logged.All()[0].Message; got != "kept" { + t.Errorf("surviving entry = %q, want %q", got, "kept") + } +} diff --git a/internal/log/testing.go b/internal/log/testing.go new file mode 100644 index 0000000..9a99c8c --- /dev/null +++ b/internal/log/testing.go @@ -0,0 +1,19 @@ +package log + +import ( + "testing" + + "go.uber.org/zap" + "go.uber.org/zap/zaptest" +) + +// NewDevelopment returns a human-readable logger for tests. +func NewDevelopment() Logger { + zl, _ := zap.NewDevelopment(zap.AddCallerSkip(1)) + return Logger{zap: zl, level: zap.NewAtomicLevel()} +} + +// NewTest returns a Logger wired to the given test's output. +func NewTest(t *testing.T) Logger { + return Logger{zap: zaptest.NewLogger(t, zaptest.WrapOptions(zap.AddCallerSkip(1))), level: zap.NewAtomicLevel()} +} diff --git a/internal/plugin/exec.go b/internal/plugin/exec.go index 1bb68af..aebfd7a 100644 --- a/internal/plugin/exec.go +++ b/internal/plugin/exec.go @@ -5,12 +5,12 @@ import ( "os/exec" "time" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) // RunCommandAsync executes a system command in a goroutine so it doesn't block the plugin handler. // It logs a warning if the command fails, aiding in debugging missing dependencies (like notify-send, xclip). -func RunCommandAsync(logger *zap.Logger, name string, args ...string) { +func RunCommandAsync(logger log.Logger, name string, args ...string) { go func() { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() @@ -18,9 +18,9 @@ func RunCommandAsync(logger *zap.Logger, name string, args ...string) { cmd := exec.CommandContext(ctx, name, args...) if out, err := cmd.CombinedOutput(); err != nil { logger.Warn("subprocess failed", - zap.String("cmd", name), - zap.Error(err), - zap.String("output", string(out)), + log.String("cmd", name), + log.Error(err), + log.String("output", string(out)), ) } }() @@ -32,3 +32,11 @@ func RunCommandSync(ctx context.Context, name string, args ...string) ([]byte, e cmd := exec.CommandContext(ctx, name, args...) return cmd.CombinedOutput() } + +// RunCommandOutput executes a system command synchronously and returns its +// standard output only (stderr is discarded). Use it where stdout is parsed: +// combined output would let stderr corrupt the parse. Timeouts via ctx. +func RunCommandOutput(ctx context.Context, name string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, name, args...) + return cmd.Output() +} diff --git a/internal/plugin/exec_test.go b/internal/plugin/exec_test.go new file mode 100644 index 0000000..3886af1 --- /dev/null +++ b/internal/plugin/exec_test.go @@ -0,0 +1,48 @@ +package plugin + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/bethropolis/kcd/internal/log" +) + +// RunCommandOutput must return stdout only: stderr must not leak into +// parsed output. +func TestRunCommandOutputDiscardsStderr(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + out, err := RunCommandOutput(ctx, "sh", "-c", "printf hello; printf noise >&2") + if err != nil { + t.Fatalf("RunCommandOutput failed: %v", err) + } + if string(out) != "hello" { + t.Fatalf("output = %q, want %q (stderr leaked)", out, "hello") + } +} + +// RunCommandSync returns combined output with the failure. +func TestRunCommandSyncCombinedOnError(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + out, err := RunCommandSync(ctx, "sh", "-c", "echo oops; exit 3") + if err == nil { + t.Fatal("expected non-nil error for exit 3") + } + if !strings.Contains(string(out), "oops") { + t.Fatalf("output = %q, want it to contain %q", out, "oops") + } +} + +// RunCommandAsync must not block the caller. +func TestRunCommandAsyncReturnsImmediately(t *testing.T) { + start := time.Now() + RunCommandAsync(log.Nop(), "sleep", "30") + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Fatalf("RunCommandAsync blocked for %v", elapsed) + } +} diff --git a/internal/plugin/registry.go b/internal/plugin/registry.go index bbf94d2..46f6cd6 100644 --- a/internal/plugin/registry.go +++ b/internal/plugin/registry.go @@ -6,25 +6,30 @@ import ( "time" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) +// defaultDispatchTimeout bounds one plugin Handle call when the plugin +// reports no Timeout of its own. Every bundled plugin sets Timeout, so +// this fires only for third-party or misconfigured plugins. +const defaultDispatchTimeout = 15 * time.Second + // Registry manages the set of active plugins and routes packets to them. type Registry struct { plugins map[string]Plugin // keyed by packet type string byName map[string]Plugin // keyed by plugin.Name() list []Plugin // ordered list for iteration mu sync.RWMutex - logger *zap.Logger + logger log.Logger } // NewRegistry creates a new plugin registry. -func NewRegistry(logger *zap.Logger) *Registry { +func NewRegistry(logger log.Logger) *Registry { return &Registry{ plugins: make(map[string]Plugin), byName: make(map[string]Plugin), - logger: logger.With(zap.String("component", "plugin_registry")), + logger: logger.With(log.String("component", "plugin_registry")), } } @@ -39,15 +44,15 @@ func (r *Registry) Register(p Plugin) { for _, typ := range p.IncomingTypes() { if existing, ok := r.plugins[typ]; ok { r.logger.Warn("plugin packet type collision, overwriting", - zap.String("type", typ), - zap.String("old_plugin", existing.Name()), - zap.String("new_plugin", p.Name()), + log.String("type", typ), + log.String("old_plugin", existing.Name()), + log.String("new_plugin", p.Name()), ) } r.plugins[typ] = p } - r.logger.Debug("registered plugin", zap.String("plugin", p.Name())) + r.logger.Debug("registered plugin", log.String("plugin", p.Name())) } // Dispatch routes an incoming packet to the appropriate plugin. @@ -62,13 +67,13 @@ func (r *Registry) Dispatch(ctx context.Context, dev device.Sender, pkt *protoco if !ok { // No plugin registered for this type — safely ignore - r.logger.Debug("unhandled packet type", zap.String("type", pkt.Type)) + r.logger.Debug("unhandled packet type", log.String("type", pkt.Type)) return true } timeout := p.Timeout() if timeout == 0 { - timeout = 15 * time.Second // Fallback default + timeout = defaultDispatchTimeout // Fallback default } // Because we must recover from panics, we execute Handle in a separate goroutine. @@ -84,9 +89,9 @@ func (r *Registry) Dispatch(ctx context.Context, dev device.Sender, pkt *protoco defer func() { if err := recover(); err != nil { r.logger.Error("plugin panic recovered", - zap.String("plugin", p.Name()), - zap.String("packet_type", pkt.Type), - zap.Any("error", err), + log.String("plugin", p.Name()), + log.String("packet_type", pkt.Type), + log.Any("error", err), ) done <- nil // unblock select } @@ -98,17 +103,17 @@ func (r *Registry) Dispatch(ctx context.Context, dev device.Sender, pkt *protoco case err := <-done: if err != nil { r.logger.Error("plugin handle error", - zap.String("plugin", p.Name()), - zap.String("packet_type", pkt.Type), - zap.Error(err), + log.String("plugin", p.Name()), + log.String("packet_type", pkt.Type), + log.Error(err), ) } return true // Completed in time; safe to recycle case <-ctx.Done(): r.logger.Warn("plugin handle timeout", - zap.String("plugin", p.Name()), - zap.String("packet_type", pkt.Type), - zap.Duration("timeout", timeout), + log.String("plugin", p.Name()), + log.String("packet_type", pkt.Type), + log.Duration("timeout", timeout), ) return false // Timed out; DO NOT recycle — goroutine still holds packet } diff --git a/internal/plugin/registry_test.go b/internal/plugin/registry_test.go index a7a27a8..dce21a5 100644 --- a/internal/plugin/registry_test.go +++ b/internal/plugin/registry_test.go @@ -8,8 +8,8 @@ import ( "time" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // --- mocks --- @@ -56,7 +56,7 @@ func (m *mockSender) GetBattery() (int, bool) { return 0, false } // --- tests --- func TestDispatch_FastPlugin_ReturnsTrue(t *testing.T) { - logger, _ := zap.NewDevelopment() + logger := log.NewDevelopment() r := NewRegistry(logger) r.Register(&mockPlugin{ @@ -78,7 +78,7 @@ func TestDispatch_FastPlugin_ReturnsTrue(t *testing.T) { } func TestDispatch_SlowPluginTimeout_ReturnsFalse(t *testing.T) { - logger, _ := zap.NewDevelopment() + logger := log.NewDevelopment() r := NewRegistry(logger) slowDone := make(chan struct{}) @@ -111,7 +111,7 @@ func TestDispatch_SlowPluginTimeout_ReturnsFalse(t *testing.T) { } func TestDispatch_UnregisteredType_ReturnsTrue(t *testing.T) { - logger, _ := zap.NewDevelopment() + logger := log.NewDevelopment() r := NewRegistry(logger) pkt := protocol.AcquirePacket() diff --git a/internal/plugins/battery/battery_test.go b/internal/plugins/battery/battery_test.go index b1a1282..0d52d2f 100644 --- a/internal/plugins/battery/battery_test.go +++ b/internal/plugins/battery/battery_test.go @@ -8,13 +8,13 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap/zaptest" ) func newPlugin(t *testing.T) (*BatteryPlugin, *events.Bus) { t.Helper() - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) bus := events.NewBus(logger) cfg := config.BatteryConfig{} cfg.Defaults() @@ -22,7 +22,7 @@ func newPlugin(t *testing.T) (*BatteryPlugin, *events.Bus) { } func TestBatteryPlugin_Handle_UpdatesDevice(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) p, _ := newPlugin(t) dev := device.NewDevice("dev1", "Test Phone", "phone", logger) @@ -44,7 +44,7 @@ func TestBatteryPlugin_Handle_UpdatesDevice(t *testing.T) { } func TestBatteryPlugin_Handle_ThresholdLow_EmitsEvent(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) p, bus := newPlugin(t) dev := device.NewDevice("dev1", "Test Phone", "phone", logger) @@ -79,7 +79,7 @@ func TestBatteryPlugin_Handle_ThresholdLow_EmitsEvent(t *testing.T) { } func TestBatteryPlugin_Handle_ThresholdFull_EmitsEvent(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) p, bus := newPlugin(t) dev := device.NewDevice("dev1", "Test Phone", "phone", logger) @@ -105,7 +105,7 @@ func TestBatteryPlugin_Handle_ThresholdFull_EmitsEvent(t *testing.T) { } func TestBatteryPlugin_Handle_RequestResponds(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) p, _ := newPlugin(t) dev := device.NewDevice("dev1", "Test Phone", "phone", logger) @@ -132,7 +132,7 @@ func TestLocalBatteryRead(t *testing.T) { } func TestBatteryPlugin_Handle_NoThreshold_NoEvent(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) p, bus := newPlugin(t) dev := device.NewDevice("dev1", "Test Phone", "phone", logger) @@ -155,7 +155,7 @@ func TestBatteryPlugin_Handle_NoThreshold_NoEvent(t *testing.T) { } func TestBatteryPlugin_Handle_RequestDoesNotClobberCharge(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) p, _ := newPlugin(t) dev := device.NewDevice("dev1", "Test Phone", "phone", logger) dev.UpdateBattery(85, true) diff --git a/internal/plugins/battery/handle.go b/internal/plugins/battery/handle.go index 39c73ab..eb5840e 100644 --- a/internal/plugins/battery/handle.go +++ b/internal/plugins/battery/handle.go @@ -7,9 +7,9 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // Handle processes incoming battery packets. @@ -46,7 +46,7 @@ func (p *BatteryPlugin) Handle(ctx context.Context, dev device.Sender, pkt *prot func (p *BatteryPlugin) sendLocalState(dev device.Sender) error { charge, charging, err := readLocalBattery() if err != nil { - p.logger.Debug("local battery unavailable, skipping response", zap.Error(err)) + p.logger.Debug("local battery unavailable, skipping response", log.Error(err)) return nil } pkt, err := protocol.NewPacket("kdeconnect.battery", BatteryBody{ @@ -112,7 +112,7 @@ func (p *BatteryPlugin) OnConnect(dev device.Sender) { // Send our local battery to the phone. charge, charging, err := readLocalBattery() if err != nil { - p.logger.Debug("local battery unavailable on connect", zap.Error(err)) + p.logger.Debug("local battery unavailable on connect", log.Error(err)) return } pkt, _ = protocol.NewPacket("kdeconnect.battery", BatteryBody{ diff --git a/internal/plugins/battery/types.go b/internal/plugins/battery/types.go index 4c62d33..d67d7e8 100644 --- a/internal/plugins/battery/types.go +++ b/internal/plugins/battery/types.go @@ -5,7 +5,7 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/events" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) // ThresholdEvent values from the KDE Connect protocol. @@ -20,11 +20,11 @@ type BatteryPlugin struct { notifications config.NotificationConfig cfg config.BatteryConfig bus *events.Bus - logger *zap.Logger + logger log.Logger } // NewBatteryPlugin creates a BatteryPlugin. -func NewBatteryPlugin(cfg config.BatteryConfig, bus *events.Bus, logger *zap.Logger, notifications ...config.NotificationConfig) *BatteryPlugin { +func NewBatteryPlugin(cfg config.BatteryConfig, bus *events.Bus, logger log.Logger, notifications ...config.NotificationConfig) *BatteryPlugin { var notificationCfg config.NotificationConfig if len(notifications) > 0 { notificationCfg = notifications[0] @@ -33,7 +33,7 @@ func NewBatteryPlugin(cfg config.BatteryConfig, bus *events.Bus, logger *zap.Log notifications: notificationCfg, cfg: cfg, bus: bus, - logger: logger.With(zap.String("plugin", "battery")), + logger: logger.With(log.String("plugin", "battery")), } } diff --git a/internal/plugins/clipboard/backend.go b/internal/plugins/clipboard/backend.go index aaf936e..7448d89 100644 --- a/internal/plugins/clipboard/backend.go +++ b/internal/plugins/clipboard/backend.go @@ -11,7 +11,7 @@ import ( "strings" "time" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) // probeBackend determines the usable clipboard backend (wl-paste/xclip) by @@ -71,7 +71,7 @@ func (p *ClipboardPlugin) getBackend() (clipboardBackend, string) { p.backend = backend p.wlDisplay = disp p.logger.Debug("clipboard: backend detected", - zap.Int("backend", int(backend)), zap.String("wl_display", disp)) + log.Int("backend", int(backend)), log.String("wl_display", disp)) } return backend, disp } diff --git a/internal/plugins/clipboard/clipboard_test.go b/internal/plugins/clipboard/clipboard_test.go index 568071e..33a0a2f 100644 --- a/internal/plugins/clipboard/clipboard_test.go +++ b/internal/plugins/clipboard/clipboard_test.go @@ -14,8 +14,8 @@ import ( "time" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // setProbe points the plugin's probe at fn for the lifetime of the plugin. @@ -121,7 +121,7 @@ func TestProbeBackend_None(t *testing.T) { } func TestClipboardPlugin_ReProbesAfterFailure(t *testing.T) { - p := NewClipboardPlugin(nil, zap.NewNop(), false) + p := NewClipboardPlugin(nil, log.Nop(), false) // First probe fails (compositor not up at boot) — nothing cached. setProbe(t, p, func() (clipboardBackend, string) { return backendUnknown, "" }) @@ -143,7 +143,7 @@ func TestClipboardPlugin_ReProbesAfterFailure(t *testing.T) { } func TestClipboardPlugin_CmdInjectsWaylandEnv(t *testing.T) { - p := NewClipboardPlugin(nil, zap.NewNop(), false) + p := NewClipboardPlugin(nil, log.Nop(), false) setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-9" }) cmd := p.clipboardCmd("wl-copy") @@ -160,7 +160,7 @@ func TestClipboardPlugin_CmdInjectsWaylandEnv(t *testing.T) { } func TestRunClipboard_Success(t *testing.T) { - p := NewClipboardPlugin(nil, zap.NewNop(), false) + p := NewClipboardPlugin(nil, log.Nop(), false) out, err := p.runClipboard(context.Background(), exec.CommandContext(context.Background(), "/bin/sh", "-c", "printf hello")) if err != nil { @@ -172,7 +172,7 @@ func TestRunClipboard_Success(t *testing.T) { } func TestRunClipboard_WrapsStderr(t *testing.T) { - p := NewClipboardPlugin(nil, zap.NewNop(), false) + p := NewClipboardPlugin(nil, log.Nop(), false) _, err := p.runClipboard(context.Background(), exec.CommandContext(context.Background(), "/bin/sh", "-c", "echo boom >&2; exit 2")) if err == nil { @@ -185,7 +185,7 @@ func TestRunClipboard_WrapsStderr(t *testing.T) { } func TestRunClipboard_TimesOutHungSubprocess(t *testing.T) { - p := NewClipboardPlugin(nil, zap.NewNop(), false) + p := NewClipboardPlugin(nil, log.Nop(), false) // A hung subprocess must be killed by the 2s timeout — and not instantly // (that would regress to the old premature-cancel bug). @@ -262,7 +262,7 @@ func shimWith(t *testing.T, name, script string) { } func TestPush_EmptyClipboardIsSoftFail(t *testing.T) { - p := NewClipboardPlugin(nil, zap.NewNop(), false) + p := NewClipboardPlugin(nil, log.Nop(), false) setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-1" }) // Empty clipboard with nothing copied: wl-paste exits non-zero with a @@ -279,7 +279,7 @@ func TestPush_EmptyClipboardIsSoftFail(t *testing.T) { } func TestPush_EmptyOutputSendsNothing(t *testing.T) { - p := NewClipboardPlugin(nil, zap.NewNop(), false) + p := NewClipboardPlugin(nil, log.Nop(), false) setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-1" }) // Tool reports success but no content — still nothing to push. @@ -295,7 +295,7 @@ func TestPush_EmptyOutputSendsNothing(t *testing.T) { } func TestPush_SendsContent(t *testing.T) { - p := NewClipboardPlugin(nil, zap.NewNop(), false) + p := NewClipboardPlugin(nil, log.Nop(), false) setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-1" }) shimWith(t, "wl-paste", "#!/bin/sh\nprintf 'hello world'\n") @@ -321,7 +321,7 @@ func TestPush_SendsContent(t *testing.T) { // newline appended by wl-copy). A --watch-triggered Push must NOT send that // same content straight back to the phone. func TestPush_DoesNotEchoReceivedClipboard(t *testing.T) { - p := NewClipboardPlugin(nil, zap.NewNop(), false) + p := NewClipboardPlugin(nil, log.Nop(), false) setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-1" }) // Phone sent "hello"; wl-copy stored "hello\n" (no -n); wl-paste reads it @@ -364,7 +364,7 @@ func TestPush_DoesNotEchoReceivedClipboard(t *testing.T) { } func TestClipboardPlugin_Handle(t *testing.T) { - logger := zap.NewNop() + logger := log.Nop() dev := device.NewDevice("dev1", "Test", "phone", logger) p := NewClipboardPlugin(nil, logger, false) // Force the no-backend path so no real clipboard tool is invoked. diff --git a/internal/plugins/clipboard/handle.go b/internal/plugins/clipboard/handle.go index 90bf11a..5890687 100644 --- a/internal/plugins/clipboard/handle.go +++ b/internal/plugins/clipboard/handle.go @@ -15,9 +15,9 @@ import ( "github.com/bethropolis/kcd/internal/cert" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) // Handle processes incoming clipboard packets. @@ -64,11 +64,11 @@ func (p *ClipboardPlugin) Handle(ctx context.Context, dev device.Sender, pkt *pr // inbound lastContent guard and makes --watch echo the phone's own // clipboard straight back to it. if err := p.runCopy(context.Background(), p.clipboardCmd("wl-copy", "-n"), strings.NewReader(body.Content)); err != nil { - p.logger.Warn("clipboard: failed to set clipboard", zap.Error(err)) + p.logger.Warn("clipboard: failed to set clipboard", log.Error(err)) } case backendX11: if err := p.runCopy(context.Background(), p.clipboardCmd("xclip", "-selection", "clipboard"), strings.NewReader(body.Content)); err != nil { - p.logger.Warn("clipboard: failed to set clipboard", zap.Error(err)) + p.logger.Warn("clipboard: failed to set clipboard", log.Error(err)) } default: p.logger.Debug("clipboard: no backend available, dropping inbound copy") @@ -92,8 +92,8 @@ func (p *ClipboardPlugin) handleClipboardFile(ctx context.Context, dev device.Se if pkt.PayloadSize > maxClipboardFileSize { p.logger.Warn("clipboard file: rejected payload exceeding size limit", - zap.Int64("size", pkt.PayloadSize), - zap.Int("limit_bytes", maxClipboardFileSize), + log.Int64("size", pkt.PayloadSize), + log.Int("limit_bytes", maxClipboardFileSize), ) return nil } @@ -116,7 +116,7 @@ func (p *ClipboardPlugin) handleClipboardFile(ctx context.Context, dev device.Se // Download to a temp file tmpFile, err := os.CreateTemp("", "kcd-clip-*"+filepath.Ext(filename)) if err != nil { - p.logger.Error("clipboard file: failed to create temp file", zap.Error(err)) + p.logger.Error("clipboard file: failed to create temp file", log.Error(err)) return } tmpPath := tmpFile.Name() @@ -124,7 +124,7 @@ func (p *ClipboardPlugin) handleClipboardFile(ctx context.Context, dev device.Se defer os.Remove(tmpPath) if err := downloadToFile(ctx, remoteIP, payloadPort, payloadSize, tmpPath, p.tlsConfig, expectedFP, p.logger, p.sidechannel); err != nil { - p.logger.Error("clipboard file: download failed", zap.Error(err)) + p.logger.Error("clipboard file: download failed", log.Error(err)) return } @@ -150,7 +150,7 @@ func (p *ClipboardPlugin) handleClipboardFile(ctx context.Context, dev device.Se return } if err := p.runCopy(context.Background(), cmd, t); err != nil { - p.logger.Warn("clipboard file: failed to set clipboard", zap.Error(err)) + p.logger.Warn("clipboard file: failed to set clipboard", log.Error(err)) } }() @@ -158,7 +158,7 @@ func (p *ClipboardPlugin) handleClipboardFile(ctx context.Context, dev device.Se } // downloadToFile dials a TLS side-channel and streams the payload to dest. -func downloadToFile(ctx context.Context, ip net.IP, port int, size int64, dest string, tlsConfig *tls.Config, expectedFP string, logger *zap.Logger, options ...transport.SidechannelOptions) error { +func downloadToFile(ctx context.Context, ip net.IP, port int, size int64, dest string, tlsConfig *tls.Config, expectedFP string, logger log.Logger, options ...transport.SidechannelOptions) error { conn, err := transport.DialSidechannel(ctx, ip, port, tlsConfig, expectedFP, logger, options...) if err != nil { return err diff --git a/internal/plugins/clipboard/push.go b/internal/plugins/clipboard/push.go index f292f57..5cb6b43 100644 --- a/internal/plugins/clipboard/push.go +++ b/internal/plugins/clipboard/push.go @@ -8,8 +8,8 @@ import ( "time" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // Push copies the local clipboard to the remote device using wl-paste or xclip -o. @@ -89,7 +89,7 @@ func (p *ClipboardPlugin) readClipboard() string { } out, err := p.runClipboard(context.Background(), cmd) if err != nil { - p.logger.Debug("clipboard: read failed", zap.Error(err)) + p.logger.Debug("clipboard: read failed", log.Error(err)) return "" } return string(out) @@ -111,7 +111,7 @@ func (p *ClipboardPlugin) OnConnect(dev device.Sender) { } pkt, err := protocol.NewPacket("kdeconnect.clipboard.connect", body) if err != nil { - p.logger.Debug("clipboard: OnConnect: failed to build packet", zap.Error(err)) + p.logger.Debug("clipboard: OnConnect: failed to build packet", log.Error(err)) return } // Best-effort — device may still be completing the TLS handshake. diff --git a/internal/plugins/clipboard/types.go b/internal/plugins/clipboard/types.go index a259efa..4836f8a 100644 --- a/internal/plugins/clipboard/types.go +++ b/internal/plugins/clipboard/types.go @@ -5,8 +5,8 @@ import ( "sync" "time" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) type clipboardBackend int @@ -23,7 +23,7 @@ type ClipboardPlugin struct { pushOnConnect bool lastTimestamp int64 tlsConfig *tls.Config - logger *zap.Logger + logger log.Logger backend clipboardBackend wlDisplay string // WAYLAND_DISPLAY value for spawned subprocesses probe func() (clipboardBackend, string) @@ -33,19 +33,16 @@ type ClipboardPlugin struct { } // NewClipboardPlugin creates a clipboard plugin. -func NewClipboardPlugin(tlsConfig *tls.Config, logger *zap.Logger, pushOnConnect bool, options ...transport.SidechannelOptions) *ClipboardPlugin { +func NewClipboardPlugin(tlsConfig *tls.Config, logger log.Logger, pushOnConnect bool, options ...transport.SidechannelOptions) *ClipboardPlugin { var sidechannel transport.SidechannelOptions if len(options) > 0 { sidechannel = options[0] } - if logger == nil { - logger = zap.NewNop() - } return &ClipboardPlugin{ sidechannel: sidechannel, tlsConfig: tlsConfig, pushOnConnect: pushOnConnect, - logger: logger.With(zap.String("plugin", "clipboard")), + logger: logger.With(log.String("plugin", "clipboard")), probe: probeBackend, } } diff --git a/internal/plugins/connectivity/connectivity_test.go b/internal/plugins/connectivity/connectivity_test.go index c7c8776..0c086a8 100644 --- a/internal/plugins/connectivity/connectivity_test.go +++ b/internal/plugins/connectivity/connectivity_test.go @@ -9,8 +9,8 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) type testSender struct { @@ -31,7 +31,7 @@ func (s testSender) UpdateBattery(charge int, charging bool) {} func (s testSender) GetBattery() (int, bool) { return 0, false } func TestHandleDeduplicatesConnectivityReports(t *testing.T) { - bus := events.NewBus(zap.NewNop()) + bus := events.NewBus(log.Nop()) sub := bus.Subscribe(4, events.TypeConnectivityUpdate) defer sub.Close() diff --git a/internal/plugins/contacts/contacts_test.go b/internal/plugins/contacts/contacts_test.go index 8a1e17e..3d7d73d 100644 --- a/internal/plugins/contacts/contacts_test.go +++ b/internal/plugins/contacts/contacts_test.go @@ -10,8 +10,8 @@ import ( "testing" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap/zaptest" ) // fakeSender captures outbound packets for round-trip tests. @@ -36,7 +36,7 @@ func (f *fakeSender) GetBattery() (int, bool) { return 0, false } func testPlugin(t *testing.T) *ContactsPlugin { t.Helper() t.Setenv("XDG_DATA_HOME", t.TempDir()) - return NewContactsPlugin(nil, zaptest.NewLogger(t)) + return NewContactsPlugin(nil, log.NewTest(t)) } func TestParseVCard(t *testing.T) { @@ -298,3 +298,34 @@ func TestListEmptyWhenNeverSynced(t *testing.T) { t.Errorf("never-synced device must list empty, got %v", list) } } + +// Issue #38: vCard 2.1 FN with ENCODING=QUOTED-PRINTABLE must be decoded, +// not stored with =XX escapes intact. +func TestParseVCardQuotedPrintable(t *testing.T) { + vcard := "BEGIN:VCARD\nVERSION:2.1\nFN;CHARSET=UTF-8;ENCODING=QUOTED-PRINTABLE:=44=61=76=C3=AD=64=20=49=72=65=6C=61=6E=64\nTEL:+1-555-0100\nEND:VCARD" + name, _, _ := parseVCard(vcard) + if name != "Davíd Ireland" { + t.Errorf("qp name = %q, want %q", name, "Davíd Ireland") + } + + // Emoji name + folded QP soft break (`=` EOL + space continuation). + vcard = "BEGIN:VCARD\nVERSION:2.1\nFN;CHARSET=UTF-8;ENCODING=QUOTED-PRINTABLE:=50=61=74=20=F0=9F=92=A\n A\nTEL:1\nEND:VCARD" + name, _, _ = parseVCard(vcard) + if name != "Pat 💪" { + t.Errorf("folded qp name = %q, want %q", name, "Pat 💪") + } + + // Malformed QP must keep the raw value, not drop the contact. + vcard = "BEGIN:VCARD\nFN;ENCODING=QUOTED-PRINTABLE:=ZZ=not-hex\nEND:VCARD" + name, _, _ = parseVCard(vcard) + if name != "=ZZ=not-hex" { + t.Errorf("malformed qp name = %q, want raw passthrough", name) + } + + // Plain FN containing `=` must not be touched (no ENCODING param). + vcard = "BEGIN:VCARD\nFN:a=b\nEND:VCARD" + name, _, _ = parseVCard(vcard) + if name != "a=b" { + t.Errorf("plain name = %q, want %q", name, "a=b") + } +} diff --git a/internal/plugins/contacts/sync.go b/internal/plugins/contacts/sync.go index 5b8d433..a1db487 100644 --- a/internal/plugins/contacts/sync.go +++ b/internal/plugins/contacts/sync.go @@ -7,7 +7,7 @@ import ( "strings" "github.com/bethropolis/kcd/internal/device" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) // coerceTimestamp parses Android's string-encoded timestamps as well as @@ -49,7 +49,7 @@ func (p *ContactsPlugin) handleUIDsResponse(dev device.Sender, body []byte) { var raw map[string]json.RawMessage if err := json.Unmarshal(body, &raw); err != nil { - p.logger.Debug("contacts: malformed uids response", zap.Error(err)) + p.logger.Debug("contacts: malformed uids response", log.Error(err)) return nil, 0, 0, 0 } uidsRaw, ok := raw["uids"] @@ -59,18 +59,18 @@ func (p *ContactsPlugin) handleUIDsResponse(dev device.Sender, body []byte) { } var uids []string if err := json.Unmarshal(uidsRaw, &uids); err != nil { - p.logger.Debug("contacts: malformed uids list", zap.Error(err)) + p.logger.Debug("contacts: malformed uids list", log.Error(err)) return nil, 0, 0, 0 } if len(uids) > maxContactUIDs { p.logger.Warn("contacts: uids response exceeds cap, refusing", - zap.Int("count", len(uids))) + log.Int("count", len(uids))) return nil, 0, 0, 0 } dir, err := p.deviceDir(deviceID) if err != nil { - p.logger.Warn("contacts: bad device dir", zap.Error(err)) + p.logger.Warn("contacts: bad device dir", log.Error(err)) return nil, 0, 0, 0 } idx := loadIndex(dir) @@ -114,7 +114,7 @@ func (p *ContactsPlugin) handleUIDsResponse(dev device.Sender, body []byte) { } } if err := saveIndex(dir, idx); err != nil { - p.logger.Warn("contacts: failed to save index", zap.Error(err)) + p.logger.Warn("contacts: failed to save index", log.Error(err)) } return toFetch, added, updated, deleted }() @@ -122,8 +122,8 @@ func (p *ContactsPlugin) handleUIDsResponse(dev device.Sender, body []byte) { if len(toFetch) > 0 { if err := p.requestVCards(dev, toFetch); err != nil { p.logger.Warn("contacts: failed to request vcards", - zap.String("device_id", deviceID), - zap.Error(err)) + log.String("device_id", deviceID), + log.Error(err)) } } p.emit(deviceID, map[string]any{ @@ -143,7 +143,7 @@ func (p *ContactsPlugin) handleVCardsResponse(deviceID string, body []byte) { var raw map[string]json.RawMessage if err := json.Unmarshal(body, &raw); err != nil { - p.logger.Debug("contacts: malformed vcards response", zap.Error(err)) + p.logger.Debug("contacts: malformed vcards response", log.Error(err)) return } uidsRaw, ok := raw["uids"] @@ -153,18 +153,18 @@ func (p *ContactsPlugin) handleVCardsResponse(deviceID string, body []byte) { } var uids []string if err := json.Unmarshal(uidsRaw, &uids); err != nil { - p.logger.Debug("contacts: malformed vcards uids list", zap.Error(err)) + p.logger.Debug("contacts: malformed vcards uids list", log.Error(err)) return } if len(uids) > maxContactUIDs { p.logger.Warn("contacts: vcards response exceeds cap, refusing", - zap.Int("count", len(uids))) + log.Int("count", len(uids))) return } dir, err := p.deviceDir(deviceID) if err != nil { - p.logger.Warn("contacts: bad device dir", zap.Error(err)) + p.logger.Warn("contacts: bad device dir", log.Error(err)) return } idx := loadIndex(dir) @@ -182,14 +182,14 @@ func (p *ContactsPlugin) handleVCardsResponse(deviceID string, body []byte) { var vcard string if err := json.Unmarshal(vRaw, &vcard); err != nil { p.logger.Debug("contacts: non-string vcard, skipping", - zap.String("uid", sanitizeUID(uid))) + log.String("uid", sanitizeUID(uid))) skipped++ continue } if len(vcard) > maxVCardBytes { p.logger.Warn("contacts: oversized vcard, skipping", - zap.String("uid", sanitizeUID(uid)), - zap.Int("bytes", len(vcard))) + log.String("uid", sanitizeUID(uid)), + log.Int("bytes", len(vcard))) skipped++ continue } @@ -204,7 +204,7 @@ func (p *ContactsPlugin) handleVCardsResponse(deviceID string, body []byte) { continue } if err := os.WriteFile(path, []byte(vcard), 0600); err != nil { - p.logger.Warn("contacts: failed to store vcard", zap.Error(err)) + p.logger.Warn("contacts: failed to store vcard", log.Error(err)) skipped++ continue } @@ -220,7 +220,7 @@ func (p *ContactsPlugin) handleVCardsResponse(deviceID string, body []byte) { stored++ } if err := saveIndex(dir, idx); err != nil { - p.logger.Warn("contacts: failed to save index", zap.Error(err)) + p.logger.Warn("contacts: failed to save index", log.Error(err)) } p.emit(deviceID, map[string]any{ diff --git a/internal/plugins/contacts/types.go b/internal/plugins/contacts/types.go index 80e889b..51e0d6c 100644 --- a/internal/plugins/contacts/types.go +++ b/internal/plugins/contacts/types.go @@ -8,7 +8,7 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) // KDE Connect contacts packet types (see kdeconnect-kde @@ -59,7 +59,7 @@ type indexEntry struct { // device, and deletes stale entries. type ContactsPlugin struct { bus *events.Bus - logger *zap.Logger + logger log.Logger baseDir string // mu serializes sync processing across devices. Syncs are rare; @@ -69,7 +69,7 @@ type ContactsPlugin struct { // NewContactsPlugin creates a contacts plugin caching under // $XDG_DATA_HOME/kcd/contacts (0600 files, 0700 dirs). -func NewContactsPlugin(bus *events.Bus, logger *zap.Logger, cacheDirs ...string) *ContactsPlugin { +func NewContactsPlugin(bus *events.Bus, logger log.Logger, cacheDirs ...string) *ContactsPlugin { dataHome := os.Getenv("XDG_DATA_HOME") if dataHome == "" { home, _ := os.UserHomeDir() @@ -83,7 +83,7 @@ func NewContactsPlugin(bus *events.Bus, logger *zap.Logger, cacheDirs ...string) return &ContactsPlugin{ bus: bus, - logger: logger.With(zap.String("plugin", "contacts")), + logger: logger.With(log.String("plugin", "contacts")), baseDir: baseDir, } } @@ -107,7 +107,7 @@ func (p *ContactsPlugin) OnConnect(dev device.Sender) { return } if err := p.RequestSync(dev); err != nil { - p.logger.Debug("contacts: initial sync request failed", zap.Error(err)) + p.logger.Debug("contacts: initial sync request failed", log.Error(err)) } } diff --git a/internal/plugins/contacts/vcard.go b/internal/plugins/contacts/vcard.go index 927bd67..cc25e0f 100644 --- a/internal/plugins/contacts/vcard.go +++ b/internal/plugins/contacts/vcard.go @@ -1,6 +1,8 @@ package contacts import ( + "io" + "mime/quotedprintable" "regexp" "strings" ) @@ -15,6 +17,32 @@ func sanitizeUID(uid string) string { return uidSafeChars.ReplaceAllString(uid, "_") } +// qpSoftBreakFold fuses a QP soft break (`=` EOL) with vCard folding (whitespace continuation). +var qpSoftBreakFold = regexp.MustCompile("=\n[ \t]") + +// isQuotedPrintable reports whether field params request QP (ENCODING=QUOTED-PRINTABLE or QP). +func isQuotedPrintable(params string) bool { + for _, tok := range strings.Split(params, ";") { + kv := strings.SplitN(tok, "=", 2) + if len(kv) != 2 || strings.TrimSpace(kv[0]) != "ENCODING" { + continue + } + if v := strings.TrimSpace(kv[1]); v == "QUOTED-PRINTABLE" || v == "QP" { + return true + } + } + return false +} + +// decodeQuotedPrintable RFC 2045-decodes s; false on invalid input (caller keeps raw). Assumes UTF-8. +func decodeQuotedPrintable(s string) (string, bool) { + b, err := io.ReadAll(quotedprintable.NewReader(strings.NewReader(s))) + if err != nil { + return "", false + } + return string(b), true +} + // cleanDisplayValue strips control characters (terminal-escape injection // via contact names is a classic) and truncates overlong fields. func cleanDisplayValue(s string) string { @@ -35,9 +63,10 @@ func cleanDisplayValue(s string) string { // line scan: unfold continuations, split Name:Value, drop parameters // (TEL;TYPE=CELL:...). Returns the first FN and all TEL/EMAIL values. func parseVCard(vcard string) (name string, phones, emails []string) { - // Normalize newlines, then unfold continuation lines (leading SP/HT). + // Normalize newlines, fuse QP soft breaks with folding, unfold continuations. raw := strings.ReplaceAll(vcard, "\r\n", "\n") raw = strings.ReplaceAll(raw, "\r", "\n") + raw = qpSoftBreakFold.ReplaceAllString(raw, "") var lines []string for _, line := range strings.Split(raw, "\n") { if line == "" { @@ -55,10 +84,18 @@ func parseVCard(vcard string) (name string, phones, emails []string) { continue } field := strings.ToUpper(line[:sep]) + params := "" if i := strings.IndexByte(field, ';'); i >= 0 { + params = field[i+1:] field = field[:i] } - value := cleanDisplayValue(line[sep+1:]) + rawValue := line[sep+1:] + if isQuotedPrintable(params) { + if decoded, ok := decodeQuotedPrintable(rawValue); ok { + rawValue = decoded + } + } + value := cleanDisplayValue(rawValue) if value == "" { continue } diff --git a/internal/plugins/findthisdevice/findthisdevice.go b/internal/plugins/findthisdevice/findthisdevice.go index d07e285..69c39fe 100644 --- a/internal/plugins/findthisdevice/findthisdevice.go +++ b/internal/plugins/findthisdevice/findthisdevice.go @@ -3,22 +3,22 @@ package findthisdevice import ( "context" "os" - "os/exec" "strings" "time" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" + "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) type FindThisDevicePlugin struct { bus *events.Bus - logger *zap.Logger + logger log.Logger } -func NewFindThisDevicePlugin(bus *events.Bus, logger *zap.Logger) *FindThisDevicePlugin { +func NewFindThisDevicePlugin(bus *events.Bus, logger log.Logger) *FindThisDevicePlugin { return &FindThisDevicePlugin{bus: bus, logger: logger} } @@ -30,7 +30,7 @@ func (p *FindThisDevicePlugin) IncomingTypes() []string { func (p *FindThisDevicePlugin) OutgoingTypes() []string { return []string{} } func (p *FindThisDevicePlugin) Handle(ctx context.Context, dev device.Sender, pkt *protocol.Packet) error { - p.logger.Info("ring request received", zap.String("device_id", dev.ID())) + p.logger.Info("ring request received", log.String("device_id", dev.ID())) go func() { p.playAlarm() @@ -66,8 +66,7 @@ func (p *FindThisDevicePlugin) playAlarm() { } for _, player := range players { - cmd := exec.CommandContext(ctx, player.cmd, player.args...) - if err := cmd.Run(); err == nil { + if _, err := plugin.RunCommandSync(ctx, player.cmd, player.args...); err == nil { return } } @@ -94,7 +93,7 @@ func (p *FindThisDevicePlugin) unmuteAudio() func() { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - out, err := exec.CommandContext(ctx, "pactl", "get-sink-mute", "@DEFAULT_SINK@").Output() + out, err := plugin.RunCommandOutput(ctx, "pactl", "get-sink-mute", "@DEFAULT_SINK@") if err != nil { return func() {} } @@ -102,13 +101,13 @@ func (p *FindThisDevicePlugin) unmuteAudio() func() { if wasMuted { muteCtx, muteCancel := context.WithTimeout(context.Background(), 5*time.Second) defer muteCancel() - _ = exec.CommandContext(muteCtx, "pactl", "set-sink-mute", "@DEFAULT_SINK@", "0").Run() + _, _ = plugin.RunCommandSync(muteCtx, "pactl", "set-sink-mute", "@DEFAULT_SINK@", "0") } return func() { if wasMuted { restoreCtx, restoreCancel := context.WithTimeout(context.Background(), 5*time.Second) defer restoreCancel() - _ = exec.CommandContext(restoreCtx, "pactl", "set-sink-mute", "@DEFAULT_SINK@", "1").Run() + _, _ = plugin.RunCommandSync(restoreCtx, "pactl", "set-sink-mute", "@DEFAULT_SINK@", "1") } } } diff --git a/internal/plugins/lockdevice/lockdevice.go b/internal/plugins/lockdevice/lockdevice.go index bf5cd6d..7ba2b9f 100644 --- a/internal/plugins/lockdevice/lockdevice.go +++ b/internal/plugins/lockdevice/lockdevice.go @@ -6,23 +6,23 @@ import ( "context" "encoding/json" "os" - "os/exec" "strings" "time" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" + "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // LockDevicePlugin handles incoming lock/unlock requests from the phone. type LockDevicePlugin struct { - logger *zap.Logger + logger log.Logger } -func NewLockDevicePlugin(logger *zap.Logger) *LockDevicePlugin { +func NewLockDevicePlugin(logger log.Logger) *LockDevicePlugin { return &LockDevicePlugin{ - logger: logger.With(zap.String("plugin", "lockdevice")), + logger: logger.With(log.String("plugin", "lockdevice")), } } @@ -51,28 +51,23 @@ func (p *LockDevicePlugin) Handle(ctx context.Context, dev device.Sender, pkt *p locked := p.getLocked() pkt, err := protocol.NewPacket("kdeconnect.lock", LockBody{IsLocked: locked}) if err != nil { - p.logger.Error("lockdevice: failed to create reply packet", zap.Error(err)) + p.logger.Error("lockdevice: failed to create reply packet", log.Error(err)) return } if err := dev.Send(pkt); err != nil { - p.logger.Error("lockdevice: failed to send lock state", zap.Error(err)) + p.logger.Error("lockdevice: failed to send lock state", log.Error(err)) } }() return nil } - // Phone is requesting a lock/unlock action. - go func() { - if body.SetLocked { - if err := exec.CommandContext(context.Background(), "loginctl", "lock-session").Run(); err != nil { - p.logger.Warn("lockdevice: lock-session failed", zap.Error(err)) - } - } else { - if err := exec.CommandContext(context.Background(), "loginctl", "unlock-session").Run(); err != nil { - p.logger.Warn("lockdevice: unlock-session failed", zap.Error(err)) - } - } - }() + // Phone is requesting a lock/unlock action. Fire and forget so Handle + // returns immediately; failures are logged by the seam. + if body.SetLocked { + plugin.RunCommandAsync(p.logger, "loginctl", "lock-session") + } else { + plugin.RunCommandAsync(p.logger, "loginctl", "unlock-session") + } return nil } @@ -83,7 +78,10 @@ func (p *LockDevicePlugin) getLocked() bool { if sessionID == "" { sessionID = "auto" // loginctl will guess the current session } - out, err := exec.CommandContext(context.Background(), "loginctl", "show-session", sessionID, "-p", "LockedHint").Output() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + out, err := plugin.RunCommandOutput(ctx, "loginctl", "show-session", sessionID, "-p", "LockedHint") if err != nil { return false } @@ -92,12 +90,20 @@ func (p *LockDevicePlugin) getLocked() bool { // Lock triggers an immediate session lock from the daemon/IPC side. func (p *LockDevicePlugin) Lock(dev device.Sender) error { - return exec.CommandContext(context.Background(), "loginctl", "lock-session").Run() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + _, err := plugin.RunCommandSync(ctx, "loginctl", "lock-session") + return err } // Unlock triggers an immediate session unlock from the daemon/IPC side. func (p *LockDevicePlugin) Unlock(dev device.Sender) error { - return exec.CommandContext(context.Background(), "loginctl", "unlock-session").Run() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + _, err := plugin.RunCommandSync(ctx, "loginctl", "unlock-session") + return err } func (p *LockDevicePlugin) OnConnect(dev device.Sender) {} diff --git a/internal/plugins/mousepad/input.go b/internal/plugins/mousepad/input.go index abd7362..ddf43bf 100644 --- a/internal/plugins/mousepad/input.go +++ b/internal/plugins/mousepad/input.go @@ -1,14 +1,12 @@ package mousepad import ( - "context" - "os/exec" "strconv" "github.com/bendahl/uinput" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) func (p *MousepadPlugin) handleMove(body MousepadBody) { @@ -180,9 +178,7 @@ func (p *MousepadPlugin) execKeyFallback(keyName string) { } func (p *MousepadPlugin) runCmd(name string, arg ...string) { - if out, err := exec.CommandContext(context.Background(), name, arg...).CombinedOutput(); err != nil { - p.logger.Debug("command failed", zap.String("cmd", name), zap.Error(err), zap.String("output", string(out))) - } + plugin.RunCommandAsync(p.logger, name, arg...) } // OnConnect explicitly tells the Android app that this device supports Keyboard input. diff --git a/internal/plugins/mousepad/types.go b/internal/plugins/mousepad/types.go index ea22b33..a5d9c5d 100644 --- a/internal/plugins/mousepad/types.go +++ b/internal/plugins/mousepad/types.go @@ -8,11 +8,11 @@ import ( "github.com/bendahl/uinput" "github.com/bethropolis/kcd/internal/config" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) type MousepadPlugin struct { - logger *zap.Logger + logger log.Logger cfg config.MousepadConfig useYdotool bool useUinput bool @@ -29,11 +29,11 @@ type MousepadPlugin struct { cancel context.CancelFunc } -func NewMousepadPlugin(cfg config.MousepadConfig, logger *zap.Logger) *MousepadPlugin { +func NewMousepadPlugin(cfg config.MousepadConfig, logger log.Logger) *MousepadPlugin { ctx, cancel := context.WithCancel(context.Background()) isWayland := os.Getenv("WAYLAND_DISPLAY") != "" p := &MousepadPlugin{ - logger: logger.With(zap.String("plugin", "mousepad")), + logger: logger.With(log.String("plugin", "mousepad")), cfg: cfg, isWayland: isWayland, moveCh: make(chan MousepadBody, 1), @@ -45,7 +45,7 @@ func NewMousepadPlugin(cfg config.MousepadConfig, logger *zap.Logger) *MousepadP // Try uinput first if auto or explicit if cfg.Backend == "auto" || cfg.Backend == "uinput" { if err := p.initUinput(); err != nil { - p.logger.Warn("uinput initialization failed, falling back to legacy backends", zap.Error(err)) + p.logger.Warn("uinput initialization failed, falling back to legacy backends", log.Error(err)) } else { p.useUinput = true p.logger.Info("uinput initialized successfully") diff --git a/internal/plugins/mpris/art.go b/internal/plugins/mpris/art.go index 2ca537f..95479c2 100644 --- a/internal/plugins/mpris/art.go +++ b/internal/plugins/mpris/art.go @@ -11,9 +11,9 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugins/share" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) func (p *MPRISPlugin) sendAlbumArt(ctx context.Context, dev device.Sender, player, artUrl string) { @@ -38,7 +38,7 @@ func (p *MPRISPlugin) sendAlbumArt(ctx context.Context, dev device.Sender, playe f, err := os.Open(filePath) if err != nil { - p.logger.Debug("mpris: album art file not found", zap.String("path", filePath), zap.Error(err)) + p.logger.Debug("mpris: album art file not found", log.String("path", filePath), log.Error(err)) return } stat, err := f.Stat() @@ -56,7 +56,7 @@ func (p *MPRISPlugin) sendAlbumArt(ctx context.Context, dev device.Sender, playe } go func() { - _ = share.AcceptAndSend(ln, filePath, p.tlsConfig, dev.ID(), cert.PinnedFingerprint(dev.PeerCert()), 10*time.Second, nil, p.logger) + _ = share.AcceptAndSend(ln, filePath, p.tlsConfig, dev.ID(), cert.PinnedFingerprint(dev.PeerCert()), 10*time.Second, nil, p.logger, shareCfg.PortMin, shareCfg.PortMax) }() pkt, err := protocol.NewPacket("kdeconnect.mpris", map[string]interface{}{ @@ -95,7 +95,7 @@ func (p *MPRISPlugin) requestAlbumArt(dev device.Sender, player, artUrl string) return } if err := dev.Send(pkt); err != nil { - p.logger.Debug("mpris: album art request failed", zap.Error(err)) + p.logger.Debug("mpris: album art request failed", log.Error(err)) } } @@ -112,13 +112,13 @@ func (p *MPRISPlugin) receiveAlbumArt(_ context.Context, dev device.Sender, play } p.logger.Debug("mpris: receiving album art from remote", - zap.String("player", player), - zap.String("device_id", dev.ID()), - zap.Int64("size", size)) + log.String("player", player), + log.String("device_id", dev.ID()), + log.Int64("size", size)) tmp, err := os.CreateTemp(p.artCache.Dir(), ".art-*") if err != nil { - p.logger.Warn("mpris: failed to create temp file for album art", zap.Error(err)) + p.logger.Warn("mpris: failed to create temp file for album art", log.Error(err)) return } tmpPath := tmp.Name() @@ -130,13 +130,13 @@ func (p *MPRISPlugin) receiveAlbumArt(_ context.Context, dev device.Sender, play dlCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() if err := share.ReceiveSideChannel(dlCtx, remoteIP, port, size, tmpPath, p.tlsConfig, cert.PinnedFingerprint(dev.PeerCert()), nil, p.logger); err != nil { - p.logger.Warn("mpris: album art transfer failed", zap.Error(err)) + p.logger.Warn("mpris: album art transfer failed", log.Error(err)) return } fileURL, err := p.artCache.Commit(artUrl, tmpPath) if err != nil { - p.logger.Warn("mpris: failed to cache album art", zap.Error(err)) + p.logger.Warn("mpris: failed to cache album art", log.Error(err)) return } diff --git a/internal/plugins/mpris/artcache.go b/internal/plugins/mpris/artcache.go index a42cd85..0377cef 100644 --- a/internal/plugins/mpris/artcache.go +++ b/internal/plugins/mpris/artcache.go @@ -10,7 +10,7 @@ import ( "strings" "sync" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) // maxAlbumArtBytes caps inbound album art payloads (matches the 5 MiB @@ -31,7 +31,7 @@ type ArtCache struct { } // NewArtCache creates the cache directory and returns an empty cache. -func NewArtCache(logger *zap.Logger, cacheDirs ...string) *ArtCache { +func NewArtCache(logger log.Logger, cacheDirs ...string) *ArtCache { base, err := os.UserCacheDir() if err != nil || base == "" { base = filepath.Join(os.TempDir(), "kcd-cache") @@ -42,7 +42,7 @@ func NewArtCache(logger *zap.Logger, cacheDirs ...string) *ArtCache { } if err := os.MkdirAll(dir, 0700); err != nil { logger.Warn("mpris: failed to create album art cache dir", - zap.String("path", dir), zap.Error(err)) + log.String("path", dir), log.Error(err)) } purgeArtCacheDir(dir, logger) return &ArtCache{ @@ -52,7 +52,7 @@ func NewArtCache(logger *zap.Logger, cacheDirs ...string) *ArtCache { } // purgeArtCacheDir clears the cache when it grows past maxArtCacheFiles. -func purgeArtCacheDir(dir string, logger *zap.Logger) { +func purgeArtCacheDir(dir string, logger log.Logger) { entries, err := os.ReadDir(dir) if err != nil { return @@ -61,7 +61,7 @@ func purgeArtCacheDir(dir string, logger *zap.Logger) { return } logger.Debug("mpris: clearing oversized album art cache", - zap.Int("files", len(entries))) + log.Int("files", len(entries))) for _, e := range entries { if !e.IsDir() { _ = os.Remove(filepath.Join(dir, e.Name())) diff --git a/internal/plugins/mpris/handle.go b/internal/plugins/mpris/handle.go index 0ceaa75..982baa1 100644 --- a/internal/plugins/mpris/handle.go +++ b/internal/plugins/mpris/handle.go @@ -8,8 +8,8 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) func (p *MPRISPlugin) Handle(ctx context.Context, dev device.Sender, pkt *protocol.Packet) error { @@ -38,6 +38,11 @@ func (p *MPRISPlugin) Handle(ctx context.Context, dev device.Sender, pkt *protoc } if body.RequestPlayerList { + // The phone asks for current truth: heal any signal drift first + // (async — the answer below goes out from the current map and the + // reconcile follow-up broadcasts corrections). Handle stays + // non-blocking per the plugin contract. + p.requestReconcile() return p.sendPlayerList(dev) } @@ -45,7 +50,7 @@ func (p *MPRISPlugin) Handle(ctx context.Context, dev device.Sender, pkt *protoc // exist (their media session was destroyed) and request fresh status for // the ones still around. if body.PlayerList != nil { - p.logger.Debug("mpris: received player list from remote", zap.Strings("players", body.PlayerList)) + p.logger.Debug("mpris: received player list from remote", log.Strings("players", body.PlayerList)) pruned := false p.mu.Lock() if prev := p.remoteStates[dev.ID()]; prev != nil { diff --git a/internal/plugins/mpris/local.go b/internal/plugins/mpris/local.go index 68aaa64..3f591db 100644 --- a/internal/plugins/mpris/local.go +++ b/internal/plugins/mpris/local.go @@ -4,8 +4,8 @@ import ( "strings" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) func (p *MPRISPlugin) sendPlayerList(dev device.Sender) error { @@ -20,7 +20,7 @@ func (p *MPRISPlugin) sendPlayerList(dev device.Sender) error { displayNames = []string{} } - p.logger.Debug("mpris: sending player list", zap.Strings("players", displayNames)) + p.logger.Debug("mpris: sending player list", log.Strings("players", displayNames)) pkt, err := protocol.NewPacket("kdeconnect.mpris", map[string]interface{}{ "playerList": displayNames, @@ -92,7 +92,7 @@ func (p *MPRISPlugin) addPlayer(busName, uniqueName, displayName, shortName stri } p.mu.Unlock() - p.logger.Debug("mpris: added player", zap.String("displayName", displayName), zap.String("busName", busName)) + p.logger.Debug("mpris: added player", log.String("displayName", displayName), log.String("busName", busName)) if state, err := p.playerState(displayName); err == nil { p.mu.Lock() @@ -111,7 +111,7 @@ func (p *MPRISPlugin) removePlayer(displayName string) { delete(p.lastStates, displayName) p.mu.Unlock() - p.logger.Debug("mpris: removed player", zap.String("displayName", displayName)) + p.logger.Debug("mpris: removed player", log.String("displayName", displayName)) p.sendPlayerListBroadcast() } diff --git a/internal/plugins/mpris/mpris.go b/internal/plugins/mpris/mpris.go index 88d3f4a..1d73742 100644 --- a/internal/plugins/mpris/mpris.go +++ b/internal/plugins/mpris/mpris.go @@ -8,8 +8,8 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/godbus/dbus/v5" - "go.uber.org/zap" ) type trackedPlayer struct { @@ -21,7 +21,7 @@ type trackedPlayer struct { type MPRISPlugin struct { tlsConfig *tls.Config - logger *zap.Logger + logger log.Logger bus *events.Bus mu sync.RWMutex devices map[string]device.Sender @@ -31,6 +31,11 @@ type MPRISPlugin struct { watching bool telephonyCancel context.CancelFunc + // reconcileCh nudges the D-Bus watcher loop to re-list player names + // and heal drift. Buffered size 1 so bursts of triggers coalesce; + // sends are non-blocking. Event-driven only — no timers. + reconcileCh chan struct{} + players map[string]*trackedPlayer lastTracks map[string]trackIdentity lastStates map[string]*NowPlaying @@ -55,17 +60,17 @@ type remotePositionTracker struct { playing bool } -func NewMPRISPlugin(tlsConfig *tls.Config, bus *events.Bus, pauseMusic bool, logger *zap.Logger, cacheDirs ...string) *MPRISPlugin { +func NewMPRISPlugin(tlsConfig *tls.Config, bus *events.Bus, pauseMusic bool, logger log.Logger, cacheDirs ...string) *MPRISPlugin { dbusConn, err := dbus.ConnectSessionBus() if err != nil { - logger.Warn("mpris: failed to connect to D-Bus session bus", zap.Error(err)) + logger.Warn("mpris: failed to connect to D-Bus session bus", log.Error(err)) } else { logger.Info("mpris: connected to D-Bus session bus") } p := &MPRISPlugin{ tlsConfig: tlsConfig, - logger: logger.With(zap.String("plugin", "mpris")), + logger: logger.With(log.String("plugin", "mpris")), bus: bus, dbus: dbusConn, pauseMusic: pauseMusic, @@ -79,6 +84,7 @@ func NewMPRISPlugin(tlsConfig *tls.Config, bus *events.Bus, pauseMusic bool, log positionTrackers: make(map[string]*remotePositionTracker), callPausedPlayers: make([]string, 0), artCache: NewArtCache(logger, cacheDirs...), + reconcileCh: make(chan struct{}, 1), } // Start the watcher immediately (like C++ does in constructor). @@ -101,6 +107,19 @@ func NewMPRISPlugin(tlsConfig *tls.Config, bus *events.Bus, pauseMusic bool, log func (p *MPRISPlugin) Name() string { return "MPRIS" } func (p *MPRISPlugin) Timeout() time.Duration { return 5 * time.Second } + +// requestReconcile asks the D-Bus watcher loop to re-list player names +// and heal any drift. Non-blocking: a pending request already covers us. +func (p *MPRISPlugin) requestReconcile() { + select { + case p.reconcileCh <- struct{}{}: + default: + } +} + +// RequestReconcile is the exported hook for daemon IPC routes (e.g. the +// remote-state listing) so user-initiated queries heal drift too. +func (p *MPRISPlugin) RequestReconcile() { p.requestReconcile() } func (p *MPRISPlugin) IncomingTypes() []string { return []string{"kdeconnect.mpris", "kdeconnect.mpris.request"} } diff --git a/internal/plugins/mpris/mpris_test.go b/internal/plugins/mpris/mpris_test.go index 7b7d284..43716b8 100644 --- a/internal/plugins/mpris/mpris_test.go +++ b/internal/plugins/mpris/mpris_test.go @@ -11,8 +11,8 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) type testSender struct { @@ -33,11 +33,11 @@ func (s testSender) UpdateBattery(charge int, charging bool) {} func (s testSender) GetBattery() (int, bool) { return 0, false } func TestHandleDeduplicatesRemoteMPRISUpdates(t *testing.T) { - bus := events.NewBus(zap.NewNop()) + bus := events.NewBus(log.Nop()) sub := bus.Subscribe(4, events.TypeMprisUpdate) defer sub.Close() - plugin := NewMPRISPlugin(nil, bus, false, zap.NewNop()) + plugin := NewMPRISPlugin(nil, bus, false, log.Nop()) if plugin.watchCancel != nil { defer plugin.watchCancel() } @@ -142,7 +142,7 @@ func (s *recordingSender) sent() []*protocol.Packet { } func TestHandleRequestsAlbumArtForKdeconnectURI(t *testing.T) { - plugin := NewMPRISPlugin(nil, events.NewBus(zap.NewNop()), false, zap.NewNop()) + plugin := NewMPRISPlugin(nil, events.NewBus(log.Nop()), false, log.Nop()) if plugin.watchCancel != nil { defer plugin.watchCancel() } @@ -188,7 +188,7 @@ func TestHandleRequestsAlbumArtForKdeconnectURI(t *testing.T) { } func TestHandleIgnoresEmptyAlbumArtPayload(t *testing.T) { - plugin := NewMPRISPlugin(nil, events.NewBus(zap.NewNop()), false, zap.NewNop()) + plugin := NewMPRISPlugin(nil, events.NewBus(log.Nop()), false, log.Nop()) if plugin.watchCancel != nil { defer plugin.watchCancel() } @@ -208,11 +208,11 @@ func TestHandleIgnoresEmptyAlbumArtPayload(t *testing.T) { } func TestStampAlbumArtMatchesCurrentTrack(t *testing.T) { - bus := events.NewBus(zap.NewNop()) + bus := events.NewBus(log.Nop()) sub := bus.Subscribe(4, events.TypeMprisUpdate) defer sub.Close() - plugin := NewMPRISPlugin(nil, bus, false, zap.NewNop()) + plugin := NewMPRISPlugin(nil, bus, false, log.Nop()) if plugin.watchCancel != nil { defer plugin.watchCancel() } @@ -286,7 +286,7 @@ func TestStampAlbumArtMatchesCurrentTrack(t *testing.T) { } func TestPollRemoteStatesOnlyTargetsKnownPlayers(t *testing.T) { - plugin := NewMPRISPlugin(nil, events.NewBus(zap.NewNop()), false, zap.NewNop()) + plugin := NewMPRISPlugin(nil, events.NewBus(log.Nop()), false, log.Nop()) if plugin.watchCancel != nil { defer plugin.watchCancel() } @@ -353,11 +353,11 @@ func newMPRISRawPacket(t *testing.T, body map[string]interface{}) *protocol.Pack } func TestHandlePrunesRemovedPlayer(t *testing.T) { - bus := events.NewBus(zap.NewNop()) + bus := events.NewBus(log.Nop()) sub := bus.Subscribe(4, events.TypeMprisUpdate) defer sub.Close() - plugin := NewMPRISPlugin(nil, bus, false, zap.NewNop()) + plugin := NewMPRISPlugin(nil, bus, false, log.Nop()) if plugin.watchCancel != nil { defer plugin.watchCancel() } @@ -394,11 +394,11 @@ func TestHandlePrunesRemovedPlayer(t *testing.T) { } func TestHandlePrunesPlayerOnEmptyList(t *testing.T) { - bus := events.NewBus(zap.NewNop()) + bus := events.NewBus(log.Nop()) sub := bus.Subscribe(4, events.TypeMprisUpdate) defer sub.Close() - plugin := NewMPRISPlugin(nil, bus, false, zap.NewNop()) + plugin := NewMPRISPlugin(nil, bus, false, log.Nop()) if plugin.watchCancel != nil { defer plugin.watchCancel() } @@ -425,11 +425,11 @@ func TestHandlePrunesPlayerOnEmptyList(t *testing.T) { } func TestHandleKeepsListedPlayer(t *testing.T) { - bus := events.NewBus(zap.NewNop()) + bus := events.NewBus(log.Nop()) sub := bus.Subscribe(4, events.TypeMprisUpdate) defer sub.Close() - plugin := NewMPRISPlugin(nil, bus, false, zap.NewNop()) + plugin := NewMPRISPlugin(nil, bus, false, log.Nop()) if plugin.watchCancel != nil { defer plugin.watchCancel() } @@ -454,11 +454,11 @@ func TestHandleKeepsListedPlayer(t *testing.T) { } func TestPublishedEventHasAnchorAndPendingArt(t *testing.T) { - bus := events.NewBus(zap.NewNop()) + bus := events.NewBus(log.Nop()) sub := bus.Subscribe(4, events.TypeMprisUpdate) defer sub.Close() - plugin := NewMPRISPlugin(nil, bus, false, zap.NewNop()) + plugin := NewMPRISPlugin(nil, bus, false, log.Nop()) if plugin.watchCancel != nil { defer plugin.watchCancel() } diff --git a/internal/plugins/mpris/playerctl.go b/internal/plugins/mpris/playerctl.go index e59e153..04ede7f 100644 --- a/internal/plugins/mpris/playerctl.go +++ b/internal/plugins/mpris/playerctl.go @@ -4,8 +4,8 @@ import ( "context" "time" + "github.com/bethropolis/kcd/internal/log" "github.com/godbus/dbus/v5" - "go.uber.org/zap" ) const dbusTimeout = 500 * time.Millisecond @@ -19,7 +19,7 @@ func dbusCall(obj dbus.BusObject, method string, args ...interface{}) *dbus.Call func (p *MPRISPlugin) handleAction(player, action string, seek, setPos *int64, volume *int, shuffle *bool, loopStatus string) { pl := p.resolvePlayer(player) if pl == nil { - p.logger.Warn("mpris: cannot resolve player", zap.String("player", player)) + p.logger.Warn("mpris: cannot resolve player", log.String("player", player)) return } obj := p.dbus.Object(pl.busName, "/org/mpris/MediaPlayer2") @@ -27,13 +27,13 @@ func (p *MPRISPlugin) handleAction(player, action string, seek, setPos *int64, v switch action { case "Play", "Pause", "PlayPause", "Next", "Previous", "Stop": if err := dbusCall(obj, "org.mpris.MediaPlayer2.Player."+action).Err; err != nil { - p.logger.Warn("mpris: action failed", zap.String("action", action), zap.Error(err)) + p.logger.Warn("mpris: action failed", log.String("action", action), log.Error(err)) } } if seek != nil { if err := dbusCall(obj, "org.mpris.MediaPlayer2.Player.Seek", (*seek)*1000).Err; err != nil { - p.logger.Warn("mpris: seek failed", zap.Int64("offset", *seek), zap.Error(err)) + p.logger.Warn("mpris: seek failed", log.Int64("offset", *seek), log.Error(err)) } } @@ -42,26 +42,26 @@ func (p *MPRISPlugin) handleAction(player, action string, seek, setPos *int64, v targetPosUs := (*setPos) * 1000 seekOffset := targetPosUs - currentPosUs if err := dbusCall(obj, "org.mpris.MediaPlayer2.Player.Seek", seekOffset).Err; err != nil { - p.logger.Warn("mpris: setPosition seek failed", zap.Int64("target", *setPos), zap.Error(err)) + p.logger.Warn("mpris: setPosition seek failed", log.Int64("target", *setPos), log.Error(err)) } } if volume != nil { volF := float64(*volume) / 100.0 if err := dbusCall(obj, "org.freedesktop.DBus.Properties.Set", "org.mpris.MediaPlayer2.Player", "Volume", dbus.MakeVariant(volF)).Err; err != nil { - p.logger.Warn("mpris: setVolume failed", zap.Float64("volume", volF), zap.Error(err)) + p.logger.Warn("mpris: setVolume failed", log.Float64("volume", volF), log.Error(err)) } } if shuffle != nil { if err := dbusCall(obj, "org.freedesktop.DBus.Properties.Set", "org.mpris.MediaPlayer2.Player", "Shuffle", dbus.MakeVariant(*shuffle)).Err; err != nil { - p.logger.Warn("mpris: setShuffle failed", zap.Bool("shuffle", *shuffle), zap.Error(err)) + p.logger.Warn("mpris: setShuffle failed", log.Bool("shuffle", *shuffle), log.Error(err)) } } if loopStatus != "" { if err := dbusCall(obj, "org.freedesktop.DBus.Properties.Set", "org.mpris.MediaPlayer2.Player", "LoopStatus", dbus.MakeVariant(loopStatus)).Err; err != nil { - p.logger.Warn("mpris: setLoopStatus failed", zap.String("loopStatus", loopStatus), zap.Error(err)) + p.logger.Warn("mpris: setLoopStatus failed", log.String("loopStatus", loopStatus), log.Error(err)) } } diff --git a/internal/plugins/mpris/reconcile.go b/internal/plugins/mpris/reconcile.go new file mode 100644 index 0000000..5aa5471 --- /dev/null +++ b/internal/plugins/mpris/reconcile.go @@ -0,0 +1,104 @@ +package mpris + +import ( + "github.com/bethropolis/kcd/internal/log" + "github.com/godbus/dbus/v5" +) + +// diffTracked compares live bus names against tracked players. +// tracked maps busName -> displayName. It returns entries present on the +// bus but untracked (to add) and bus names tracked but no longer owned +// (to drop). Pure function for testability; all D-Bus I/O stays in +// reconcilePlayers. +func diffTracked(entries []playerEntry, tracked map[string]string) (add []playerEntry, drop []string) { + owned := make(map[string]struct{}, len(entries)) + for _, e := range entries { + owned[e.busName] = struct{}{} + if _, ok := tracked[e.busName]; !ok { + add = append(add, e) + } + } + for busName := range tracked { + if _, ok := owned[busName]; !ok { + drop = append(drop, busName) + } + } + return add, drop +} + +// reconcilePlayers heals drift between the session bus and p.players. +// Missing players are added through the same path as the signal handler +// (match rules + broadcast so the phone learns about them); ghosts are +// removed. A removal only fires if the tracked entry still points at the +// dead bus name, so a same-named live player (duplicate Identity, e.g. two +// Firefox profiles) is never evicted by its dead twin. +func (p *MPRISPlugin) reconcilePlayers(conn *dbus.Conn, uniqueToDisplay map[string]string) { + if p.dbus == nil || conn == nil { + return + } + entries, err := listPlayersDBus(p.dbus) + if err != nil { + // Transient failure: never prune on a failed listing. + p.logger.Debug("mpris: reconcile listing failed", log.Error(err)) + return + } + + p.mu.RLock() + tracked := make(map[string]string, len(p.players)) + for display, pl := range p.players { + tracked[pl.busName] = display + } + p.mu.RUnlock() + + add, drop := diffTracked(entries, tracked) + if len(add) == 0 && len(drop) == 0 { + return + } + p.logger.Debug("mpris: reconciling players") + + for _, e := range add { + var owner string + if err := conn.BusObject().Call("org.freedesktop.DBus.GetNameOwner", 0, e.busName).Store(&owner); err != nil || owner == "" { + continue + } + if err := conn.AddMatchSignal( + dbus.WithMatchSender(e.busName), + dbus.WithMatchInterface("org.freedesktop.DBus.Properties"), + dbus.WithMatchMember("PropertiesChanged"), + ); err != nil { + p.logger.Warn("mpris: match rule rejected", log.String("busName", e.busName), log.Error(err)) + } + if err := conn.AddMatchSignal( + dbus.WithMatchSender(e.busName), + dbus.WithMatchInterface("org.mpris.MediaPlayer2.Player"), + dbus.WithMatchMember("Seeked"), + ); err != nil { + p.logger.Warn("mpris: match rule rejected", log.String("busName", e.busName), log.Error(err)) + } + uniqueToDisplay[owner] = e.identity + p.addPlayer(e.busName, owner, e.identity, e.shortName) + } + + for _, busName := range drop { + p.mu.RLock() + display, ok := tracked[busName] + current, stillTracked := p.players[display] + p.mu.RUnlock() + if !ok || !stillTracked || current.busName != busName { + continue + } + p.removePlayer(display) + // Drop stale owner mappings only when no same-named live player + // remains; otherwise the live owner's routing must stay intact. + p.mu.RLock() + _, displayAlive := p.players[display] + p.mu.RUnlock() + if !displayAlive { + for owner, mapped := range uniqueToDisplay { + if mapped == display { + delete(uniqueToDisplay, owner) + } + } + } + } +} diff --git a/internal/plugins/mpris/reconcile_test.go b/internal/plugins/mpris/reconcile_test.go new file mode 100644 index 0000000..098598d --- /dev/null +++ b/internal/plugins/mpris/reconcile_test.go @@ -0,0 +1,70 @@ +package mpris + +import ( + "testing" +) + +func TestDiffTrackedAddsMissing(t *testing.T) { + entries := []playerEntry{ + {busName: "org.mpris.MediaPlayer2.firefox.instance_1_99", shortName: "firefox", identity: "Mozilla firefox"}, + {busName: "org.mpris.MediaPlayer2.spotify", shortName: "spotify", identity: "Spotify"}, + } + add, drop := diffTracked(entries, map[string]string{}) + if len(add) != 2 || len(drop) != 0 { + t.Fatalf("add=%d drop=%d, want add=2 drop=0", len(add), len(drop)) + } +} + +func TestDiffTrackedSteadyState(t *testing.T) { + entries := []playerEntry{ + {busName: "org.mpris.MediaPlayer2.spotify", shortName: "spotify", identity: "Spotify"}, + } + tracked := map[string]string{"org.mpris.MediaPlayer2.spotify": "Spotify"} + add, drop := diffTracked(entries, tracked) + if len(add) != 0 || len(drop) != 0 { + t.Fatalf("add=%d drop=%d, want no-op", len(add), len(drop)) + } +} + +func TestDiffTrackedDropsReleased(t *testing.T) { + tracked := map[string]string{"org.mpris.MediaPlayer2.spotify": "Spotify"} + add, drop := diffTracked(nil, tracked) + if len(add) != 0 || len(drop) != 1 || drop[0] != "org.mpris.MediaPlayer2.spotify" { + t.Fatalf("add=%v drop=%v, want drop=[spotify bus]", add, drop) + } +} + +// Instance churn: old Firefox instance released, new one acquired. +// Both directions must be reported so the map converges. +func TestDiffTrackedInstanceChurn(t *testing.T) { + entries := []playerEntry{ + {busName: "org.mpris.MediaPlayer2.firefox.instance_1_99", shortName: "firefox", identity: "Mozilla firefox"}, + } + tracked := map[string]string{"org.mpris.MediaPlayer2.firefox.instance_1_98": "Mozilla firefox"} + add, drop := diffTracked(entries, tracked) + if len(add) != 1 || add[0].busName != "org.mpris.MediaPlayer2.firefox.instance_1_99" { + t.Fatalf("add=%v, want [instance_1_99]", add) + } + if len(drop) != 1 || drop[0] != "org.mpris.MediaPlayer2.firefox.instance_1_98" { + t.Fatalf("drop=%v, want [instance_1_98]", drop) + } +} + +// Two live players sharing one Identity are tracked independently by bus +// name, so evicting one never implies evicting the other. +func TestDiffTrackedDuplicateIdentity(t *testing.T) { + entries := []playerEntry{ + {busName: "org.mpris.MediaPlayer2.firefox.instance_1_99", shortName: "firefox", identity: "Mozilla firefox"}, + } + tracked := map[string]string{ + "org.mpris.MediaPlayer2.firefox.instance_1_98": "Mozilla firefox", + "org.mpris.MediaPlayer2.firefox.instance_1_99": "Mozilla firefox", + } + add, drop := diffTracked(entries, tracked) + if len(add) != 0 { + t.Fatalf("add=%v, want none (1_99 already tracked)", add) + } + if len(drop) != 1 || drop[0] != "org.mpris.MediaPlayer2.firefox.instance_1_98" { + t.Fatalf("drop=%v, want only [instance_1_98]", drop) + } +} diff --git a/internal/plugins/mpris/remote.go b/internal/plugins/mpris/remote.go index 3e27c83..8c3a940 100644 --- a/internal/plugins/mpris/remote.go +++ b/internal/plugins/mpris/remote.go @@ -6,8 +6,8 @@ import ( "time" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // SendAction sends a media control action to a remote device. @@ -167,7 +167,8 @@ func (p *MPRISPlugin) ActivePlayers() []string { } func (p *MPRISPlugin) OnConnect(dev device.Sender) { - p.logger.Info("mpris: device connected, requesting player list", zap.String("device_id", dev.ID())) + p.logger.Info("mpris: device connected, requesting player list", log.String("device_id", dev.ID())) + p.requestReconcile() go p.requestPlayerListPeriodic(dev) } @@ -237,7 +238,7 @@ func (p *MPRISPlugin) pollRemoteStates() { for _, t := range targets { if err := p.requestPlayerStatus(t.dev, t.player); err != nil { - p.logger.Debug("mpris: state poll request failed", zap.Error(err)) + p.logger.Debug("mpris: state poll request failed", log.Error(err)) } } } diff --git a/internal/plugins/mpris/signals.go b/internal/plugins/mpris/signals.go index 7bf71e8..d9b87c9 100644 --- a/internal/plugins/mpris/signals.go +++ b/internal/plugins/mpris/signals.go @@ -4,9 +4,9 @@ import ( "strings" "time" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" "github.com/godbus/dbus/v5" - "go.uber.org/zap" ) func (p *MPRISPlugin) handleNameOwnerChanged(sig *dbus.Signal, conn *dbus.Conn, uniqueToDisplay map[string]string) { @@ -38,16 +38,20 @@ func (p *MPRISPlugin) handleNameOwnerChanged(sig *dbus.Signal, conn *dbus.Conn, return } - _ = conn.AddMatchSignal( + if err := conn.AddMatchSignal( dbus.WithMatchSender(name), dbus.WithMatchInterface("org.freedesktop.DBus.Properties"), dbus.WithMatchMember("PropertiesChanged"), - ) - _ = conn.AddMatchSignal( + ); err != nil { + p.logger.Warn("mpris: match rule rejected", log.String("busName", name), log.Error(err)) + } + if err := conn.AddMatchSignal( dbus.WithMatchSender(name), dbus.WithMatchInterface("org.mpris.MediaPlayer2.Player"), dbus.WithMatchMember("Seeked"), - ) + ); err != nil { + p.logger.Warn("mpris: match rule rejected", log.String("busName", name), log.Error(err)) + } uniqueToDisplay[newOwner] = entry.identity p.addPlayer(entry.busName, newOwner, entry.identity, entry.shortName) @@ -60,6 +64,9 @@ func (p *MPRISPlugin) handleSeeked(sig *dbus.Signal, uniqueToDisplay map[string] } displayName, ok := uniqueToDisplay[string(sig.Sender)] if !ok { + // Seek from an untracked player: drift evidence, heal it. The + // position itself is dropped — the re-list follow-up resyncs. + p.requestReconcile() return } pos, ok := sig.Body[0].(int64) @@ -89,7 +96,9 @@ func (p *MPRISPlugin) handlePropertiesChanged(sig *dbus.Signal, uniqueToDisplay } } if displayName == "" { - p.logger.Debug("mpris: signal dropped, unknown sender", zap.String("sender", string(sig.Sender)), zap.String("signal", sig.Name)) + p.logger.Debug("mpris: signal dropped, unknown sender", log.String("sender", string(sig.Sender)), log.String("signal", sig.Name)) + // A signal from an untracked player is proof of drift — heal it. + p.requestReconcile() return } diff --git a/internal/plugins/mpris/watcher.go b/internal/plugins/mpris/watcher.go index 70b7d85..0c9368a 100644 --- a/internal/plugins/mpris/watcher.go +++ b/internal/plugins/mpris/watcher.go @@ -4,8 +4,8 @@ import ( "context" "time" + "github.com/bethropolis/kcd/internal/log" "github.com/godbus/dbus/v5" - "go.uber.org/zap" ) func (p *MPRISPlugin) startWatcher(ctx context.Context) { @@ -20,7 +20,7 @@ func (p *MPRISPlugin) startWatcher(ctx context.Context) { return } if err := p.runDBusWatcher(ctx); err != nil && ctx.Err() == nil { - p.logger.Warn("mpris: D-Bus watcher exited, restarting in 3s", zap.Error(err)) + p.logger.Warn("mpris: D-Bus watcher exited, restarting in 3s", log.Error(err)) select { case <-time.After(3 * time.Second): case <-ctx.Done(): @@ -101,24 +101,30 @@ func (p *MPRISPlugin) runDBusWatcher(ctx context.Context) error { var owner string if err := conn.BusObject().Call("org.freedesktop.DBus.GetNameOwner", 0, e.busName).Store(&owner); err == nil { uniqueToDisplay[owner] = e.identity - _ = conn.AddMatchSignal( + if err := conn.AddMatchSignal( dbus.WithMatchSender(e.busName), dbus.WithMatchInterface("org.freedesktop.DBus.Properties"), dbus.WithMatchMember("PropertiesChanged"), - ) - _ = conn.AddMatchSignal( + ); err != nil { + p.logger.Warn("mpris: match rule rejected", log.String("busName", e.busName), log.Error(err)) + } + if err := conn.AddMatchSignal( dbus.WithMatchSender(e.busName), dbus.WithMatchInterface("org.mpris.MediaPlayer2.Player"), dbus.WithMatchMember("Seeked"), - ) + ); err != nil { + p.logger.Warn("mpris: match rule rejected", log.String("busName", e.busName), log.Error(err)) + } p.addPlayer(e.busName, owner, e.identity, e.shortName) } } - _ = conn.AddMatchSignal( + if err := conn.AddMatchSignal( dbus.WithMatchInterface("org.freedesktop.DBus"), dbus.WithMatchMember("NameOwnerChanged"), - ) + ); err != nil { + return err + } ch := make(chan *dbus.Signal, 64) conn.Signal(ch) @@ -127,6 +133,8 @@ func (p *MPRISPlugin) runDBusWatcher(ctx context.Context) error { select { case <-ctx.Done(): return nil + case <-p.reconcileCh: + p.reconcilePlayers(conn, uniqueToDisplay) case sig := <-ch: if sig == nil { continue diff --git a/internal/plugins/notification/icon.go b/internal/plugins/notification/icon.go index a3a4300..f1c923b 100644 --- a/internal/plugins/notification/icon.go +++ b/internal/plugins/notification/icon.go @@ -10,8 +10,8 @@ import ( "regexp" "strings" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) // notifIDChars restricts phone-provided notification IDs to filename-safe @@ -62,7 +62,7 @@ func (p *NotificationPlugin) fetchIcon( // sanitizer above ever regresses. if rel, err := filepath.Rel(p.iconDir, iconPath); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { p.logger.Warn("notification: icon path escapes cache dir, refusing", - zap.String("id", notifID)) + log.String("id", notifID)) return "" } @@ -80,7 +80,7 @@ func (p *NotificationPlugin) fetchIcon( conn, err := transport.DialSidechannel(ctx, remoteIP, port, p.tlsConfig, expectedFP, p.logger, p.sidechannel) if err != nil { - p.logger.Debug("notification: icon dial failed", zap.Error(err)) + p.logger.Debug("notification: icon dial failed", log.Error(err)) return "" } defer conn.Close() @@ -92,7 +92,7 @@ func (p *NotificationPlugin) fetchIcon( defer f.Close() if _, err := io.Copy(f, io.LimitReader(conn, size)); err != nil { - p.logger.Debug("notification: icon download failed", zap.Error(err)) + p.logger.Debug("notification: icon download failed", log.Error(err)) _ = os.Remove(iconPath) return "" } diff --git a/internal/plugins/notification/notification_test.go b/internal/plugins/notification/notification_test.go index 4813433..96feedc 100644 --- a/internal/plugins/notification/notification_test.go +++ b/internal/plugins/notification/notification_test.go @@ -19,13 +19,13 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap/zaptest" ) func newPlugin(t *testing.T) *NotificationPlugin { t.Helper() - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) bus := events.NewBus(logger) cfg := config.NotificationPluginConfig{} cfg.Defaults() @@ -41,7 +41,7 @@ func newPlugin(t *testing.T) *NotificationPlugin { func TestNotificationPlugin_Handle_Normal(t *testing.T) { p := newPlugin(t) - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) dev := device.NewDevice("dev1", "Test", "phone", logger) body := NotificationBody{ @@ -59,7 +59,7 @@ func TestNotificationPlugin_Handle_Cancel(t *testing.T) { p := newPlugin(t) // Immediate close — no grace debounce for this test. p.cfg.CancelGraceMS = 0 - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) dev := device.NewDevice("dev1", "Test", "phone", logger) // Store a fake desktop ID so the cancel path can look it up. @@ -82,7 +82,7 @@ func TestNotificationPlugin_Handle_Cancel(t *testing.T) { func TestNotificationPlugin_Handle_Silent(t *testing.T) { p := newPlugin(t) - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) dev := device.NewDevice("dev1", "Test", "phone", logger) body := NotificationBody{ @@ -142,7 +142,7 @@ func (f *fakeNotifier) lastCall() []string { func newFakePlugin(t *testing.T, replace bool) (*NotificationPlugin, *fakeNotifier) { t.Helper() - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) bus := events.NewBus(logger) cfg := config.NotificationPluginConfig{} cfg.Defaults() @@ -159,7 +159,7 @@ func newFakePlugin(t *testing.T, replace bool) (*NotificationPlugin, *fakeNotifi func TestNotificationPlugin_ReplaceByID(t *testing.T) { p, f := newFakePlugin(t, true) - dev := device.NewDevice("dev1", "Test", "phone", zaptest.NewLogger(t)) + dev := device.NewDevice("dev1", "Test", "phone", log.NewTest(t)) id := "0|com.arn.scrobble|0|com.msob7y.namida|10247" // First post — no replace, stores the printed desktop id. @@ -182,7 +182,7 @@ func TestNotificationPlugin_ReplaceByID(t *testing.T) { } // Same id from a different device must not replace this device's popup. - other := device.NewDevice("dev2", "Other", "phone", zaptest.NewLogger(t)) + other := device.NewDevice("dev2", "Other", "phone", log.NewTest(t)) p.sendDesktopNotification(other.ID(), "Pano Scrobbler", id, "Song", "Artist", "") if r := f.argFor(2, "-r"); r != "" { t.Fatalf("expected no replace for a different device, got -r %q", r) @@ -201,7 +201,7 @@ func TestNotificationPlugin_ReplaceByID(t *testing.T) { func TestNotificationPlugin_ReplaceByIDDisabled(t *testing.T) { p, f := newFakePlugin(t, false) - dev := device.NewDevice("dev1", "Test", "phone", zaptest.NewLogger(t)) + dev := device.NewDevice("dev1", "Test", "phone", log.NewTest(t)) id := "0|com.arn.scrobble|0|com.msob7y.namida|10247" // Seed a stored id — with replacement disabled it must be ignored. @@ -285,7 +285,7 @@ func TestFetchIconRefusesTraversal(t *testing.T) { func TestNotifySendEndsOptions(t *testing.T) { p, f := newFakePlugin(t, true) - dev := device.NewDevice("dev1", "Test", "phone", zaptest.NewLogger(t)) + dev := device.NewDevice("dev1", "Test", "phone", log.NewTest(t)) // A title starting with '-' must be passed after "--" so notify-send // can't misparse it as a flag. @@ -323,7 +323,7 @@ func TestNotifySendEndsOptions(t *testing.T) { } func TestNotificationPlugin_ShowIconsGating(t *testing.T) { - dev := device.NewDevice("dev1", "Test", "phone", zaptest.NewLogger(t)) + dev := device.NewDevice("dev1", "Test", "phone", log.NewTest(t)) // Default (show_icons = false): an explicit empty icon — popups render // icon-less and daemons won't derive a placeholder from the app name. @@ -380,7 +380,7 @@ func postAndCancel(t *testing.T, p *NotificationPlugin, dev device.Sender, id st func TestNotificationPlugin_CancelGrace_RePostWithinWindow(t *testing.T) { p, f := newFakePlugin(t, true) p.cfg.CancelGraceMS = 200 - dev := device.NewDevice("dev1", "Test", "phone", zaptest.NewLogger(t)) + dev := device.NewDevice("dev1", "Test", "phone", log.NewTest(t)) id := "0|com.arn.scrobble|0|com.msob7y.namida|10247" key := p.notifKey(dev.ID(), id) @@ -412,7 +412,7 @@ func TestNotificationPlugin_CancelGrace_RePostWithinWindow(t *testing.T) { func TestNotificationPlugin_CancelGrace_NoRePost(t *testing.T) { p, f := newFakePlugin(t, true) p.cfg.CancelGraceMS = 50 - dev := device.NewDevice("dev1", "Test", "phone", zaptest.NewLogger(t)) + dev := device.NewDevice("dev1", "Test", "phone", log.NewTest(t)) id := "0|com.arn.scrobble|0|com.msob7y.namida|10247" key := p.notifKey(dev.ID(), id) @@ -437,7 +437,7 @@ func TestNotificationPlugin_CancelGrace_NoRePost(t *testing.T) { func TestNotificationPlugin_CancelGraceDisabled(t *testing.T) { p, f := newFakePlugin(t, true) p.cfg.CancelGraceMS = 0 - dev := device.NewDevice("dev1", "Test", "phone", zaptest.NewLogger(t)) + dev := device.NewDevice("dev1", "Test", "phone", log.NewTest(t)) id := "0|com.arn.scrobble|0|com.msob7y.namida|10247" key := p.notifKey(dev.ID(), id) diff --git a/internal/plugins/notification/types.go b/internal/plugins/notification/types.go index 6bb8168..aa0d8dc 100644 --- a/internal/plugins/notification/types.go +++ b/internal/plugins/notification/types.go @@ -12,8 +12,9 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" + "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) // NotificationPlugin handles incoming notifications and displays them on the desktop. @@ -21,7 +22,7 @@ type NotificationPlugin struct { sidechannel transport.SidechannelOptions bus *events.Bus tlsConfig *tls.Config - logger *zap.Logger + logger log.Logger notifIDs sync.Map // maps deviceID|body.ID -> desktop notify-send ID (string) pendingCloses sync.Map // maps deviceID|body.ID -> *time.Timer (deferred close for cancel-grace) iconDir string // temp dir for cached notification icons @@ -35,7 +36,7 @@ type NotificationPlugin struct { // NewNotificationPlugin creates a NotificationPlugin. // tlsConfig is used to fetch notification icon payloads over the KDE Connect // side-channel; pass nil to disable icon fetching. -func NewNotificationPlugin(cfg config.NotificationPluginConfig, bus *events.Bus, tlsConfig *tls.Config, logger *zap.Logger, options ...transport.SidechannelOptions) *NotificationPlugin { +func NewNotificationPlugin(cfg config.NotificationPluginConfig, bus *events.Bus, tlsConfig *tls.Config, logger log.Logger, options ...transport.SidechannelOptions) *NotificationPlugin { var sidechannel transport.SidechannelOptions if len(options) > 0 { sidechannel = options[0] @@ -45,13 +46,15 @@ func NewNotificationPlugin(cfg config.NotificationPluginConfig, bus *events.Bus, cfg: cfg, bus: bus, tlsConfig: tlsConfig, - logger: logger.With(zap.String("plugin", "notification")), + logger: logger.With(log.String("plugin", "notification")), newExec: exec.CommandContext, } // Probe --print-id support by checking --help output. // This is side-effect-free and immune to version string format changes. - if out, err := exec.CommandContext(context.Background(), "notify-send", "--help").CombinedOutput(); err == nil { + probeCtx, probeCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer probeCancel() + if out, err := plugin.RunCommandSync(probeCtx, "notify-send", "--help"); err == nil { p.canCloseNotifs = strings.Contains(string(out), "--print-id") } diff --git a/internal/plugins/pair/actions.go b/internal/plugins/pair/actions.go index 740b142..e389ced 100644 --- a/internal/plugins/pair/actions.go +++ b/internal/plugins/pair/actions.go @@ -6,8 +6,8 @@ import ( "github.com/bethropolis/kcd/internal/cert" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // AcceptPairing accepts an incoming pair request. @@ -18,7 +18,7 @@ func (p *PairPlugin) AcceptPairing(dev *device.Device) error { } if err := dev.Send(pkt); err != nil { - p.logger.Error("failed to send pair accept", zap.Error(err)) + p.logger.Error("failed to send pair accept", log.Error(err)) dev.SetState(device.StateUnpaired) dev.ClearEphemeral() dev.ClearPairDial() @@ -32,7 +32,7 @@ func (p *PairPlugin) AcceptPairing(dev *device.Device) error { // RequestPairing initiates a pairing request to a device. func (p *PairPlugin) RequestPairing(dev *device.Device) error { if dev.State() == device.StatePaired { - p.logger.Warn("device already paired", zap.String("device_id", dev.ID())) + p.logger.Warn("device already paired", log.String("device_id", dev.ID())) return nil } @@ -54,7 +54,7 @@ func (p *PairPlugin) RequestPairing(dev *device.Device) error { p.mu.Unlock() if err := dev.Send(pkt); err != nil { - p.logger.Error("failed to send pair request", zap.Error(err)) + p.logger.Error("failed to send pair request", log.Error(err)) return err } @@ -62,12 +62,12 @@ func (p *PairPlugin) RequestPairing(dev *device.Device) error { if peerCert != nil { vKey := cert.VerificationKey(p.localCert, peerCert, timestamp) p.logger.Info("pairing verification code", - zap.String("device_id", dev.ID()), - zap.String("code", vKey)) + log.String("device_id", dev.ID()), + log.String("code", vKey)) } dev.SetState(device.StatePairRequested) - p.logger.Info("pair request sent", zap.String("device_id", dev.ID())) + p.logger.Info("pair request sent", log.String("device_id", dev.ID())) if p.onStateChanged != nil { p.onStateChanged() @@ -96,7 +96,7 @@ func (p *PairPlugin) RejectPairing(dev *device.Device) error { p.onStateChanged() } - p.logger.Info("pair request rejected", zap.String("device_id", dev.ID())) + p.logger.Info("pair request rejected", log.String("device_id", dev.ID())) return nil } @@ -120,7 +120,7 @@ func (p *PairPlugin) Unpair(dev *device.Device) error { p.onStateChanged() } - p.logger.Info("device unpaired", zap.String("device_id", dev.ID())) + p.logger.Info("device unpaired", log.String("device_id", dev.ID())) return nil } @@ -136,6 +136,6 @@ func (p *PairPlugin) pairingDone(dev *device.Device) { p.onStateChanged() } - p.logger.Info("pairing complete", zap.String("device_id", dev.ID())) + p.logger.Info("pairing complete", log.String("device_id", dev.ID())) p.emit(events.TypePairAccepted, dev, "") } diff --git a/internal/plugins/pair/handle.go b/internal/plugins/pair/handle.go index 019fce8..4f98277 100644 --- a/internal/plugins/pair/handle.go +++ b/internal/plugins/pair/handle.go @@ -8,8 +8,8 @@ import ( "github.com/bethropolis/kcd/internal/cert" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) func (p *PairPlugin) Handle(ctx context.Context, sender device.Sender, pkt *protocol.Packet) error { @@ -20,7 +20,7 @@ func (p *PairPlugin) Handle(ctx context.Context, sender device.Sender, pkt *prot dev, ok := p.devices.Get(sender.ID()) if !ok { - p.logger.Warn("pair packet from unknown device", zap.String("device_id", sender.ID())) + p.logger.Warn("pair packet from unknown device", log.String("device_id", sender.ID())) return nil } @@ -36,18 +36,18 @@ func (p *PairPlugin) handlePairRequest(_ context.Context, dev *device.Device, bo switch state { case device.StatePairRequested: // We requested pairing, they accepted - p.logger.Info("pairing accepted by peer", zap.String("device_id", dev.ID())) + p.logger.Info("pairing accepted by peer", log.String("device_id", dev.ID())) p.pairingDone(dev) case device.StatePairRequestedByPeer: // Already have a pending request, ignore duplicate - p.logger.Debug("ignoring duplicate pair request", zap.String("device_id", dev.ID())) + p.logger.Debug("ignoring duplicate pair request", log.String("device_id", dev.ID())) case device.StatePaired: // Already paired - this is normal behavior in KDE Connect. // The peer sends pair:true as confirmation/keep-alive. // Just acknowledge by sending pair:true back. - p.logger.Debug("received pair confirmation from already paired device", zap.String("device_id", dev.ID())) + p.logger.Debug("received pair confirmation from already paired device", log.String("device_id", dev.ID())) pkt, _ := protocol.NewPairPacket(protocol.PairAccept, 0) dev.Send(pkt) return nil @@ -60,9 +60,9 @@ func (p *PairPlugin) handlePairRequest(_ context.Context, dev *device.Device, bo diff := now - body.Timestamp if diff < -AllowedTimestampDiff || diff > AllowedTimestampDiff { p.logger.Warn("pair request timestamp out of range", - zap.String("device_id", dev.ID()), - zap.Int64("timestamp", body.Timestamp), - zap.Int64("now", now)) + log.String("device_id", dev.ID()), + log.Int64("timestamp", body.Timestamp), + log.Int64("now", now)) // Send rejection pkt, _ := protocol.NewPairPacket(protocol.PairReject, 0) dev.Send(pkt) @@ -74,15 +74,15 @@ func (p *PairPlugin) handlePairRequest(_ context.Context, dev *device.Device, bo p.mu.Unlock() } - p.logger.Info("incoming pair request", zap.String("device_id", dev.ID())) + p.logger.Info("incoming pair request", log.String("device_id", dev.ID())) var vKey string peerCert := dev.PeerCert() if peerCert != nil { vKey = cert.VerificationKey(p.localCert, peerCert, p.pairingTimestampFor(dev.ID())) p.logger.Info("pairing verification code", - zap.String("device_id", dev.ID()), - zap.String("code", vKey)) + log.String("device_id", dev.ID()), + log.String("code", vKey)) } // Set state and wait for user to accept via CLI @@ -103,7 +103,7 @@ func (p *PairPlugin) handleUnpairRequest(_ context.Context, dev *device.Device) switch state { case device.StatePairRequested: // We requested, they rejected - p.logger.Info("pair request rejected by peer", zap.String("device_id", dev.ID())) + p.logger.Info("pair request rejected by peer", log.String("device_id", dev.ID())) dev.SetState(device.StateUnpaired) dev.ClearEphemeral() dev.ClearPairDial() @@ -111,7 +111,7 @@ func (p *PairPlugin) handleUnpairRequest(_ context.Context, dev *device.Device) case device.StatePairRequestedByPeer: // They requested, then cancelled - p.logger.Info("pair request cancelled by peer", zap.String("device_id", dev.ID())) + p.logger.Info("pair request cancelled by peer", log.String("device_id", dev.ID())) dev.SetState(device.StateUnpaired) dev.ClearEphemeral() dev.ClearPairDial() @@ -119,14 +119,14 @@ func (p *PairPlugin) handleUnpairRequest(_ context.Context, dev *device.Device) case device.StatePaired: // Unpair request - p.logger.Info("unpair request received", zap.String("device_id", dev.ID())) + p.logger.Info("unpair request received", log.String("device_id", dev.ID())) dev.SetState(device.StateUnpaired) dev.ClearEphemeral() dev.ClearPairDial() case device.StateUnpaired, device.StateUnknown: // Already unpaired, ignore - p.logger.Debug("ignoring unpair request for unpaired device", zap.String("device_id", dev.ID())) + p.logger.Debug("ignoring unpair request for unpaired device", log.String("device_id", dev.ID())) } // Clean up stored timestamp diff --git a/internal/plugins/pair/types.go b/internal/plugins/pair/types.go index 8a81525..8787c8e 100644 --- a/internal/plugins/pair/types.go +++ b/internal/plugins/pair/types.go @@ -8,8 +8,8 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) const ( @@ -22,7 +22,7 @@ type PairPlugin struct { devices *device.Registry localCert *x509.Certificate onStateChanged func() // callback to persist state - logger *zap.Logger + logger log.Logger bus *events.Bus cfg config.PairingConfig @@ -31,7 +31,7 @@ type PairPlugin struct { } // NewPairPlugin creates a new pairing plugin. -func NewPairPlugin(devices *device.Registry, localCert *x509.Certificate, cfg config.PairingConfig, onStateChanged func(), bus *events.Bus, logger *zap.Logger) *PairPlugin { +func NewPairPlugin(devices *device.Registry, localCert *x509.Certificate, cfg config.PairingConfig, onStateChanged func(), bus *events.Bus, logger log.Logger) *PairPlugin { return &PairPlugin{ devices: devices, localCert: localCert, diff --git a/internal/plugins/ping/ping.go b/internal/plugins/ping/ping.go index 99c6852..e63f65a 100644 --- a/internal/plugins/ping/ping.go +++ b/internal/plugins/ping/ping.go @@ -8,18 +8,18 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) type PingPlugin struct { cfg config.PingConfig bus *events.Bus - logger *zap.Logger + logger log.Logger } -func NewPingPlugin(cfg config.PingConfig, bus *events.Bus, logger *zap.Logger, notifications ...config.NotificationConfig) *PingPlugin { +func NewPingPlugin(cfg config.PingConfig, bus *events.Bus, logger log.Logger, notifications ...config.NotificationConfig) *PingPlugin { if cfg.AppName == "" { var notificationCfg config.NotificationConfig if len(notifications) > 0 { @@ -30,7 +30,7 @@ func NewPingPlugin(cfg config.PingConfig, bus *events.Bus, logger *zap.Logger, n return &PingPlugin{ cfg: cfg, bus: bus, - logger: logger.With(zap.String("plugin", "ping")), + logger: logger.With(log.String("plugin", "ping")), } } diff --git a/internal/plugins/presenter/presenter.go b/internal/plugins/presenter/presenter.go index 440c0bc..a941ce0 100644 --- a/internal/plugins/presenter/presenter.go +++ b/internal/plugins/presenter/presenter.go @@ -8,21 +8,21 @@ import ( "context" "encoding/json" "fmt" - "os/exec" "strconv" "strings" "sync" "time" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" + "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // PresenterPlugin handles kdeconnect.presenter packets containing // gyroscope-based pointer deltas (dx/dy) from the Android presenter remote. type PresenterPlugin struct { - logger *zap.Logger + logger log.Logger mu sync.Mutex xPos float64 @@ -42,9 +42,9 @@ type PresenterBody struct { } // NewPresenterPlugin creates a new presenter remote plugin. -func NewPresenterPlugin(logger *zap.Logger) *PresenterPlugin { +func NewPresenterPlugin(logger log.Logger) *PresenterPlugin { p := &PresenterPlugin{ - logger: logger.With(zap.String("plugin", "Presenter")), + logger: logger.With(log.String("plugin", "Presenter")), xPos: 0.5, yPos: 0.5, moveCh: make(chan PresenterBody, 1), @@ -138,7 +138,10 @@ func (p *PresenterPlugin) handleBody(body PresenterBody) { // getScreenSize attempts to detect the screen resolution via xdpyinfo (X11) // or wlr-randr (Wayland). Returns 0x0 if neither is available. func getScreenSize() (int, int) { - out, err := exec.CommandContext(context.Background(), "xdpyinfo").Output() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + out, err := plugin.RunCommandOutput(ctx, "xdpyinfo") if err == nil { for _, line := range strings.Split(string(out), "\n") { if strings.Contains(line, "dimensions:") { @@ -150,7 +153,7 @@ func getScreenSize() (int, int) { } } - out, err = exec.CommandContext(context.Background(), "wlr-randr").Output() + out, err = plugin.RunCommandOutput(ctx, "wlr-randr") if err == nil { for _, line := range strings.Split(string(out), "\n") { line = strings.TrimSpace(line) @@ -169,10 +172,12 @@ func getScreenSize() (int, int) { // moveCursor moves the system cursor to the given absolute screen coordinates. // Prefers ydotool (Wayland) with xdotool as fallback (X11). func moveCursor(x, y int) { - cmd := exec.CommandContext(context.Background(), "ydotool", "mousemove", "-x", strconv.Itoa(x), "-y", strconv.Itoa(y)) - if err := cmd.Run(); err == nil { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + if _, err := plugin.RunCommandSync(ctx, "ydotool", "mousemove", "-x", strconv.Itoa(x), "-y", strconv.Itoa(y)); err == nil { return } - exec.CommandContext(context.Background(), "xdotool", "mousemove", strconv.Itoa(x), strconv.Itoa(y)).Run() + _, _ = plugin.RunCommandSync(ctx, "xdotool", "mousemove", strconv.Itoa(x), strconv.Itoa(y)) } diff --git a/internal/plugins/remotesystemvolume/remotesystemvolume.go b/internal/plugins/remotesystemvolume/remotesystemvolume.go index 235d3a5..7822a17 100644 --- a/internal/plugins/remotesystemvolume/remotesystemvolume.go +++ b/internal/plugins/remotesystemvolume/remotesystemvolume.go @@ -8,8 +8,8 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) type VolumeBody struct { @@ -30,14 +30,14 @@ type SinkInfo struct { } type RemoteSystemVolumePlugin struct { - logger *zap.Logger + logger log.Logger bus *events.Bus sinks sync.Map // device ID -> []SinkInfo } -func NewRemoteSystemVolumePlugin(bus *events.Bus, logger *zap.Logger) *RemoteSystemVolumePlugin { +func NewRemoteSystemVolumePlugin(bus *events.Bus, logger log.Logger) *RemoteSystemVolumePlugin { return &RemoteSystemVolumePlugin{ - logger: logger.With(zap.String("plugin", "remotesystemvolume")), + logger: logger.With(log.String("plugin", "remotesystemvolume")), bus: bus, } } @@ -135,8 +135,8 @@ func (p *RemoteSystemVolumePlugin) OnConnect(dev device.Sender) { go func() { if err := p.RequestSinkList(dev); err != nil { p.logger.Warn("failed to request sink list", - zap.String("device", dev.ID()), - zap.Error(err), + log.String("device", dev.ID()), + log.Error(err), ) } }() diff --git a/internal/plugins/runcommand/runcommand.go b/internal/plugins/runcommand/runcommand.go index 19ff21c..3c2f206 100644 --- a/internal/plugins/runcommand/runcommand.go +++ b/internal/plugins/runcommand/runcommand.go @@ -9,9 +9,9 @@ import ( "time" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // RunCommandPlugin allows remote devices to trigger pre-configured local commands. @@ -19,18 +19,18 @@ type RunCommandPlugin struct { Mu sync.RWMutex // exported so daemon.go can lock it during reload Commands map[string]string CommandsPerDevice map[string]map[string]string // keyed by device ID - logger *zap.Logger + logger log.Logger wg sync.WaitGroup // exported for tests to synchronize with background goroutines } -func NewRunCommandPlugin(commands map[string]string, commandsPerDevice map[string]map[string]string, logger *zap.Logger) *RunCommandPlugin { +func NewRunCommandPlugin(commands map[string]string, commandsPerDevice map[string]map[string]string, logger log.Logger) *RunCommandPlugin { if commandsPerDevice == nil { commandsPerDevice = make(map[string]map[string]string) } return &RunCommandPlugin{ Commands: commands, CommandsPerDevice: commandsPerDevice, - logger: logger.With(zap.String("plugin", "runcommand")), + logger: logger.With(log.String("plugin", "runcommand")), } } @@ -133,7 +133,7 @@ func (p *RunCommandPlugin) Handle(ctx context.Context, dev device.Sender, pkt *p // Do not send notifications for massive outputs (e.g. log dumps) if len(text) > 4096 { - p.logger.Warn("command output too large for notification, truncating", zap.Int("len", len(text))) + p.logger.Warn("command output too large for notification, truncating", log.Int("len", len(text))) text = text[:4000] + "\n...[output truncated]" } @@ -149,14 +149,14 @@ func (p *RunCommandPlugin) Handle(ctx context.Context, dev device.Sender, pkt *p } p.logger.Debug("sending command output notification", - zap.String("key", body.Key), - zap.Int("output_len", len(text)), + log.String("key", body.Key), + log.Int("output_len", len(text)), ) if pkt, err := protocol.NewPacket("kdeconnect.notification", notifBody); err == nil { if err := dev.Send(pkt); err != nil { p.logger.Warn("failed to send command output notification", - zap.String("key", body.Key), - zap.Error(err), + log.String("key", body.Key), + log.Error(err), ) } } diff --git a/internal/plugins/runcommand/runcommand_test.go b/internal/plugins/runcommand/runcommand_test.go index 4069251..5378602 100644 --- a/internal/plugins/runcommand/runcommand_test.go +++ b/internal/plugins/runcommand/runcommand_test.go @@ -5,12 +5,12 @@ import ( "testing" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap/zaptest" ) func TestRunCommandPlugin_Handle_GlobalCommand(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) dev := device.NewDevice("dev1", "Test", "phone", logger) p := NewRunCommandPlugin( map[string]string{"key1": "echo test"}, @@ -26,7 +26,7 @@ func TestRunCommandPlugin_Handle_GlobalCommand(t *testing.T) { } func TestRunCommandPlugin_Handle_PerDeviceOverrides(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) p := NewRunCommandPlugin( map[string]string{"cmd": "echo global"}, @@ -54,7 +54,7 @@ func TestRunCommandPlugin_Handle_PerDeviceOverrides(t *testing.T) { } func TestRunCommandPlugin_Handle_RequestCommandList(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) dev := device.NewDevice("dev1", "Test", "phone", logger) p := NewRunCommandPlugin( map[string]string{"global": "echo global"}, diff --git a/internal/plugins/sftp/handle.go b/internal/plugins/sftp/handle.go index 67ff5c6..59e8627 100644 --- a/internal/plugins/sftp/handle.go +++ b/internal/plugins/sftp/handle.go @@ -7,8 +7,8 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) func (p *SftpPlugin) Handle(_ context.Context, dev device.Sender, pkt *protocol.Packet) error { @@ -19,8 +19,8 @@ func (p *SftpPlugin) Handle(_ context.Context, dev device.Sender, pkt *protocol. if body.ErrorMessage != "" { p.logger.Warn("SFTP server error from device", - zap.String("device_id", dev.ID()), - zap.String("error", body.ErrorMessage), + log.String("device_id", dev.ID()), + log.String("error", body.ErrorMessage), ) if p.bus != nil { p.bus.Publish(events.TypeSftpMount, dev.ID(), map[string]interface{}{ @@ -35,7 +35,7 @@ func (p *SftpPlugin) Handle(_ context.Context, dev device.Sender, pkt *protocol. p.mu.Unlock() safeURI := fmt.Sprintf("sftp://%s@%s:%s%s", body.User, body.IP, body.Port.String(), body.Path) - p.logger.Info("SFTP server available", zap.String("uri", safeURI)) + p.logger.Info("SFTP server available", log.String("uri", safeURI)) evtPayload := map[string]interface{}{ "uri": fmt.Sprintf("sftp://%s:%s@%s:%s%s", body.User, body.Password, body.IP, body.Port.String(), body.Path), diff --git a/internal/plugins/sftp/mount.go b/internal/plugins/sftp/mount.go index 127d612..d55beab 100644 --- a/internal/plugins/sftp/mount.go +++ b/internal/plugins/sftp/mount.go @@ -14,7 +14,8 @@ import ( "time" "github.com/bethropolis/kcd/internal/device" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" + "github.com/bethropolis/kcd/internal/plugin" ) // sshUserPattern allows the generated Android SFTP usernames (alphanumerics, @@ -142,14 +143,14 @@ func (p *SftpPlugin) mountWithBody(ctx context.Context, deviceID string, body Sf p.mu.Lock() p.mountPIDs[deviceID] = pid p.mu.Unlock() - p.logger.Debug("tracking sshfs PID", zap.Int("pid", pid)) + p.logger.Debug("tracking sshfs PID", log.Int("pid", pid)) } else { - p.logger.Debug("could not find sshfs PID", zap.Error(err)) + p.logger.Debug("could not find sshfs PID", log.Error(err)) } p.logger.Info("SFTP mounted", - zap.String("mount_point", mountPoint), - zap.String("browse_path", browsePath), + log.String("mount_point", mountPoint), + log.String("browse_path", browsePath), ) // Open in the default file manager (best effort, non-blocking). @@ -160,7 +161,7 @@ func (p *SftpPlugin) mountWithBody(ctx context.Context, deviceID string, body Sf cmd = "xdg-open" } if err := exec.CommandContext(context.Background(), cmd, browsePath).Start(); err != nil { - p.logger.Debug("auto-open failed", zap.String("command", cmd), zap.Error(err)) + p.logger.Debug("auto-open failed", log.String("command", cmd), log.Error(err)) } }() } @@ -178,12 +179,12 @@ func (p *SftpPlugin) OnDisconnect(dev device.Sender) { if mounted { p.logger.Info("device disconnected, cleaning up SFTP mount", - zap.String("device_id", deviceID), + log.String("device_id", deviceID), ) if err := p.Unmount(deviceID); err != nil { p.logger.Warn("failed to unmount on disconnect", - zap.String("device_id", deviceID), - zap.Error(err), + log.String("device_id", deviceID), + log.Error(err), ) } } @@ -214,11 +215,11 @@ func (p *SftpPlugin) Unmount(deviceID string) error { return fmt.Errorf("no active SFTP mount for device %s", deviceID) } - p.logger.Info("unmounting SFTP share", zap.String("mount_point", mountPoint)) + p.logger.Info("unmounting SFTP share", log.String("mount_point", mountPoint)) // Graceful shutdown: SIGTERM → wait → SIGKILL. if hasPID { - p.logger.Debug("sending SIGTERM to sshfs", zap.Int("pid", pid)) + p.logger.Debug("sending SIGTERM to sshfs", log.Int("pid", pid)) proc, err := os.FindProcess(pid) if err == nil { if err := proc.Signal(syscall.SIGTERM); err == nil { @@ -238,22 +239,24 @@ func (p *SftpPlugin) Unmount(deviceID string) error { } } - // Ensure the mount point is released. + // Ensure the mount point is released (bounded: a wedged FUSE mount + // must not hang Unmount forever). tool := "fusermount3" if _, err := exec.LookPath(tool); err != nil { tool = "fusermount" } - - if out, err := exec.CommandContext(context.Background(), tool, "-u", mountPoint).CombinedOutput(); err != nil { + unmountCtx, unmountCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer unmountCancel() + if out, err := plugin.RunCommandSync(unmountCtx, tool, "-u", mountPoint); err != nil { p.logger.Warn("fusermount cleanup failed", - zap.String("mount_point", mountPoint), - zap.Error(err), - zap.String("output", strings.TrimSpace(string(out))), + log.String("mount_point", mountPoint), + log.Error(err), + log.String("output", strings.TrimSpace(string(out))), ) } _ = os.Remove(mountPoint) - p.logger.Info("SFTP unmounted", zap.String("mount_point", mountPoint)) + p.logger.Info("SFTP unmounted", log.String("mount_point", mountPoint)) return nil } diff --git a/internal/plugins/sftp/request.go b/internal/plugins/sftp/request.go index 70dff91..b3fd556 100644 --- a/internal/plugins/sftp/request.go +++ b/internal/plugins/sftp/request.go @@ -7,8 +7,8 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // RequestMount sends a kdeconnect.sftp.request packet asking the device to @@ -39,7 +39,7 @@ func (p *SftpPlugin) RequestAndMount(ctx context.Context, dev device.Sender) (st return "", fmt.Errorf("send SFTP request: %w", err) } - p.logger.Info("SFTP request sent, waiting for phone response", zap.String("device", dev.ID())) + p.logger.Info("SFTP request sent, waiting for phone response", log.String("device", dev.ID())) timeout := time.Duration(p.cfg.CredentialsTimeoutSecs) * time.Second if timeout == 0 { @@ -87,7 +87,7 @@ func (p *SftpPlugin) RequestAndMountVolume(ctx context.Context, dev device.Sende return "", nil, fmt.Errorf("send SFTP request: %w", err) } - p.logger.Info("SFTP request sent, waiting for phone response", zap.String("device", dev.ID())) + p.logger.Info("SFTP request sent, waiting for phone response", log.String("device", dev.ID())) timeout := time.Duration(p.cfg.CredentialsTimeoutSecs) * time.Second if timeout == 0 { diff --git a/internal/plugins/sftp/types.go b/internal/plugins/sftp/types.go index 0d49f94..2625d4a 100644 --- a/internal/plugins/sftp/types.go +++ b/internal/plugins/sftp/types.go @@ -7,25 +7,25 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/events" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) // SftpPlugin handles KDE Connect SFTP negotiation and optional sshfs mounting. type SftpPlugin struct { cfg config.SFTPConfig bus *events.Bus - logger *zap.Logger + logger log.Logger mu sync.RWMutex lastBody map[string]SftpBody mountPoints map[string]string // deviceID -> local mountPoint path mountPIDs map[string]int // deviceID -> sshfs PID for graceful shutdown } -func NewSftpPlugin(cfg config.SFTPConfig, bus *events.Bus, logger *zap.Logger) *SftpPlugin { +func NewSftpPlugin(cfg config.SFTPConfig, bus *events.Bus, logger log.Logger) *SftpPlugin { return &SftpPlugin{ cfg: cfg, bus: bus, - logger: logger.With(zap.String("plugin", "sftp")), + logger: logger.With(log.String("plugin", "sftp")), lastBody: make(map[string]SftpBody), mountPoints: make(map[string]string), mountPIDs: make(map[string]int), diff --git a/internal/plugins/share/handle.go b/internal/plugins/share/handle.go index 15a44ec..051d5cb 100644 --- a/internal/plugins/share/handle.go +++ b/internal/plugins/share/handle.go @@ -14,9 +14,9 @@ import ( "github.com/bethropolis/kcd/internal/cert" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) func (p *SharePlugin) Handle(ctx context.Context, dev device.Sender, pkt *protocol.Packet) error { @@ -26,7 +26,7 @@ func (p *SharePlugin) Handle(ctx context.Context, dev device.Sender, pkt *protoc } if body.Text != "" && pkt.PayloadSize <= 0 { - p.Logger.Info("share: received text", zap.String("text", body.Text)) + p.Logger.Info("share: received text", log.String("text", body.Text)) if p.bus != nil { p.bus.Publish(events.TypeShareText, dev.ID(), map[string]string{"text": body.Text}) } @@ -44,7 +44,7 @@ func (p *SharePlugin) Handle(ctx context.Context, dev device.Sender, pkt *protoc } if body.Url != "" && pkt.PayloadSize <= 0 { - p.Logger.Info("share: received url", zap.String("url", body.Url)) + p.Logger.Info("share: received url", log.String("url", body.Url)) if p.bus != nil { p.bus.Publish(events.TypeShareURL, dev.ID(), map[string]string{"url": body.Url}) } @@ -56,7 +56,7 @@ func (p *SharePlugin) Handle(ctx context.Context, dev device.Sender, pkt *protoc plugin.RunCommandAsync(p.Logger, "xdg-open", body.Url) } else { p.Logger.Warn("share: refusing to open non-http(s) URL", - zap.String("url", body.Url)) + log.String("url", body.Url)) } return nil } @@ -99,7 +99,7 @@ func (p *SharePlugin) Handle(ctx context.Context, dev device.Sender, pkt *protoc err := ReceiveSideChannel(context.Background(), remoteIP, payloadPort, payloadSize, destPath, p.TLSConfig, expectedFP, onProgress, p.Logger, p.sidechannel) if err != nil { - p.Logger.Error("share receive failed", zap.Error(err)) + p.Logger.Error("share receive failed", log.Error(err)) if p.bus != nil { p.bus.Publish(events.TypeShareComplete, dev.ID(), map[string]interface{}{ "file": body.Filename, @@ -111,7 +111,7 @@ func (p *SharePlugin) Handle(ctx context.Context, dev device.Sender, pkt *protoc if body.LastModified > 0 { modTime := time.UnixMilli(body.LastModified) if err := os.Chtimes(destPath, modTime, modTime); err != nil { - p.Logger.Debug("share: failed to restore file timestamps", zap.Error(err)) + p.Logger.Debug("share: failed to restore file timestamps", log.Error(err)) } } @@ -127,7 +127,7 @@ func (p *SharePlugin) Handle(ctx context.Context, dev device.Sender, pkt *protoc // file itself is still saved and announced — open it manually. if autoOpenBlocked(destPath) { p.Logger.Warn("share: refusing to auto-open executable file", - zap.String("file", destPath)) + log.String("file", destPath)) } else { cmd := p.cfg.OpenCommand if cmd == "" { diff --git a/internal/plugins/share/send.go b/internal/plugins/share/send.go index cb2b09a..ef0af01 100644 --- a/internal/plugins/share/send.go +++ b/internal/plugins/share/send.go @@ -11,8 +11,8 @@ import ( "github.com/bethropolis/kcd/internal/cert" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) func (p *SharePlugin) SendFile(ctx context.Context, dev device.Sender, filePath string) error { @@ -51,19 +51,19 @@ func (p *SharePlugin) SendFile(ctx context.Context, dev device.Sender, filePath if timeout == 0 { timeout = 2 * time.Minute } - err := AcceptAndSend(ln, filePath, p.TLSConfig, dev.ID(), expectedFP, timeout, onProgress, p.Logger, p.sidechannel) + err := AcceptAndSend(ln, filePath, p.TLSConfig, dev.ID(), expectedFP, timeout, onProgress, p.Logger, p.cfg.PortMin, p.cfg.PortMax, p.sidechannel) if err != nil { p.Logger.Error("share: send failed", - zap.String("device_id", dev.ID()), - zap.String("file", filepath.Base(filePath)), - zap.Int("port", port), - zap.Error(err), + log.String("device_id", dev.ID()), + log.String("file", filepath.Base(filePath)), + log.Int("port", port), + log.Error(err), ) } else { p.Logger.Info("share: send complete", - zap.String("device_id", dev.ID()), - zap.String("file", filepath.Base(filePath)), + log.String("device_id", dev.ID()), + log.String("file", filepath.Base(filePath)), ) } @@ -100,10 +100,10 @@ func (p *SharePlugin) SendFile(ctx context.Context, dev device.Sender, filePath } p.Logger.Info("share: sending transfer invitation", - zap.String("device_id", dev.ID()), - zap.String("path", filePath), - zap.Int64("size", pkt.PayloadSize), - zap.Int("port", port), + log.String("device_id", dev.ID()), + log.String("path", filePath), + log.Int64("size", pkt.PayloadSize), + log.Int("port", port), ) return dev.Send(pkt) diff --git a/internal/plugins/share/share_test.go b/internal/plugins/share/share_test.go index 07d3b55..39fbd56 100644 --- a/internal/plugins/share/share_test.go +++ b/internal/plugins/share/share_test.go @@ -13,11 +13,11 @@ import ( "github.com/bethropolis/kcd/internal/cert" "github.com/bethropolis/kcd/internal/config" - "go.uber.org/zap/zaptest" + "github.com/bethropolis/kcd/internal/log" ) func TestSharePlugin_SideChannelRoundTrip(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) dir := t.TempDir() sourcePath := filepath.Join(dir, "source.bin") destPath := filepath.Join(dir, "dest.bin") @@ -56,7 +56,7 @@ func TestSharePlugin_SideChannelRoundTrip(t *testing.T) { } serverDone := make(chan error, 1) go func() { - serverDone <- AcceptAndSend(ln, sourcePath, tlsConfig, "test_device_share", fp, 2*time.Second, func(c, t int64) {}, logger) + serverDone <- AcceptAndSend(ln, sourcePath, tlsConfig, "test_device_share", fp, 2*time.Second, func(c, t int64) {}, logger, cfg.PortMin, cfg.PortMax) }() // 2. Run Receiver (dial loopback) @@ -88,7 +88,7 @@ func TestSharePlugin_SideChannelRoundTrip(t *testing.T) { } func TestSharePlugin_SideChannelRejectsWrongFingerprint(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) dir := t.TempDir() sourcePath := filepath.Join(dir, "source.bin") destPath := filepath.Join(dir, "dest.bin") @@ -128,7 +128,7 @@ func TestSharePlugin_SideChannelRejectsWrongFingerprint(t *testing.T) { } serverDone := make(chan error, 1) go func() { - serverDone <- AcceptAndSend(ln, sourcePath, tlsConfig, "test_device_share", fp, 2*time.Second, nil, logger) + serverDone <- AcceptAndSend(ln, sourcePath, tlsConfig, "test_device_share", fp, 2*time.Second, nil, logger, cfg.PortMin, cfg.PortMax) }() // Dial with a wrong expected fingerprint: the receiver must refuse diff --git a/internal/plugins/share/transfer.go b/internal/plugins/share/transfer.go index b028311..5c69b44 100644 --- a/internal/plugins/share/transfer.go +++ b/internal/plugins/share/transfer.go @@ -12,8 +12,8 @@ import ( "github.com/bethropolis/kcd/internal/cert" "github.com/bethropolis/kcd/internal/config" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) type progressWriter struct { @@ -31,7 +31,7 @@ func (pw *progressWriter) Write(p []byte) (int, error) { return n, nil } -func ReceiveSideChannel(ctx context.Context, ip net.IP, port int, size int64, dest string, tlsConfig *tls.Config, expectedFP string, onProgress func(int64, int64), logger *zap.Logger, options ...transport.SidechannelOptions) error { +func ReceiveSideChannel(ctx context.Context, ip net.IP, port int, size int64, dest string, tlsConfig *tls.Config, expectedFP string, onProgress func(int64, int64), logger log.Logger, options ...transport.SidechannelOptions) error { if size < 0 { return fmt.Errorf("share: indefinite payload sizes (-1) are not supported") } @@ -66,7 +66,7 @@ func ReceiveSideChannel(ctx context.Context, ip net.IP, port int, size int64, de return fmt.Errorf("share: transfer truncated (%d/%d bytes)", n, size) } - logger.Info("share: transfer complete", zap.String("path", dest), zap.Int64("bytes", n)) + logger.Info("share: transfer complete", log.String("path", dest), log.Int64("bytes", n)) return nil } @@ -87,7 +87,8 @@ func ListenSideChannel(ctx context.Context, cfg config.ShareConfig, tlsConfig *t // AcceptAndSend waits for the phone to connect, performs the TLS handshake, and streams the file. // Optional SidechannelOptions bound streaming silence the same way as the dial path. -func AcceptAndSend(ln net.Listener, filePath string, tlsConfig *tls.Config, expectedDeviceID, expectedFP string, timeout time.Duration, onProgress func(int64, int64), logger *zap.Logger, options ...transport.SidechannelOptions) error { +// portMin/portMax are the configured side-channel range, used only for firewall hints. +func AcceptAndSend(ln net.Listener, filePath string, tlsConfig *tls.Config, expectedDeviceID, expectedFP string, timeout time.Duration, onProgress func(int64, int64), logger log.Logger, portMin, portMax int, options ...transport.SidechannelOptions) error { defer ln.Close() addr := ln.Addr().String() @@ -102,38 +103,38 @@ func AcceptAndSend(ln net.Listener, filePath string, tlsConfig *tls.Config, expe }() logger.Info("share: waiting for device to connect", - zap.String("listen_addr", addr), - zap.String("device_id", expectedDeviceID), - zap.String("file", filePath), + log.String("listen_addr", addr), + log.String("device_id", expectedDeviceID), + log.String("file", filePath), ) conn, err := ln.Accept() if err != nil { if acceptCtx.Err() != nil { - logger.Warn("share: timed out waiting for device to connect — is TCP 1739-1764 open in your firewall?", - zap.String("listen_addr", addr), - zap.String("device_id", expectedDeviceID), + logger.Warn(fmt.Sprintf("share: timed out waiting for device to connect — is TCP %d-%d open in your firewall?", portMin, portMax), + log.String("listen_addr", addr), + log.String("device_id", expectedDeviceID), ) - return fmt.Errorf("timed out waiting for device to connect on %s (check firewall: ufw allow 1739:1764/tcp)", addr) + return fmt.Errorf("timed out waiting for device to connect on %s (check firewall: ufw allow %d:%d/tcp)", addr, portMin, portMax) } return fmt.Errorf("accept failed: %w", err) } defer conn.Close() logger.Info("share: device connected to side-channel, starting TLS handshake", - zap.String("remote_addr", conn.RemoteAddr().String()), + log.String("remote_addr", conn.RemoteAddr().String()), ) tlsConn := tls.Server(conn, tlsConfig) if err := tlsConn.HandshakeContext(acceptCtx); err != nil { if err == io.EOF { logger.Debug("share: TLS handshake aborted by remote device (EOF)", - zap.String("remote_addr", conn.RemoteAddr().String()), + log.String("remote_addr", conn.RemoteAddr().String()), ) } else { logger.Error("share: TLS handshake failed on side-channel", - zap.String("remote_addr", conn.RemoteAddr().String()), - zap.Error(err), + log.String("remote_addr", conn.RemoteAddr().String()), + log.Error(err), ) } return fmt.Errorf("tls handshake failed: %w", err) @@ -142,8 +143,8 @@ func AcceptAndSend(ln net.Listener, filePath string, tlsConfig *tls.Config, expe state := tlsConn.ConnectionState() if len(state.PeerCertificates) == 0 { logger.Error("share: device presented no client certificate", - zap.String("remote_addr", conn.RemoteAddr().String()), - zap.String("expected_device", expectedDeviceID), + log.String("remote_addr", conn.RemoteAddr().String()), + log.String("expected_device", expectedDeviceID), ) return fmt.Errorf("share: client presented no certificate (expected device %s)", expectedDeviceID) } @@ -154,8 +155,8 @@ func AcceptAndSend(ln net.Listener, filePath string, tlsConfig *tls.Config, expe normCN := strings.ReplaceAll(certCN, "-", "_") if normCN != expectedDeviceID { logger.Error("share: cert CN mismatch — unexpected device connected to side-channel", - zap.String("expected_device", expectedDeviceID), - zap.String("cert_cn", certCN), + log.String("expected_device", expectedDeviceID), + log.String("cert_cn", certCN), ) return fmt.Errorf("share: cert CN mismatch: expected device %s, got %s", expectedDeviceID, certCN) } @@ -164,19 +165,19 @@ func AcceptAndSend(ln net.Listener, filePath string, tlsConfig *tls.Config, expe // with the same CN can't pull the file. if expectedFP == "" { logger.Warn("share: no pinned peer fingerprint, skipping side-channel verification", - zap.String("expected_device", expectedDeviceID)) + log.String("expected_device", expectedDeviceID)) } else if err := cert.VerifySideChannelPeer(state, expectedFP); err != nil { logger.Error("share: side-channel peer verification failed", - zap.String("expected_device", expectedDeviceID), - zap.String("expected_fp", expectedFP), - zap.Error(err), + log.String("expected_device", expectedDeviceID), + log.String("expected_fp", expectedFP), + log.Error(err), ) return fmt.Errorf("share: side-channel peer verification failed: %w", err) } logger.Info("share: TLS OK, streaming file", - zap.String("file", filePath), - zap.String("remote_addr", conn.RemoteAddr().String()), + log.String("file", filePath), + log.String("remote_addr", conn.RemoteAddr().String()), ) f, err := os.Open(filePath) @@ -203,6 +204,6 @@ func AcceptAndSend(ln net.Listener, filePath string, tlsConfig *tls.Config, expe return fmt.Errorf("stream error: %w", err) } - logger.Info("share: send complete", zap.String("path", filePath), zap.Int64("bytes", n)) + logger.Info("share: send complete", log.String("path", filePath), log.Int64("bytes", n)) return nil } diff --git a/internal/plugins/share/types.go b/internal/plugins/share/types.go index 5bd58d6..73f2d96 100644 --- a/internal/plugins/share/types.go +++ b/internal/plugins/share/types.go @@ -7,8 +7,8 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) type progressThrottle struct { @@ -59,11 +59,11 @@ type SharePlugin struct { DownloadDir string cfg config.ShareConfig TLSConfig *tls.Config - Logger *zap.Logger + Logger log.Logger bus *events.Bus } -func NewSharePlugin(downloadDir string, cfg config.ShareConfig, tlsConfig *tls.Config, bus *events.Bus, logger *zap.Logger, options ...transport.SidechannelOptions) *SharePlugin { +func NewSharePlugin(downloadDir string, cfg config.ShareConfig, tlsConfig *tls.Config, bus *events.Bus, logger log.Logger, options ...transport.SidechannelOptions) *SharePlugin { var sidechannel transport.SidechannelOptions if len(options) > 0 { sidechannel = options[0] @@ -73,7 +73,7 @@ func NewSharePlugin(downloadDir string, cfg config.ShareConfig, tlsConfig *tls.C DownloadDir: downloadDir, cfg: cfg, TLSConfig: tlsConfig, - Logger: logger.With(zap.String("plugin", "share")), + Logger: logger.With(log.String("plugin", "share")), bus: bus, } } diff --git a/internal/plugins/sms/attachment.go b/internal/plugins/sms/attachment.go index 3dad891..b044f6f 100644 --- a/internal/plugins/sms/attachment.go +++ b/internal/plugins/sms/attachment.go @@ -14,9 +14,9 @@ import ( "github.com/bethropolis/kcd/internal/cert" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) // handleAttachmentFile downloads an MMS attachment file sent by the phone. @@ -53,12 +53,12 @@ func (p *SMSPlugin) handleAttachmentFile(ctx context.Context, dev device.Sender, go func() { if err := p.receiveAttachment(ctx, remoteIP, port, payloadSize, destPath, expectedFP); err != nil { - p.logger.Error("sms: attachment download failed", zap.Error(err)) + p.logger.Error("sms: attachment download failed", log.Error(err)) return } p.logger.Info("sms: attachment downloaded", - zap.String("path", destPath), - zap.String("filename", body.Filename), + log.String("path", destPath), + log.String("filename", body.Filename), ) if p.bus != nil { p.bus.Publish(events.TypeSMSAttachment, dev.ID(), map[string]any{ diff --git a/internal/plugins/sms/handle.go b/internal/plugins/sms/handle.go index c635910..2d24205 100644 --- a/internal/plugins/sms/handle.go +++ b/internal/plugins/sms/handle.go @@ -7,9 +7,9 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) // --- Handle ---------------------------------------------------------------- @@ -53,9 +53,9 @@ func (p *SMSPlugin) handleMessages(_ context.Context, dev device.Sender, pkt *pr } p.logger.Debug("sms: message received", - zap.String("from", sender), - zap.String("body", msg.Body), - zap.Int64("thread_id", msg.ThreadID), + log.String("from", sender), + log.String("body", msg.Body), + log.Int64("thread_id", msg.ThreadID), ) if p.bus != nil { diff --git a/internal/plugins/sms/sms_test.go b/internal/plugins/sms/sms_test.go index c7dbe19..8fa8cf9 100644 --- a/internal/plugins/sms/sms_test.go +++ b/internal/plugins/sms/sms_test.go @@ -10,8 +10,8 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) func TestCleanFilename(t *testing.T) { @@ -38,7 +38,7 @@ func TestCleanFilename(t *testing.T) { } func TestReceiveAttachmentRejectsBadSize(t *testing.T) { - p := NewSMSPlugin(config.SMSConfig{}, nil, nil, zap.NewNop()) + p := NewSMSPlugin(config.SMSConfig{}, nil, nil, log.Nop()) for _, size := range []int64{0, -1, -999, maxSMSAttachmentBytes + 1} { err := p.receiveAttachment(context.Background(), nil, 0, size, "/nonexistent/x", "") if err == nil { @@ -66,7 +66,7 @@ func (s *captureSender) UpdateBattery(int, bool) {} func (s *captureSender) GetBattery() (int, bool) { return 0, false } func TestSendSMSUsesV2Schema(t *testing.T) { - p := NewSMSPlugin(config.SMSConfig{}, nil, nil, zap.NewNop()) + p := NewSMSPlugin(config.SMSConfig{}, nil, nil, log.Nop()) dev := &captureSender{} if err := p.SendSMS(dev, "+1234567890", "hello"); err != nil { t.Fatalf("SendSMS: %v", err) @@ -100,7 +100,7 @@ func TestMessagesBatchAcceptsIntReadFlag(t *testing.T) { Type: PacketTypeSMSMessages, Body: json.RawMessage(`{"version":2,"messages":[{"event":1,"body":"hi","addresses":[{"address":"+1"}],"date":1711234567,"type":1,"thread_id":7,"read":` + read + `}]}`), } - p := NewSMSPlugin(config.SMSConfig{}, nil, nil, zap.NewNop()) + p := NewSMSPlugin(config.SMSConfig{}, nil, nil, log.Nop()) if err := p.Handle(context.Background(), &captureSender{}, pkt); err != nil { t.Errorf("Handle with read=%s: %v, want nil", read, err) } diff --git a/internal/plugins/sms/types.go b/internal/plugins/sms/types.go index 08b111d..da8e1fa 100644 --- a/internal/plugins/sms/types.go +++ b/internal/plugins/sms/types.go @@ -8,9 +8,9 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" "github.com/bethropolis/kcd/internal/transport" - "go.uber.org/zap" ) const ( @@ -37,7 +37,7 @@ type SMSPlugin struct { cfg config.SMSConfig bus *events.Bus tlsConfig *tls.Config - logger *zap.Logger + logger log.Logger cacheDir string } @@ -48,7 +48,7 @@ type Options struct { Notifications config.NotificationConfig } -func NewSMSPlugin(cfg config.SMSConfig, bus *events.Bus, tlsConfig *tls.Config, logger *zap.Logger, options ...Options) *SMSPlugin { +func NewSMSPlugin(cfg config.SMSConfig, bus *events.Bus, tlsConfig *tls.Config, logger log.Logger, options ...Options) *SMSPlugin { var opts Options if len(options) > 0 { opts = options[0] @@ -65,7 +65,7 @@ func NewSMSPlugin(cfg config.SMSConfig, bus *events.Bus, tlsConfig *tls.Config, cfg: cfg, bus: bus, tlsConfig: tlsConfig, - logger: logger.With(zap.String("plugin", "sms")), + logger: logger.With(log.String("plugin", "sms")), cacheDir: cacheDir, } } diff --git a/internal/plugins/systemvolume/backend.go b/internal/plugins/systemvolume/backend.go index 40d0fbc..686a5a5 100644 --- a/internal/plugins/systemvolume/backend.go +++ b/internal/plugins/systemvolume/backend.go @@ -2,10 +2,11 @@ package systemvolume import ( "context" - "os/exec" "strconv" "strings" "time" + + "github.com/bethropolis/kcd/internal/plugin" ) // getSinks returns a list of available audio output sinks. @@ -23,7 +24,7 @@ func (p *SystemVolumePlugin) getSinksWpctl() []SinkInfo { // Get current volume from wpctl: wpctl get-volume @DEFAULT_AUDIO_SINK@ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - out, err := exec.CommandContext(ctx, "wpctl", "get-volume", "@DEFAULT_AUDIO_SINK@").Output() + out, err := plugin.RunCommandOutput(ctx, "wpctl", "get-volume", "@DEFAULT_AUDIO_SINK@") if err != nil { return nil } @@ -50,7 +51,7 @@ func (p *SystemVolumePlugin) getSinksWpctl() []SinkInfo { func (p *SystemVolumePlugin) getSinksPactl() []SinkInfo { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - out, err := exec.CommandContext(ctx, "pactl", "get-sink-volume", "@DEFAULT_SINK@").Output() + out, err := plugin.RunCommandOutput(ctx, "pactl", "get-sink-volume", "@DEFAULT_SINK@") if err != nil { return nil } @@ -66,7 +67,7 @@ func (p *SystemVolumePlugin) getSinksPactl() []SinkInfo { } muteCtx, muteCancel := context.WithTimeout(context.Background(), 5*time.Second) defer muteCancel() - muteOut, _ := exec.CommandContext(muteCtx, "pactl", "get-sink-mute", "@DEFAULT_SINK@").Output() + muteOut, _ := plugin.RunCommandOutput(muteCtx, "pactl", "get-sink-mute", "@DEFAULT_SINK@") muted := strings.Contains(string(muteOut), "yes") return []SinkInfo{{ Name: "@DEFAULT_SINK@", @@ -88,23 +89,25 @@ func (p *SystemVolumePlugin) setVolumeStr(_ string, volumeStr string, muted bool switch p.backend { case "wpctl": pct := volumeStr + "%" - if err := exec.CommandContext(ctx, "wpctl", "set-volume", "@DEFAULT_AUDIO_SINK@", pct).Run(); err != nil { + if _, err := plugin.RunCommandSync(ctx, "wpctl", "set-volume", "@DEFAULT_AUDIO_SINK@", pct); err != nil { return err } muteArg := "0" if muted { muteArg = "1" } - return exec.CommandContext(ctx, "wpctl", "set-mute", "@DEFAULT_AUDIO_SINK@", muteArg).Run() + _, err := plugin.RunCommandSync(ctx, "wpctl", "set-mute", "@DEFAULT_AUDIO_SINK@", muteArg) + return err case "pactl": - if err := exec.CommandContext(ctx, "pactl", "set-sink-volume", "@DEFAULT_SINK@", volumeStr+"%").Run(); err != nil { + if _, err := plugin.RunCommandSync(ctx, "pactl", "set-sink-volume", "@DEFAULT_SINK@", volumeStr+"%"); err != nil { return err } muteArg := "false" if muted { muteArg = "true" } - return exec.CommandContext(ctx, "pactl", "set-sink-mute", "@DEFAULT_SINK@", muteArg).Run() + _, err := plugin.RunCommandSync(ctx, "pactl", "set-sink-mute", "@DEFAULT_SINK@", muteArg) + return err } return nil } diff --git a/internal/plugins/systemvolume/handle.go b/internal/plugins/systemvolume/handle.go index 4e4d78b..b13cd88 100644 --- a/internal/plugins/systemvolume/handle.go +++ b/internal/plugins/systemvolume/handle.go @@ -6,8 +6,8 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) func (p *SystemVolumePlugin) Handle(ctx context.Context, dev device.Sender, pkt *protocol.Packet) error { @@ -26,11 +26,11 @@ func (p *SystemVolumePlugin) Handle(ctx context.Context, dev device.Sender, pkt sinks := p.getSinks() pkt, err := protocol.NewPacket("kdeconnect.systemvolume", sinkListBody{SinkList: sinks}) if err != nil { - p.logger.Error("systemvolume: failed to create sink list packet", zap.Error(err)) + p.logger.Error("systemvolume: failed to create sink list packet", log.Error(err)) return } if err := dev.Send(pkt); err != nil { - p.logger.Error("systemvolume: failed to send sink list", zap.Error(err)) + p.logger.Error("systemvolume: failed to send sink list", log.Error(err)) } }() return nil @@ -42,7 +42,7 @@ func (p *SystemVolumePlugin) Handle(ctx context.Context, dev device.Sender, pkt body.Name = "@DEFAULT_AUDIO_SINK@" } if err := p.setVolume(body.Name, body.Volume, body.Muted); err != nil { - p.logger.Warn("systemvolume: failed to set volume", zap.Error(err)) + p.logger.Warn("systemvolume: failed to set volume", log.Error(err)) return } if p.bus != nil { diff --git a/internal/plugins/systemvolume/types.go b/internal/plugins/systemvolume/types.go index 1467b2a..65af86d 100644 --- a/internal/plugins/systemvolume/types.go +++ b/internal/plugins/systemvolume/types.go @@ -7,19 +7,19 @@ import ( "time" "github.com/bethropolis/kcd/internal/events" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) // SystemVolumePlugin handles volume control packets from the phone. type SystemVolumePlugin struct { - logger *zap.Logger + logger log.Logger bus *events.Bus backend string // "wpctl" or "pactl" } -func NewSystemVolumePlugin(bus *events.Bus, logger *zap.Logger) *SystemVolumePlugin { +func NewSystemVolumePlugin(bus *events.Bus, logger log.Logger) *SystemVolumePlugin { p := &SystemVolumePlugin{ - logger: logger.With(zap.String("plugin", "systemvolume")), + logger: logger.With(log.String("plugin", "systemvolume")), bus: bus, } // Detect available audio backend at init time. diff --git a/internal/plugins/telephony/telephony.go b/internal/plugins/telephony/telephony.go index 5a6fa93..1757bbe 100644 --- a/internal/plugins/telephony/telephony.go +++ b/internal/plugins/telephony/telephony.go @@ -8,18 +8,18 @@ import ( "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/plugin" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" ) type TelephonyPlugin struct { notifications config.NotificationConfig bus *events.Bus - logger *zap.Logger + logger log.Logger } -func NewTelephonyPlugin(bus *events.Bus, logger *zap.Logger, notifications ...config.NotificationConfig) *TelephonyPlugin { +func NewTelephonyPlugin(bus *events.Bus, logger log.Logger, notifications ...config.NotificationConfig) *TelephonyPlugin { var notificationCfg config.NotificationConfig if len(notifications) > 0 { notificationCfg = notifications[0] @@ -27,7 +27,7 @@ func NewTelephonyPlugin(bus *events.Bus, logger *zap.Logger, notifications ...co return &TelephonyPlugin{ notifications: notificationCfg, bus: bus, - logger: logger.With(zap.String("plugin", "telephony")), + logger: logger.With(log.String("plugin", "telephony")), } } diff --git a/internal/plugins/telephony/telephony_test.go b/internal/plugins/telephony/telephony_test.go index 24fc42a..581f246 100644 --- a/internal/plugins/telephony/telephony_test.go +++ b/internal/plugins/telephony/telephony_test.go @@ -10,9 +10,8 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/log" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap" - "go.uber.org/zap/zaptest" ) type stubSender struct{} @@ -43,7 +42,7 @@ func nextEvent(t *testing.T, ch <-chan events.Event) events.Event { func TestHandleStringIsCancel(t *testing.T) { // Stock phones end calls with the string "true", not a boolean. - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) bus := events.NewBus(logger) p := NewTelephonyPlugin(bus, logger) sub := bus.Subscribe(0, events.TypeTelephonyCanceled) @@ -62,11 +61,11 @@ func TestHandleStringIsCancel(t *testing.T) { } func TestHandleMissedCallEvent(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := log.NewTest(t) bus := events.NewBus(logger) // The plugin notifies via a background goroutine that can outlive the // test; a test-bound logger would panic on late writes, so detach it. - p := NewTelephonyPlugin(bus, zap.NewNop()) + p := NewTelephonyPlugin(bus, log.Nop()) sub := bus.Subscribe(0, events.TypeTelephonyMissed) defer sub.Close() diff --git a/internal/protocol/ports.go b/internal/protocol/ports.go new file mode 100644 index 0000000..95d22d0 --- /dev/null +++ b/internal/protocol/ports.go @@ -0,0 +1,13 @@ +package protocol + +// Well-known KDE Connect ports. DefaultTCPPort is the control channel +// (TCP listener, UDP discovery, identity tcpPort); the side-channel range +// bounds inbound file-transfer listeners. All three are user-overridable +// via config — code must reference the configured value and fall back to +// these constants only for defaults. +const ( + DefaultTCPPort = 1716 + + DefaultSidechannelPortMin = 1739 + DefaultSidechannelPortMax = 1764 +) diff --git a/internal/testutil/peer.go b/internal/testutil/peer.go index 4c5cd67..0240c88 100644 --- a/internal/testutil/peer.go +++ b/internal/testutil/peer.go @@ -55,7 +55,7 @@ func (p *MockPeer) TryDial(serverAddr string) (net.Conn, error) { DeviceName: "Mock Peer", DeviceType: "phone", ProtocolVersion: protocol.ProtocolVersion, - TCPPort: 1716, + TCPPort: protocol.DefaultTCPPort, }) if err != nil { conn.Close() diff --git a/internal/transport/keepalive.go b/internal/transport/keepalive.go new file mode 100644 index 0000000..4a4c737 --- /dev/null +++ b/internal/transport/keepalive.go @@ -0,0 +1,35 @@ +package transport + +import ( + "net" + "time" +) + +// TCP keepalive probe schedule, shared by the inbound listener and the +// outbound dial path. One definition keeps both ends symmetric — +// asymmetric timeouts are a classic source of one-sided zombie +// connections where one end holds a dead socket the other has dropped. +const ( + keepAliveIdle = 30 * time.Second + keepAliveInterval = 10 * time.Second + keepAliveCount = 3 +) + +// SetTCPKeepAlive enables keepalive probing on conn. Non-TCP connections +// are left untouched. Falls back to the legacy period-only API where the +// full config call is unavailable. +func SetTCPKeepAlive(conn net.Conn) { + tcpConn, ok := conn.(*net.TCPConn) + if !ok { + return + } + if err := tcpConn.SetKeepAliveConfig(net.KeepAliveConfig{ + Enable: true, + Idle: keepAliveIdle, + Interval: keepAliveInterval, + Count: keepAliveCount, + }); err != nil { + _ = tcpConn.SetKeepAlive(true) + _ = tcpConn.SetKeepAlivePeriod(keepAliveIdle) + } +} diff --git a/internal/transport/listener.go b/internal/transport/listener.go index 95318dc..fb9fb88 100644 --- a/internal/transport/listener.go +++ b/internal/transport/listener.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net" - "time" ) // Listener wraps a net.Listener. @@ -30,17 +29,7 @@ func (l *Listener) Accept() (net.Conn, error) { return nil, err } - if tcpConn, ok := conn.(*net.TCPConn); ok { - if err := tcpConn.SetKeepAliveConfig(net.KeepAliveConfig{ - Enable: true, - Idle: 30 * time.Second, - Interval: 10 * time.Second, - Count: 3, - }); err != nil { - _ = tcpConn.SetKeepAlive(true) - _ = tcpConn.SetKeepAlivePeriod(30 * time.Second) - } - } + SetTCPKeepAlive(conn) return conn, nil } diff --git a/internal/transport/sidechannel.go b/internal/transport/sidechannel.go index 52f330f..5e45995 100644 --- a/internal/transport/sidechannel.go +++ b/internal/transport/sidechannel.go @@ -9,7 +9,7 @@ import ( "time" "github.com/bethropolis/kcd/internal/cert" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) // SidechannelOptions bounds the connection establishment phase — TCP dial @@ -22,11 +22,15 @@ type SidechannelOptions struct { IdleTimeout time.Duration } +// defaultSetupTimeout bounds side-channel establishment (TCP dial + TLS +// handshake) when the caller passes no explicit timeout. +const defaultSetupTimeout = 15 * time.Second + // DialSidechannel connects as a TLS client and verifies the paired certificate // before returning any payload bytes. The caller owns and must close the result. // Once established, payload streaming carries no absolute deadline — only the // optional idle bound from SidechannelOptions applies. -func DialSidechannel(ctx context.Context, ip net.IP, port int, tlsConfig *tls.Config, expectedFP string, logger *zap.Logger, options ...SidechannelOptions) (net.Conn, error) { +func DialSidechannel(ctx context.Context, ip net.IP, port int, tlsConfig *tls.Config, expectedFP string, logger log.Logger, options ...SidechannelOptions) (net.Conn, error) { if ip == nil || port < 1 || port > 65535 { return nil, fmt.Errorf("side-channel: invalid peer address") } @@ -38,14 +42,14 @@ func DialSidechannel(ctx context.Context, ip net.IP, port int, tlsConfig *tls.Co opts = options[0] } if opts.Timeout <= 0 { - opts.Timeout = 15 * time.Second + opts.Timeout = defaultSetupTimeout } addr := net.JoinHostPort(ip.String(), strconv.Itoa(port)) // One wall-clock budget covers everything before payload streaming: // the TCP connect and the TLS handshake together. setupCtx, cancel := context.WithTimeout(ctx, opts.Timeout) defer cancel() - dialer := net.Dialer{Timeout: opts.Timeout, KeepAlive: 30 * time.Second} + dialer := net.Dialer{Timeout: opts.Timeout, KeepAlive: keepAliveIdle} raw, err := dialer.DialContext(setupCtx, "tcp", addr) if err != nil { return nil, fmt.Errorf("side-channel: dial %s: %w", addr, err) @@ -56,9 +60,7 @@ func DialSidechannel(ctx context.Context, ip net.IP, port int, tlsConfig *tls.Co return nil, fmt.Errorf("side-channel: handshake %s: %w", addr, err) } if expectedFP == "" { - if logger != nil { - logger.Warn("side-channel: no pinned peer fingerprint, skipping verification", zap.String("remote_addr", addr)) - } + logger.Warn("side-channel: no pinned peer fingerprint, skipping verification", log.String("remote_addr", addr)) } else if err := cert.VerifySideChannelPeer(conn.ConnectionState(), expectedFP); err != nil { conn.Close() return nil, fmt.Errorf("side-channel: peer verification failed: %w", err) diff --git a/internal/transport/sidechannel_test.go b/internal/transport/sidechannel_test.go index 3652f1e..fe34718 100644 --- a/internal/transport/sidechannel_test.go +++ b/internal/transport/sidechannel_test.go @@ -14,7 +14,7 @@ import ( "time" "github.com/bethropolis/kcd/internal/cert" - "go.uber.org/zap" + "github.com/bethropolis/kcd/internal/log" ) // startSidechannelServer accepts one TLS connection and streams N bytes to it. @@ -92,7 +92,7 @@ func TestDialSidechannelRoundTrip(t *testing.T) { host, portStr, _ := net.SplitHostPort(addr) port, _ := strconv.Atoi(portStr) - conn, err := DialSidechannel(context.Background(), net.ParseIP(host), port, tlsConfig, fp, zap.NewNop(), SidechannelOptions{Timeout: 2 * time.Second}) + conn, err := DialSidechannel(context.Background(), net.ParseIP(host), port, tlsConfig, fp, log.Nop(), SidechannelOptions{Timeout: 2 * time.Second}) if err != nil { t.Fatalf("DialSidechannel: %v", err) } @@ -125,7 +125,7 @@ func TestDialSidechannelPinMismatch(t *testing.T) { port, _ := strconv.Atoi(portStr) wrongFP := "0000000000000000000000000000000000000000000000000000000000000000" - _, err = DialSidechannel(context.Background(), net.ParseIP(host), port, tlsConfig, wrongFP, zap.NewNop(), SidechannelOptions{Timeout: 2 * time.Second}) + _, err = DialSidechannel(context.Background(), net.ParseIP(host), port, tlsConfig, wrongFP, log.Nop(), SidechannelOptions{Timeout: 2 * time.Second}) if err == nil { t.Fatal("expected pin verification failure, got nil error") } @@ -170,7 +170,7 @@ func TestDialSidechannelSetupTimeout(t *testing.T) { port, _ := strconv.Atoi(portStr) start := time.Now() - _, err = DialSidechannel(context.Background(), net.ParseIP(host), port, tlsConfig, "", zap.NewNop(), SidechannelOptions{Timeout: 500 * time.Millisecond}) + _, err = DialSidechannel(context.Background(), net.ParseIP(host), port, tlsConfig, "", log.Nop(), SidechannelOptions{Timeout: 500 * time.Millisecond}) elapsed := time.Since(start) if err == nil { @@ -202,7 +202,7 @@ func TestDialSidechannelSurvivesContextCancel(t *testing.T) { port, _ := strconv.Atoi(portStr) ctx, cancel := context.WithCancel(context.Background()) - conn, err := DialSidechannel(ctx, net.ParseIP(host), port, tlsConfig, fp, zap.NewNop(), SidechannelOptions{Timeout: 2 * time.Second}) + conn, err := DialSidechannel(ctx, net.ParseIP(host), port, tlsConfig, fp, log.Nop(), SidechannelOptions{Timeout: 2 * time.Second}) if err != nil { t.Fatalf("DialSidechannel: %v", err) } diff --git a/packaging/nautilus-kcd.py b/packaging/nautilus-kcd.py index 7c02a1d..c21abac 100644 --- a/packaging/nautilus-kcd.py +++ b/packaging/nautilus-kcd.py @@ -15,6 +15,7 @@ import os import subprocess import threading +import time import urllib.parse import gi @@ -79,9 +80,15 @@ def _connected_devices() -> list[dict]: def _uri_to_path(uri: str) -> str | None: """Convert a file:// URI to a local absolute path, or return None.""" - if not uri.startswith("file://"): + try: + parts = urllib.parse.urlparse(uri) + except ValueError: + return None + if parts.scheme != "file": return None - return urllib.parse.unquote(uri[len("file://"):]) + if parts.netloc not in ("", "localhost"): + return None # remote host — not a local file + return urllib.parse.unquote(parts.path) # ── Nautilus extension ──────────────────────────────────────────────────────── @@ -95,7 +102,6 @@ class KcdSendExtension(GObject.GObject, Nautilus.MenuProvider): _CACHE_SECONDS: float = 3.0 def _get_devices(self) -> list[dict]: - import time now = time.monotonic() with self._cache_lock: if now < self._cache_ttl and self._cache: @@ -112,9 +118,12 @@ def get_file_items(self, *args): if not files: return [] - # Only show the menu for local files. + # Only show the menu when the selection holds at least one regular + # file. Directories are skipped: `kcd share` rejects them, so + # offering the menu for dir-only selections would only produce + # failure notifications. paths = [_uri_to_path(f.get_uri()) for f in files] - paths = [p for p in paths if p and os.path.exists(p)] + paths = [p for p in paths if p and os.path.isfile(p)] if not paths: return [] @@ -132,7 +141,7 @@ def get_file_items(self, *args): if len(devices) == 1: # Single device — no submenu, activate directly. dev = devices[0] - top.set_property("label", f"Send to {dev['name']}") + top.set_property("label", f"Send to {dev.get('name', dev['id'])}") top.connect("activate", self._on_send, paths, dev) else: submenu = Nautilus.Menu() @@ -140,7 +149,7 @@ def get_file_items(self, *args): for dev in devices: item = Nautilus.MenuItem( name=f"Kcd::Device_{dev['id']}", - label=dev["name"], + label=dev.get("name", dev["id"]), icon="phone", ) item.connect("activate", self._on_send, paths, dev) diff --git a/pkg/client/client_sms_contacts.go b/pkg/client/client_sms_contacts.go index cd6066c..bf48c7f 100644 --- a/pkg/client/client_sms_contacts.go +++ b/pkg/client/client_sms_contacts.go @@ -60,3 +60,10 @@ func (c *Client) ContactsList(deviceID string) ([]contacts.ContactSummary, error } return list, nil } + +// ContactsClear deletes a device's cached contacts. Offline-capable: the +// cache is local state. Re-sync restores everything from the phone. +func (c *Client) ContactsClear(deviceID string) error { + _, err := c.Call(ipc.CmdContactsClear, ipc.DevicePayload{DeviceID: deviceID}) + return err +}