From 3d853d9d9d10835f3866d5de4b086b868038987f Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Wed, 27 May 2026 12:48:17 +0300 Subject: [PATCH 01/25] feat: switch Docker image to Alpine with entrypoint for permission handling and default config --- .goreleaser.yaml | 2 ++ Dockerfile.goreleaser | 8 ++++---- docs/CONTAINER.md | 6 +++--- entrypoint.sh | 22 ++++++++++++++++++++++ 4 files changed, 31 insertions(+), 7 deletions(-) create mode 100755 entrypoint.sh diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 0e4fa21..a6a9017 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -115,6 +115,8 @@ dockers_v2: - "v{{ .Version }}" - latest dockerfile: Dockerfile.goreleaser + extra_files: + - entrypoint.sh ids: - kcd labels: diff --git a/Dockerfile.goreleaser b/Dockerfile.goreleaser index 407d0d0..ef88caa 100644 --- a/Dockerfile.goreleaser +++ b/Dockerfile.goreleaser @@ -5,7 +5,7 @@ RUN apk add --no-cache ca-certificates && \ chown -R 65534:65534 /out/config /out/state /out/data /out/run # ─── Runtime ─────────────────────────────────────────────────────────────────── -FROM scratch +FROM alpine:3.20 LABEL org.opencontainers.image.title="kcd" \ org.opencontainers.image.description="Headless KDE Connect daemon" \ @@ -19,13 +19,13 @@ COPY --from=base --chown=65534:65534 /out/state /state COPY --from=base --chown=65534:65534 /out/data /data COPY --from=base --chown=65534:65534 /out/run /run +COPY entrypoint.sh /entrypoint.sh + ARG TARGETPLATFORM COPY $TARGETPLATFORM/kcd /usr/bin/kcd VOLUME ["/config", "/state", "/data"] -USER 65534:65534 - ENV XDG_CONFIG_HOME=/config \ XDG_STATE_HOME=/state \ XDG_RUNTIME_DIR=/run \ @@ -36,5 +36,5 @@ EXPOSE 1716/tcp 1716/udp 1739-1764/tcp HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ CMD ["/usr/bin/kcd", "devices"] -ENTRYPOINT ["/usr/bin/kcd"] +ENTRYPOINT ["/entrypoint.sh"] CMD ["daemon"] diff --git a/docs/CONTAINER.md b/docs/CONTAINER.md index cf1670e..20dc3d3 100644 --- a/docs/CONTAINER.md +++ b/docs/CONTAINER.md @@ -53,9 +53,9 @@ docker compose run --rm kcd-cli devices | `/data` | Received files | Optional | | `/run` | IPC Unix socket | Yes (tmpfs recommended) | -The container runs as UID 65534 (nobody). All volume directories are pre-created -in the image with correct ownership, so bind mounts must grant write access to -UID 65534. +The entrypoint auto-fixes ownership on all volumes at startup (remaps host-root +owned paths to UID 65534). A default `kcd.toml` is created at `/config/kcd/` if +none exists. ## Building Locally diff --git a/entrypoint.sh b/entrypoint.sh new file mode 100755 index 0000000..1b1d7f4 --- /dev/null +++ b/entrypoint.sh @@ -0,0 +1,22 @@ +#!/bin/sh +# kcd entrypoint — fixes volume ownership and creates default config. + +for d in /config /state /data /run; do + [ "$(stat -c %u "$d")" = "0" ] && chown -R 65534:65534 "$d" +done + +if [ ! -f /config/kcd/kcd.toml ]; then + mkdir -p /config/kcd + cat > /config/kcd/kcd.toml << 'EOF' +log_level = "info" + +[plugins] +battery = true +notification = false +mpris = false +systemvolume = false +mousepad = false +EOF +fi + +exec "$@" From db1cc20a4907d8d637f864f100ea15c01f305643 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Wed, 27 May 2026 12:56:17 +0300 Subject: [PATCH 02/25] feat: switch Docker image to Alpine with entrypoint for permission handling and default config --- Dockerfile | 34 +++++----------------------------- entrypoint.sh | 2 +- 2 files changed, 6 insertions(+), 30 deletions(-) diff --git a/Dockerfile b/Dockerfile index 7df6f20..6a1fda1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,18 +1,15 @@ # ─── Builder ─────────────────────────────────────────────────────────────────── FROM golang:1.25-alpine AS builder -# Build-time version info (injected by GoReleaser / docker buildx) ARG VERSION=dev ARG COMMIT=none ARG DATE=unknown WORKDIR /build -# Fetch dependencies in a separate layer so they are cached between code changes COPY go.mod go.sum ./ RUN go mod download && go mod verify -# Build a fully static binary COPY . . RUN CGO_ENABLED=0 GOOS=linux go build \ -trimpath \ @@ -23,23 +20,18 @@ RUN CGO_ENABLED=0 GOOS=linux go build \ -o /out/kcd \ ./cmd/kcd -# Pre-create empty directories owned by nobody:nogroup (65534:65534) -# so the runtime container can run unprivileged and still write to them. RUN mkdir -p /out/empty && \ for d in config state data run/kcd; do \ mkdir -p "/out/$d"; \ done && \ chown -R 65534:65534 /out/config /out/state /out/data /out/run -# Smoke-test: binary must be statically linked RUN apk add --no-cache file && \ file /out/kcd | grep -q "statically linked" || \ (echo "ERROR: binary is not statically linked" && exit 1) # ─── Runtime ─────────────────────────────────────────────────────────────────── -# `scratch` gives us a zero-footprint image. -# The binary is fully static, so no libc or shell is needed. -FROM scratch +FROM alpine:3.20 LABEL org.opencontainers.image.title="kcd" \ org.opencontainers.image.description="Headless KDE Connect daemon" \ @@ -47,43 +39,27 @@ LABEL org.opencontainers.image.title="kcd" \ org.opencontainers.image.source="https://github.com/bethropolis/kcd" \ org.opencontainers.image.licenses="MIT" -# TLS root certificates (needed for outbound TLS when communicating with devices) COPY --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ - -# The binary COPY --from=builder /out/kcd /usr/bin/kcd -# Pre-populate volume mount points with directories owned by nobody -# so the container runs unprivileged without permission errors. COPY --from=builder --chown=65534:65534 /out/config /config COPY --from=builder --chown=65534:65534 /out/state /state COPY --from=builder --chown=65534:65534 /out/data /data COPY --from=builder --chown=65534:65534 /out/run /run -# Volume mount points — must be provided at runtime -# /config → $XDG_CONFIG_HOME/kcd (kcd.toml, cert.pem, key.pem) -# /state → $XDG_STATE_HOME/kcd (devices.json — persisted pairs) -# /data → download_dir (received files) -VOLUME ["/config", "/state", "/data"] +COPY entrypoint.sh /entrypoint.sh -# Drop privileges — nobody can still bind ports ≥1024 and write to volumes. -USER 65534:65534 +VOLUME ["/config", "/state", "/data"] -# kcd reads these to find its paths without a real home directory ENV XDG_CONFIG_HOME=/config \ XDG_STATE_HOME=/state \ XDG_RUNTIME_DIR=/run \ XDG_CACHE_HOME=/tmp -# KDE Connect control port (TCP + UDP) -EXPOSE 1716/tcp -EXPOSE 1716/udp - -# File-transfer side-channels -EXPOSE 1739-1764/tcp +EXPOSE 1716/tcp 1716/udp 1739-1764/tcp HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ CMD ["/usr/bin/kcd", "devices"] -ENTRYPOINT ["/usr/bin/kcd"] +ENTRYPOINT ["/entrypoint.sh"] CMD ["daemon"] diff --git a/entrypoint.sh b/entrypoint.sh index 1b1d7f4..d430f10 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -19,4 +19,4 @@ mousepad = false EOF fi -exec "$@" +exec /usr/bin/kcd "$@" From 7ea1b27f22caefe92a0c146513d709793cab114b Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Wed, 27 May 2026 17:56:25 +0300 Subject: [PATCH 03/25] sftp: add mountWithBody volume path parameter and RequestAndMountVolume method mountWithBody now accepts an optional volumePath parameter. When empty, existing behaviour is preserved (auto-select first volume from multiPaths). RequestAndMountVolume follows the same subscribe-request-wait pattern as RequestAndMount but accepts a specific volume path to mount. If the path is empty, available volumes are returned without mounting (list mode). Extracted buildVolumes helper to avoid duplicating the volume-building logic in Volumes() and the new method. --- internal/plugins/sftp/sftp.go | 106 +++++++++++++++++++++++++++++----- 1 file changed, 90 insertions(+), 16 deletions(-) diff --git a/internal/plugins/sftp/sftp.go b/internal/plugins/sftp/sftp.go index 5f0aa68..3080ce7 100644 --- a/internal/plugins/sftp/sftp.go +++ b/internal/plugins/sftp/sftp.go @@ -190,7 +190,7 @@ func (p *SftpPlugin) RequestAndMount(ctx context.Context, dev device.Sender) (st if !exists { return "", fmt.Errorf("credentials missing after event (internal error)") } - return p.mountWithBody(ctx, dev.ID(), body) + return p.mountWithBody(ctx, dev.ID(), body, "") case <-deadline.Done(): return "", fmt.Errorf("timed out after %s waiting for SFTP response — is the KDE Connect app open on the phone?", timeout) @@ -198,6 +198,65 @@ func (p *SftpPlugin) RequestAndMount(ctx context.Context, dev device.Sender) (st } } +// RequestAndMountVolume sends the SFTP request, waits for credentials, then +// mounts the specified volume. If volumePath is empty, the available volumes +// are returned without mounting (list mode). The caller is responsible for +// closing the returned closer when done with the mounted path. +func (p *SftpPlugin) RequestAndMountVolume(ctx context.Context, dev device.Sender, volumePath string) (mountPath string, volumes []StorageVolume, err error) { + if p.bus == nil { + return "", nil, fmt.Errorf("event bus not available") + } + + sub := p.bus.Subscribe(0, events.TypeSftpMount) + defer sub.Close() + + if err := p.RequestMount(dev); err != nil { + return "", nil, fmt.Errorf("send SFTP request: %w", err) + } + + p.logger.Info("SFTP request sent, waiting for phone response", zap.String("device", dev.ID())) + + timeout := time.Duration(p.cfg.CredentialsTimeoutSecs) * time.Second + if timeout == 0 { + timeout = 20 * time.Second + } + deadline, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + for { + select { + case evt, ok := <-sub.C: + if !ok { + return "", nil, fmt.Errorf("event bus closed") + } + if evt.DeviceID != dev.ID() { + continue + } + p.mu.RLock() + body, exists := p.lastBody[dev.ID()] + p.mu.RUnlock() + if !exists { + return "", nil, fmt.Errorf("credentials missing after event (internal error)") + } + + vols := p.buildVolumes(body) + + if volumePath == "" { + return "", vols, nil + } + + path, err := p.mountWithBody(ctx, dev.ID(), body, volumePath) + if err != nil { + return "", nil, err + } + return path, vols, nil + + case <-deadline.Done(): + return "", nil, fmt.Errorf("timed out after %s waiting for SFTP response — is the KDE Connect app open on the phone?", timeout) + } + } +} + // MountLocally mounts using previously cached credentials. // Prefer RequestAndMount for a one-step experience. func (p *SftpPlugin) MountLocally(ctx context.Context, deviceID string) (string, error) { @@ -207,11 +266,13 @@ func (p *SftpPlugin) MountLocally(ctx context.Context, deviceID string) (string, if !ok { return "", fmt.Errorf("no SFTP credentials cached for device %s — use 'kcd sftp mount' which requests them automatically", deviceID) } - return p.mountWithBody(ctx, deviceID, body) + return p.mountWithBody(ctx, deviceID, body, "") } // mountWithBody performs the sshfs mount and returns the local browse path. -func (p *SftpPlugin) mountWithBody(ctx context.Context, deviceID string, body SftpBody) (string, error) { +// volumePath specifies which storage volume to mount. If empty, the first +// available volume is selected automatically. +func (p *SftpPlugin) mountWithBody(ctx context.Context, deviceID string, body SftpBody, volumePath string) (string, error) { baseDir := p.cfg.MountDir if baseDir == "" { baseDir = os.TempDir() @@ -224,13 +285,17 @@ func (p *SftpPlugin) mountWithBody(ctx context.Context, deviceID string, body Sf // Determine the remote path on the Android device. // The Android SFTP server exposes the real filesystem at "/". // Listing "/" via sshfs fails because it contains permission-denied - // entries (/proc, /sys). Instead, mount directly to the first storage + // entries (/proc, /sys). Instead, mount directly to a storage // volume (e.g. /storage/emulated/0) which is guaranteed browsable. - remotePath := "" - if len(body.MultiPaths) > 0 { - remotePath = body.MultiPaths[0] - } else if body.Path != "" && body.Path != "/" { - remotePath = body.Path + // If a specific volumePath is provided, use it; otherwise auto-select + // the first available volume. + remotePath := volumePath + if remotePath == "" { + if len(body.MultiPaths) > 0 { + remotePath = body.MultiPaths[0] + } else if body.Path != "" && body.Path != "/" { + remotePath = body.Path + } } remoteRoot := fmt.Sprintf("%s@%s:%s", body.User, body.IP, remotePath) @@ -339,13 +404,10 @@ func (p *SftpPlugin) Info(deviceID string) *SftpInfo { return info } -// Volumes returns the list of available storage volumes from cached credentials. -// Returns nil if no credentials or no multiPaths data. -func (p *SftpPlugin) Volumes(deviceID string) []StorageVolume { - p.mu.RLock() - defer p.mu.RUnlock() - body, ok := p.lastBody[deviceID] - if !ok || len(body.MultiPaths) == 0 { +// buildVolumes constructs a StorageVolume slice from a SftpBody. +// Caller must hold at least a read lock on p.mu if body comes from p.lastBody. +func (p *SftpPlugin) buildVolumes(body SftpBody) []StorageVolume { + if len(body.MultiPaths) == 0 { return nil } volumes := make([]StorageVolume, 0, len(body.MultiPaths)) @@ -359,6 +421,18 @@ func (p *SftpPlugin) Volumes(deviceID string) []StorageVolume { return volumes } +// Volumes returns the list of available storage volumes from cached credentials. +// Returns nil if no credentials or no multiPaths data. +func (p *SftpPlugin) Volumes(deviceID string) []StorageVolume { + p.mu.RLock() + defer p.mu.RUnlock() + body, ok := p.lastBody[deviceID] + if !ok { + return nil + } + return p.buildVolumes(body) +} + func (p *SftpPlugin) OnConnect(_ device.Sender) {} func (p *SftpPlugin) OnDisconnect(dev device.Sender) { From 57394c28b941ffed5f0b4792c3d3ddf5412f95c8 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Wed, 27 May 2026 17:57:11 +0300 Subject: [PATCH 04/25] cli: add kcd sftp browse subcommand Adds sftp_browse IPC command with volume resolution logic (index, name, or path matching). The daemon handler requests fresh credentials, then either lists volumes or mounts the chosen one via sshfs. New IPC types: - CmdSftpBrowse constant - SftpBrowsePayload (deviceId + optional volume) - SftpBrowseResponse (optional path + volumes) New client method: - SftpBrowse(deviceID, volume) returns (mountPath, volumes, err) CLI: - kcd sftp browse list volumes (fresh creds) - kcd sftp browse mount volume by index/name/path --- cmd/kcd/cli_sftp.go | 55 +++++++++++++++++++ internal/daemon/ipc_routes_sftp.go | 86 ++++++++++++++++++++++++++++++ internal/ipc/proto.go | 13 +++++ pkg/client/client.go | 19 +++++++ 4 files changed, 173 insertions(+) diff --git a/cmd/kcd/cli_sftp.go b/cmd/kcd/cli_sftp.go index 99938bd..7ef2e46 100644 --- a/cmd/kcd/cli_sftp.go +++ b/cmd/kcd/cli_sftp.go @@ -137,5 +137,60 @@ Safe to call even if already unmounted (returns error in that case).`, return nil }, }, + { + Name: "browse", + Usage: "Request fresh credentials and browse or mount a storage volume", + ArgsUsage: " [volume-index|volume-name|volume-path]", + Description: `Request fresh SFTP credentials from the phone and either list +available volumes or mount a specific one. + +Without a volume argument, lists available volumes with their index, name, and path. + +With a volume argument (index, name, or path), mounts that volume via sshfs and +opens it in the default file manager. + +Examples: + kcd sftp browse myphone + kcd sftp browse myphone 0 + kcd sftp browse myphone "SD card" + kcd sftp browse myphone /storage/ABCD-1234`, + 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 + } + + volume := "" + if c.NArg() > 1 { + volume = c.Args().Get(1) + } + + fmt.Println("Requesting SFTP credentials from phone (waiting up to 20s)…") + path, volumes, err := cl.SftpBrowse(c.Args().First(), volume) + if err != nil { + return err + } + + if path != "" { + fmt.Printf("Mounted at: %s\n", path) + return nil + } + + if len(volumes) == 0 { + fmt.Println("No storage volumes reported by device.") + return nil + } + + fmt.Println("Available volumes:") + for i, v := range volumes { + fmt.Printf(" %d. %-30s %s\n", i, v.Name, v.Path) + } + fmt.Println("\nMount a volume: kcd sftp browse ") + return nil + }, + }, }, } diff --git a/internal/daemon/ipc_routes_sftp.go b/internal/daemon/ipc_routes_sftp.go index 5d14719..8f7011a 100644 --- a/internal/daemon/ipc_routes_sftp.go +++ b/internal/daemon/ipc_routes_sftp.go @@ -3,6 +3,8 @@ package daemon import ( "context" "encoding/json" + "strconv" + "strings" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/ipc" @@ -10,6 +12,42 @@ import ( "github.com/bethropolis/kcd/internal/plugins/sftp" ) +// resolveVolume resolves a user-supplied volume argument (index, name, or path) +// against a list of StorageVolume. Returns the matching path or empty string. +func resolveVolume(arg string, volumes []ipc.StorageVolumeResponse) string { + if len(volumes) == 0 { + return "" + } + + // Try index first. + if idx, err := strconv.Atoi(arg); err == nil && idx >= 0 && idx < len(volumes) { + return volumes[idx].Path + } + + // Try name match (case-insensitive). + for _, v := range volumes { + if strings.EqualFold(v.Name, arg) { + return v.Path + } + } + + // Try path match (exact). + for _, v := range volumes { + if v.Path == arg { + return v.Path + } + } + + // Try path match (case-insensitive). + for _, v := range volumes { + if strings.EqualFold(v.Path, arg) { + return v.Path + } + } + + return "" +} + 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 @@ -95,4 +133,52 @@ func registerSftpRoutes(handler *ipc.Handler, devices *device.Registry, plugins } 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) + + 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 + } + } + } + + 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} + } + + resp := ipc.SftpBrowseResponse{ + Path: mountPath, + Volumes: vols, + } + data, _ := json.Marshal(resp) + return ipc.Response{OK: true, Data: data} + }) } diff --git a/internal/ipc/proto.go b/internal/ipc/proto.go index a9b1c9b..cc7f784 100644 --- a/internal/ipc/proto.go +++ b/internal/ipc/proto.go @@ -38,6 +38,7 @@ const ( CmdMprisStatus = "mpris_status" CmdMprisAction = "mpris_action" CmdMprisRemote = "mpris_remote" + CmdSftpBrowse = "sftp_browse" ) // ConnectPayload carries the target IP for the CmdConnect command. @@ -139,6 +140,18 @@ type StorageVolumeResponse struct { Path string `json:"path"` } +// SftpBrowsePayload is used for CmdSftpBrowse. +type SftpBrowsePayload struct { + DeviceID string `json:"deviceId"` + Volume string `json:"volume,omitempty"` +} + +// SftpBrowseResponse is returned by CmdSftpBrowse. +type SftpBrowseResponse struct { + Path string `json:"path,omitempty"` + Volumes []StorageVolumeResponse `json:"volumes,omitempty"` +} + type MprisPlayerInfo struct { DisplayName string `json:"displayName"` BusName string `json:"busName"` diff --git a/pkg/client/client.go b/pkg/client/client.go index c97daf7..7f240b2 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -228,6 +228,25 @@ func (c *Client) SftpUnmount(deviceID string) error { return err } +// SftpBrowse requests fresh SFTP credentials from the phone and either lists +// available volumes (volume arg empty) or mounts the specified volume. +// volume can be an index (0-based), volume name, or path. +// Returns the mount path (empty if listing) and available volumes. +func (c *Client) SftpBrowse(deviceID string, volume string) (string, []ipc.StorageVolumeResponse, error) { + resp, err := c.Call(ipc.CmdSftpBrowse, ipc.SftpBrowsePayload{ + DeviceID: deviceID, + Volume: volume, + }) + if err != nil { + return "", nil, err + } + var result ipc.SftpBrowseResponse + if len(resp.Data) > 0 { + _ = json.Unmarshal(resp.Data, &result) + } + return result.Path, result.Volumes, nil +} + // BroadcastStart asks the daemon to begin UDP/mDNS broadcasting. func (c *Client) BroadcastStart() error { _, err := c.Call(ipc.CmdBroadcastStart, nil) From b4e275a8918999ac36dd6f28eca73fe4f8938aa7 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Wed, 27 May 2026 17:57:38 +0300 Subject: [PATCH 05/25] docs: add IPC protocol and client developer guides, update CLI docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two new reference documents: - docs/IPC_PROTOCOL.md — complete wire protocol reference (28 commands, 27 event types, packet tables, walkthroughs) - docs/CLIENT_GUIDE.md — client developer guide with Python examples, feature patterns, and integration templates Updates docs/CLI.md: - Adds sftp browse subcommand docs - Adds mpris raw section - Fills event type table with 6 previously missing types - Fixes share.complete payload schema Updates packaging/kcd.fish-completion: - Adds missing event types to watch --events completions Updates AGENTS.md: - Adds step 8 to plugin checklist (documentation requirement) --- AGENTS.md | 12 + docs/CLI.md | 106 ++- docs/CLIENT_GUIDE.md | 696 +++++++++++++++++++ docs/IPC_PROTOCOL.md | 1187 +++++++++++++++++++++++++++++++++ packaging/kcd.fish-completion | 7 +- 5 files changed, 2002 insertions(+), 6 deletions(-) create mode 100644 docs/CLIENT_GUIDE.md create mode 100644 docs/IPC_PROTOCOL.md diff --git a/AGENTS.md b/AGENTS.md index 720c54b..3fe4477 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -185,6 +185,18 @@ const CmdMyAction = "my_action" **7. Add config field to `packaging/kcd.example.toml`** +**8. Document in `docs/IPC_PROTOCOL.md` and `docs/CLIENT_GUIDE.md`** + +- Add the new IPC command to the command reference table (section 3) with + request/response payload schemas. +- If the plugin introduces new event types, add them to the event types + section (section 5) with full JSON payload examples. +- If the plugin sends or receives new KDE Connect packet types, add entries + to the outbound (section 6) or inbound (section 7) packet reference tables. +- Add the new CLI command to the command reference in `docs/CLI.md`. +- Cover error cases in the walkthrough—what happens when the device is + disconnected, the plugin is disabled, or the payload is malformed. + ### Plugin lookup To get a plugin and type-assert to its concrete type from an IPC handler: diff --git a/docs/CLI.md b/docs/CLI.md index 42ebcf5..a5e66e2 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -381,6 +381,29 @@ kcd mpris seek -10s kcd mpris seek 1m30s ``` +### mpris raw + +Dump raw MPRIS debug state for all local media players as JSON. Shows player +names, their running status, and device-to-player mappings. Useful for +diagnosing "no player found" issues. + +``` +kcd mpris raw +``` + +Returns the same data as the `mpris_status` IPC command. + +```json +{ + "watcherRunning": true, + "deviceCount": 2, + "players": ["spotify", "firefox"], + "playerMappings": { + "a1b2c3d4e5f6_...": "spotify" + } +} +``` + ### Scripting examples ```bash @@ -570,6 +593,46 @@ Calls `fusermount3` (or `fusermount` on older systems) and removes the temporary mount point directory. Returns an error if the device was never mounted in this daemon session. +### sftp browse + +Request fresh SFTP credentials and list available storage volumes or mount +a specific one immediately. + +``` +kcd sftp browse [volume-index|volume-name|volume-path] +``` + +**Without a volume argument** — requests fresh credentials and lists volumes: + +``` +$ kcd sftp browse a1b2c3d4 +Requesting SFTP credentials from phone (waiting up to 20s)… +Available volumes: + 0. Internal shared storage /storage/emulated/0 + 1. SD card /storage/ABCD-1234 + +Mount a volume: kcd sftp browse +``` + +**With a volume argument** — requests fresh credentials and mounts the +specified volume via sshfs, opening it in the default file manager: + +``` +$ kcd sftp browse a1b2c3d4 "SD card" +Requesting SFTP credentials from phone (waiting up to 20s)… +Mounted at: /home/user/Downloads/kcd/mnt/kcd-sftp-a1b2c3d4 +``` + +The volume argument is resolved in this order: + +1. **Index** (0-based) — `0`, `1`, etc. +2. **Name** (case-insensitive) — `"SD card"`, `"internal shared storage"` +3. **Path** (exact then case-insensitive) — `/storage/ABCD-1234` + +This is the recommended way to mount a specific phone volume without having +to `request` + `mount` separately. It always fetches fresh credentials, +so the credentials are guaranteed valid. + --- ## run @@ -615,19 +678,46 @@ kcd run exec a1b2... uptime ## sms -Send an SMS via a connected phone. +Send and receive SMS via a connected phone. + +### sms send + +Send an SMS message. ``` -kcd sms +kcd sms send ``` **Example** ```bash -kcd sms a1b2... +1555000111 "Heading home in 10" +kcd sms send a1b2... +1555000111 "Heading home in 10" ``` -> Incoming SMS threads are not yet supported. Only sending is implemented. +### sms conversations + +Request a list of SMS conversation threads. Results arrive as `sms.incoming` events. + +``` +kcd sms conversations +``` + +### sms conversation + +Request messages from a specific conversation thread. + +``` +kcd sms conversation +``` + +### sms attachment + +Request an MMS attachment file from a device. The file is saved locally and an +`sms.attachment` event is emitted with the path. + +``` +kcd sms attachment +``` --- @@ -666,7 +756,7 @@ kcd watch [--events ] [--json] | `notification` | Notification from the phone: `{appName, title, text, id, ...}` | | `notification.canceled` | The phone dismissed a notification: `{id}` | | `share.progress` | File transfer progress: `{file, current, total}` | -| `share.complete` | File transfer finished: `{file, path}` | +| `share.complete` | File transfer finished: `{file, success, error?}` | | `share.text` | Plain text received: `{text}` | | `share.url` | URL received: `{url}` | | `ping.received` | A ping arrived | @@ -674,8 +764,14 @@ kcd watch [--events ] [--json] | `telephony.missed` | Missed call: `{contactName, phoneNumber}` | | `telephony.canceled` | Call ended | | `connectivity.update` | Signal strength: `{signal, networkType}` | +| `volume.update` | Device volume changed: `{name, volume, muted}` | | `mpris.update` | Now playing: `{player, title, artist, album, isPlaying, pos, length, volume}` | | `sftp.mount` | SFTP credentials: `{uri, ip, port, user, password, path, multiPaths, pathNames, errorMessage}` | +| `battery.threshold` | Battery low/full alert: `{charge, charging, event}` | +| `telephony.talking` | Call in progress: `{contactName, phoneNumber}` | +| `sms.incoming` | SMS/MMS received: `{body, sender, date, thread_id, read}` | +| `sms.attachment` | MMS attachment downloaded: `{filename, path, thread_id}` | +| `ring.received` | Phone wants this PC to ring | ### Examples diff --git a/docs/CLIENT_GUIDE.md b/docs/CLIENT_GUIDE.md new file mode 100644 index 0000000..2f7af7d --- /dev/null +++ b/docs/CLIENT_GUIDE.md @@ -0,0 +1,696 @@ +# Client Developer Guide + +This guide explains how to write a client (GUI, CLI, widget) that communicates +with the `kcd` daemon through its Unix socket IPC interface. The daemon is +headless — there is no built-in GUI. Everything a client can do goes through +the IPC protocol documented in [`IPC_PROTOCOL.md`](IPC_PROTOCOL.md). + +If you are building a Rust/GTK4 client, see the separate reference documents +in [`../../rust/kcd-client/`](../../rust/kcd-client/) (`PROJECT_GUIDE.md`, +`AGENT.md`, `ROADMAP.md`). + +--- + +## 1. Finding the Socket + +The IPC socket path depends on the platform: + +- **Linux (systemd user session):** `/run/user//kcd/kcd.sock` +- **Linux (no systemd):** `$XDG_RUNTIME_DIR/kcd/kcd.sock` +- **Fallback:** Run `kcd doctor` which prints the socket path and whether the + daemon is reachable. + +The socket is a Unix stream socket with `S_IRUSR|S_IWUSR` permissions (the +owning user only). The directory `kcd/` is created inside the runtime dir. + +**Python: Helper to find the socket:** + +```python +import os +import pwd + +def socket_path(): + uid = os.getuid() + runtime_dir = os.environ.get("XDG_RUNTIME_DIR", f"/run/user/{uid}") + return os.path.join(runtime_dir, "kcd", "kcd.sock") +``` + +--- + +## 2. Connecting + +All IPC communication uses newline-delimited JSON (NDJSON) over a Unix stream +socket. Send one JSON object per line, terminated by `\n`. Read one line per +response. + +**Python: Basic request-response:** + +```python +import json +import socket + +def ipc_request(sock, cmd, payload=None): + req = {"cmd": cmd} + if payload is not None: + req["payload"] = payload + sock.sendall((json.dumps(req) + "\n").encode()) + # Read one line + buf = b"" + while True: + c = sock.recv(1) + if c == b"\n" or not c: + break + buf += c + return json.loads(buf.decode()) + +sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) +sock.connect(socket_path()) + +resp = ipc_request(sock, "devices") +if resp["ok"]: + for dev in resp["data"]: + print(f'{dev["name"]} ({dev["id"]}) — {dev["state"]}') +``` + +--- + +## 3. Device Lifecycle + +### 3.1 Listing Devices + +The `devices` command returns all known devices (paired and unpaired). + +```python +resp = ipc_request(sock, "devices") +for dev in resp["data"]: + print(f'{dev["name"]} — {dev["state"]} — connected={dev["connected"]}') +``` + +States: `UNPAIRED`, `PAIR_REQUESTED`, `PAIR_REQUESTED_BY_PEER`, `PAIRED`. + +### 3.2 Pairing Flow + +Pairing requires a persistent watch connection to receive the pairing request +event. + +**Step 1: Start listening for a pair request (in a thread or second socket):** + +```python +import threading, json, socket + +def listen_for_pair(): + lsock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + lsock.connect(socket_path()) + lsock.sendall(b'{"cmd":"pair_listen"}\n') + buf = b"" + while True: + c = lsock.recv(1) + if c == b"\n" or not c: + break + buf += c + resp = json.loads(buf.decode()) + if resp["ok"]: + data = resp["data"] + print(f'Pair request from: {data["deviceName"]}') + print(f'Verification key: {data["verificationKey"]}') + # User verifies the key matches on both devices + return data["deviceId"] + return None + +# Run in background: +pair_thread = threading.Thread(target=listen_for_pair, daemon=True) +pair_thread.start() +``` + +**Step 2: Have the user initiate pairing on the phone** (KDE Connect → kcd +device → Request Pair). The `pair_listen` handler will return the request. + +**Step 3: Accept the pairing:** + +```python +resp = ipc_request(sock, "pair", {"deviceId": device_id, "accept": True}) +if resp["ok"]: + print("Paired successfully!") +``` + +### 3.3 Unpairing + +```python +resp = ipc_request(sock, "unpair", {"deviceId": device_id}) +``` + +--- + +## 4. Watching Events + +The `watch` command creates a persistent connection that streams live events. + +### 4.1 Basic Watch Loop + +```python +import json +import socket +import threading + +class KcdWatcher: + def __init__(self, socket_path, event_types=None): + self.socket_path = socket_path + self.event_types = event_types + self._running = False + self._callbacks = {} + + def on(self, event_type, callback): + self._callbacks.setdefault(event_type, []).append(callback) + + def _read_line(self, sock): + buf = b"" + while True: + c = sock.recv(1) + if c == b"\n" or not c: + break + buf += c + return buf.decode() + + def start(self): + self._running = True + thread = threading.Thread(target=self._run, daemon=True) + thread.start() + + def stop(self): + self._running = False + + def _run(self): + import time + backoff = 1 + while self._running: + try: + sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + sock.settimeout(30) + sock.connect(self.socket_path) + + payload = None + if self.event_types: + payload = {"events": self.event_types} + req = {"cmd": "watch"} + if payload: + req["payload"] = payload + sock.sendall((json.dumps(req) + "\n").encode()) + + # Consume the ack line + ack = self._read_line(sock) + if not json.loads(ack).get("ok"): + continue + + backoff = 1 # Reset on successful connect + while self._running: + line = self._read_line(sock) + if not line: + break + event = json.loads(line) + etype = event.get("type") + if etype in self._callbacks: + for cb in self._callbacks[etype]: + cb(event["deviceId"], event.get("payload")) + + except (socket.error, json.JSONDecodeError, ConnectionError) as e: + print(f"Watch error: {e}") + finally: + try: + sock.close() + except Exception: + pass + + # Exponential backoff 1s → 30s + if self._running: + time.sleep(backoff) + backoff = min(backoff * 2, 30) +``` + +### 4.2 Using the Watcher + +```python +w = KcdWatcher(socket_path(), ["battery.update", "notification", "mpris.update"]) + +@w.on("battery.update") +def on_battery(device_id, payload): + print(f'Battery: {payload["charge"]}% ({"charging" if payload["charging"] else "discharging"})') + +@w.on("notification") +def on_notification(device_id, payload): + print(f'{payload["appName"]}: {payload["title"]} — {payload["text"]}') + +@w.on("mpris.update") +def on_mpris(device_id, payload): + if payload and payload.get("isPlaying"): + print(f'Now playing: {payload["title"]} by {payload["artist"]}') + else: + print("Paused/stopped") + +w.start() + +# Keep main thread alive +import time +try: + while True: + time.sleep(1) +except KeyboardInterrupt: + w.stop() +``` + +### 4.3 Event Types Quick Reference + +| Filter string | When it fires | +|---|---| +| `device.connected` | TCP connection established | +| `device.disconnected` | TCP connection lost | +| `battery.update` | Battery level or charging state changed | +| `notification` | Push notification from device | +| `share.progress` | File transfer progress update | +| `share.complete` | File transfer finished | +| `mpris.update` | Now-playing state changed | +| `sms.incoming` | SMS/MMS received | +| `pair.requested` | Remote device wants to pair | +| `ping.received` | Ping from device | + +See [`IPC_PROTOCOL.md §5`](IPC_PROTOCOL.md#5-event-types) for the full list. + +--- + +## 5. Sending Commands + +### 5.1 Request Battery State + +```python +resp = ipc_request(sock, "battery", {"deviceId": dev_id}) +if resp["ok"]: + data = resp["data"] + print(f'Battery: {data["charge"]}% (charging: {data["charging"]})') +``` + +### 5.2 Send a Ping + +```python +ipc_request(sock, "ping", {"deviceId": dev_id}) +``` + +The phone should vibrate/show a notification. + +### 5.3 Share a File + +```python +ipc_request(sock, "share", {"deviceId": dev_id, "file": "/path/to/file.pdf"}) +``` + +### 5.4 MPRIS Control + +```python +# Play/Pause +ipc_request(sock, "mpris_action", {"deviceId": dev_id, "action": "playpause"}) + +# Next track +ipc_request(sock, "mpris_action", {"deviceId": dev_id, "action": "next"}) + +# Set volume +ipc_request(sock, "mpris_action", {"deviceId": dev_id, "action": "setVolume", "value": 50}) +``` + +### 5.5 Send SMS + +```python +ipc_request(sock, "send_sms", { + "deviceId": dev_id, + "phoneNumber": "+1234567890", + "message": "Hello from kcd!" +}) +``` + +### 5.6 Ring the Phone + +```python +ipc_request(sock, "findmyphone", {"deviceId": dev_id}) +# or: ipc_request(sock, "ring", {"deviceId": dev_id}) +``` + +### 5.7 Lock/Unlock + +```python +ipc_request(sock, "lock", {"deviceId": dev_id}) +ipc_request(sock, "unlock", {"deviceId": dev_id}) +``` + +### 5.8 Push Clipboard + +```python +ipc_request(sock, "clipboard_push", {"deviceId": dev_id}) +``` + +The daemon reads the local clipboard (`wl-paste`/`xclip`) and sends it. + +### 5.9 Get SFTP Connection Info + +```python +resp = ipc_request(sock, "sftp_info", {"deviceId": dev_id}) +if resp["ok"]: + info = resp["data"] + print(f'SSH: {info["user"]}@{info["ip"]} -p {info["port"]}') +``` + +### 5.10 Get Daemon Status + +```python +resp = ipc_request(sock, "status") +if resp["ok"]: + s = resp["data"] + print(f'kcd v{s["version"]} — {s["uptimeHuman"]} uptime') + print(f'Plugins: {", ".join(s["plugins"])}') + print(f'{s["connectedCount"]}/{s["deviceCount"]} devices connected') +``` + +--- + +## 6. Handling Payloads + +Each event type carries a different payload shape. Here are the common ones: + +### Battery Update + +```json +{"charge": 85, "charging": true} +``` + +### Notification + +```json +{ + "appName": "Signal", + "title": "Alice", + "text": "See you later", + "requestReplyId": "reply-123", + "id": "notif-456" +} +``` + +- `requestReplyId` is present only for notifications that support inline + replies. Use it with the `notify_reply` command. + +### Share Progress + +```json +{"file": "video.mp4", "current": 1048576, "total": 8388608} +``` + +### MPRIS Update + +```json +{ + "player": "spotify", + "title": "Song Title", + "artist": "Artist", + "album": "Album", + "isPlaying": true, + "pos": 45000, + "length": 240000, + "volume": 80, + "canControl": true, + "shuffle": false, + "loopStatus": "None" +} +``` + +### SMS Incoming + +```json +{ + "body": "Hello!", + "sender": "+1234567890", + "date": 1716800000000, + "type": 1, + "thread_id": 42, + "read": false +} +``` + +### Device Connected + +```json +{"id": "a1b2...", "name": "Pixel 9", "type": "phone"} +``` + +--- + +## 7. Building Features + +### 7.1 File Transfer Tracker + +Watch for `share.progress` and `share.complete` events to build a transfer +progress UI: + +```python +transfers = {} + +@w.on("share.progress") +def on_progress(dev_id, payload): + fname = payload["file"] + cur, total = payload["current"], payload["total"] + pct = (cur / total) * 100 if total > 0 else 0 + transfers[fname] = pct + print(f'{fname}: {pct:.0f}%') + +@w.on("share.complete") +def on_complete(dev_id, payload): + fname = payload["file"] + if payload["success"]: + print(f'{fname}: complete!') + else: + print(f'{fname}: FAILED — {payload.get("error", "unknown")}') + transfers.pop(fname, None) +``` + +### 7.2 Notification Inbox + +Subscribe to all `notification` events and build an inbox: + +```python +inbox = [] + +@w.on("notification") +def on_notification(dev_id, payload): + inbox.append({ + "app": payload["appName"], + "title": payload["title"], + "text": payload["text"], + "time": import_time.time(), + "can_reply": "requestReplyId" in payload + }) + # Keep last 50 + if len(inbox) > 50: + inbox.pop(0) + +def reply_to_notification(dev_id, notif_id, message): + ipc_request(command_sock, "notify_reply", { + "deviceId": dev_id, + "replyId": notif_id, + "message": message + }) +``` + +### 7.3 SMS Viewer + +```python +conversations = {} + +@w.on("sms.incoming") +def on_sms(dev_id, payload): + thread_id = payload["thread_id"] + if thread_id not in conversations: + conversations[thread_id] = [] + conversations[thread_id].append({ + "from": payload["sender"], + "body": payload["body"], + "date": payload["date"], + "type": "received" if payload["type"] == 1 else "sent" + }) + +def send_sms(dev_id, phone_number, message): + ipc_request(command_sock, "send_sms", { + "deviceId": dev_id, + "phoneNumber": phone_number, + "message": message + }) +``` + +### 7.4 Remote Media Controller + +```python +class MediaController: + def __init__(self, sock, dev_id): + self.sock = sock + self.dev_id = dev_id + self.now_playing = None + + def refresh(self): + resp = ipc_request(self.sock, "mpris_action", { + "deviceId": self.dev_id, + "action": "playpause" # triggers state push + }) + + def play_pause(self): + ipc_request(self.sock, "mpris_action", { + "deviceId": self.dev_id, "action": "playpause" + }) + + def next(self): + ipc_request(self.sock, "mpris_action", { + "deviceId": self.dev_id, "action": "next" + }) + + def previous(self): + ipc_request(self.sock, "mpris_action", { + "deviceId": self.dev_id, "action": "previous" + }) + + def set_volume(self, vol): + ipc_request(self.sock, "mpris_action", { + "deviceId": self.dev_id, "action": "setVolume", "value": vol + }) +``` + +--- + +## 8. Error Recovery + +### 8.1 Daemon Not Running + +```python +def check_daemon(path): + try: + sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + sock.settimeout(2) + sock.connect(path) + sock.close() + return True + except (FileNotFoundError, ConnectionRefusedError, socket.error): + return False +``` + +### 8.2 Watch Reconnection + +The `KcdWatcher` class above implements exponential backoff (1s → 30s max). +The reconnection strategy should: + +1. Close the old socket on any read error. +2. Wait with backoff (1s, 2s, 4s, 8s, 16s, 30s) before retrying. +3. Reset backoff to 1s on successful connection. +4. Re-send the `watch` request with the same event type filter. +5. After reconnecting, wait for the initial state dump to catch up on missed + state (device.connected events will re-establish connectivity). + +### 8.3 Device Disconnection + +When a device disconnects: +- A `device.disconnected` event is emitted. +- The device remains in `PAIRED` state and will auto-reconnect when the phone + re-enters the network (if LAN broadcast is enabled). +- Your UI should show the device as offline but keep its configuration. +- On reconnection, a `device.connected` event is emitted followed by the state + dump for that device. + +### 8.4 Pairing Loss + +If the phone is reset or the app is reinstalled: +- The device ID changes (permanent identifier generated on first app launch). +- The old device entry will never reconnect. +- Remove the stale entry with `unpair` and initiate a fresh pairing. + +--- + +## 9. Integration Examples + +### 9.1 Desktop Widget (Waybar) + +The `desktop-integration/` directory contains ready-to-use scripts. For a +simple waybar custom module: + +```bash +#!/bin/bash +# ~/.config/waybar/scripts/kcd-custom.sh +while true; do + socat - UNIX-CONNECT:/run/user/$(id -u)/kcd/kcd.sock <<'EOF' | while read -r line; do +{"cmd":"watch","payload":{"events":["battery.update","mpris.update"]}} +EOF + [ "$line" = '{"ok":true}' ] && continue + echo "$line" | jq -c 'select(.type=="battery.update") | {text: ("🔋 \(.payload.charge)%")}' + done + sleep 1 +done +``` + +See `desktop-integration/README.md` for the maintained integration scripts. + +### 9.2 Minimal Python Event Monitor + +```python +#!/usr/bin/env python3 +"""Simple kcd event monitor.""" +import json, socket, sys, os + +SOCK = os.path.join( + os.environ.get("XDG_RUNTIME_DIR", f"/run/user/{os.getuid()}"), + "kcd", "kcd.sock" +) + +def main(): + sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + sock.connect(SOCK) + sock.sendall(b'{"cmd":"watch","payload":{"events":' + + sys.argv[1:].encode() + b'}}\n') + # Discard ack + ack = sock.makefile("r").readline() + for line in sock.makefile("r"): + event = json.loads(line) + print(json.dumps(event, indent=2)) + +if __name__ == "__main__": + main() +``` + +Usage: `python3 monitor.py '["battery.update","mpris.update"]'` + +### 9.3 GTK4/Shell Proxy + +For desktop shell widgets (eww, ags, quickshell), run `kcd watch` in the +background and pipe the JSON output to a named pipe or parse it directly: + +```bash +# In the shell widget's startup: +kcd watch --json '["battery.update","mpris.update"]' | while read -r line; do + [ "$line" = '{"ok":true}' ] && continue + # Parse and update widget state +done +``` + +--- + +## 10. Going Further + +### Rust/GTK4 Client + +A reference Rust GTK4 client implementation is being developed separately at +[`../../rust/kcd-client/`](../../rust/kcd-client/). Its `PROJECT_GUIDE.md`, +`AGENT.md`, and `ROADMAP.md` documents provide additional architectural +context for building a full-featured GUI client. + +### Protocol-Level Implementation + +If you are implementing a full KDE Connect network-level client (not just IPC), +refer to the upstream protocol spec at +. + +### Getting Help + +- **Daemon issues:** `/home/bet/Projects/go/kde-connect` — open a GitHub + issue. +- **Protocol questions:** The IPC protocol reference in + [`IPC_PROTOCOL.md`](IPC_PROTOCOL.md) is the source of truth. +- **API stability:** The IPC protocol is considered stable within a major + version. Breaking changes will be documented in release notes. diff --git a/docs/IPC_PROTOCOL.md b/docs/IPC_PROTOCOL.md new file mode 100644 index 0000000..033936f --- /dev/null +++ b/docs/IPC_PROTOCOL.md @@ -0,0 +1,1187 @@ +# IPC Protocol Reference + +kcd exposes a Unix socket for inter-process communication. This document is the +authoritative reference for client authors who want to build tools against the +daemon without importing Go packages. + +--- + +## 1. Transport + +- **Socket path:** `$XDG_RUNTIME_DIR/kcd/kcd.sock` (typically + `/run/user//kcd/kcd.sock`) +- **Type:** Unix stream socket (`SOCK_STREAM`) +- **Framing:** Newline-delimited JSON (NDJSON). Every message is a single JSON + object terminated by `\n`. The daemon reads one line per request from its + `bufio.Scanner`. +- **Max request payload:** No explicit limit (scanner reads a single line; Unix + socket buffer is the practical bound). Do not send payloads exceeding ~1 MiB + over IPC; use the side-channel TLS port for file transfers instead. + +--- + +## 2. Request / Response Format + +### Request + +```json +{"cmd": "", "payload": } +``` + +- `cmd` (`string`, required) — the command name. +- `payload` (`JSON`, optional) — per-command parameters. Parsed as + `json.RawMessage` by the handler. If absent, the field must be omitted or + `null`. + +### Response (non-watch commands) + +```json +{"ok": true, "data": } +{"ok": false, "error": "human-readable message"} +``` + +- `ok` (`bool`) — success. +- `error` (`string`, present only when `ok` is `false`) — error description. +- `data` (`JSON`, present only when `ok` is `true` and the command has a + payload) — response payload. Refer to the per-command reference for its shape. + +### Error responses + +All errors return the same shape. The `error` string is human-readable and may +change between releases. Do not parse it programmatically beyond logging. + +```json +{"ok": false, "error": "device not found"} +``` + +--- + +## 3. Command Reference + +Every command, its request payload, and its response data shape. + +### 3.1 Built-in Commands (handler.go) + +#### `devices` + +List all known devices (paired + unpaired). + +**Request payload:** none + +**Response data:** `[]DeviceInfo` + +```json +{ + "ok": true, + "data": [ + { + "id": "a1b2c3d4e5f6_...", + "name": "Pixel 9", + "type": "phone", + "state": "PAIRED", + "cert_fp": "", + "last_seen": "0001-01-01T00:00:00Z", + "connected": true + } + ] +} +``` + +Fields: + +| Field | Type | Description | +|---|---|---| +| `id` | string | Permanent device identifier | +| `name` | string | Human-readable device name | +| `type` | string | `"phone"`, `"tablet"`, `"laptop"`, `"desktop"` | +| `state` | string | `"UNPAIRED"`, `"PAIR_REQUESTED"`, `"PAIR_REQUESTED_BY_PEER"`, `"PAIRED"`, `"UNKNOWN"` | +| `cert_fp` | string | Not populated in this response (empty) | +| `last_seen` | string (RFC3339) | Not populated in this response (zero time) | +| `connected` | bool | Whether the device currently has an active TCP connection | + +#### `pair` + +Initiate pairing with a device, or respond to an incoming pairing request. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +Optional fields: + +```json +{"deviceId": "a1b2c3d4e5f6_...", "accept": true, "reject": false} +``` + +- If neither `accept` nor `reject` is set, sends a pair request to the device. +- If `accept: true`, accepts an incoming pair request from the device. +- If `reject: true`, rejects or unpairs. + +**Response data:** none (`{"ok": true}`) + +#### `pair_listen` + +Wait for an incoming pairing request and return the verification key. + +**Request payload:** none + +**Response data:** `PairListenResult` + +```json +{ + "ok": true, + "data": { + "deviceId": "a1b2c3d4e5f6_...", + "deviceName": "Pixel 9", + "verificationKey": "ABCD1234EFGH5678" + } +} +``` + +The handler blocks until a pair request arrives or the context is cancelled. +The 16-character verification key should be displayed to the user to verify +identity match on both sides. + +#### `unpair` + +Remove a paired device. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** none + +#### `ping` + +Send a ping packet to a device. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** none + +### 3.2 Plugin Commands (registered via `handler.Register`) + +#### `connect` + +Connect to a device by IP address. Used when LAN broadcast is unavailable or +the device is on a different subnet. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_...", "ip": "192.168.1.42"} +``` + +**Response data:** none + +#### `broadcast_start` / `broadcast_stop` + +Disable/enable LAN UDP broadcasting at runtime. + +**Request payload:** none + +**Response data:** none + +#### `status` + +Return daemon status info. + +**Request payload:** none + +**Response data:** `StatusResponse` + +```json +{ + "ok": true, + "data": { + "version": "1.13.0", + "startedAt": "2026-05-27T10:00:00Z", + "uptimeHuman": "2h34m", + "socketPath": "/run/user/1000/kcd/kcd.sock", + "configPath": "/home/user/.config/kcd/kcd.toml", + "plugins": ["pair", "ping", "battery", "share", "sftp", "clipboard", "mpris", "notification", "sms", "telephony", "connectivity", "systemvolume", "mousepad", "lockdevice", "findmyphone", "runcommand"], + "deviceCount": 3, + "connectedCount": 1 + } +} +``` + +#### `battery` + +Request battery state from a device. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** `{"charge": 85, "charging": true}` (the daemon waits for the +device to respond and returns the value). + +| Field | Type | Description | +|---|---|---| +| `charge` | number | Battery percentage (0–100) | +| `charging` | bool | Whether the device is currently charging | + +#### `clipboard_push` + +Push the local clipboard content to a device. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +The daemon reads the local clipboard via `wl-paste` or `xclip` and sends it. + +**Response data:** none + +#### `share` + +Send a file to a device. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_...", "file": "/path/to/file.pdf"} +``` + +**Response data:** none + +The daemon opens a side-channel TLS port for the actual file transfer. + +#### `send_sms` + +Send an SMS via a paired phone. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_...", "phoneNumber": "+1234567890", "message": "Hello!"} +``` + +**Response data:** none + +#### `sms_request_conversations` + +Request a list of SMS conversations from a device. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** none (results arrive as `sms.incoming` events if the phone +uses the deprecated event-based protocol) or via the conversation response +packet (handled internally). For client authors: subscribe to `notification` +or watch the event stream for the reply. + +#### `sms_request_conversation` + +Request messages from a specific conversation thread. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_...", "threadID": 42} +``` + +Optional fields: `rangeStartTimestamp` (int64), `numberToRequest` (int64). + +**Response data:** none + +#### `sms_request_attachment` + +Request an MMS attachment file from a device. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_...", "threadID": 42, "partID": 1, "uniqueIdentifier": "..."} +``` + +**Response data:** none (attachment arrives via side-channel transfer, emitted +as `sms.attachment` event). + +#### `call_mute` + +Mute an incoming phone call. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** none + +#### `notify_reply` + +Reply to a notification that supports inline replies. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_...", "replyId": "notification-id-here", "message": "OK, I'll be there"} +``` + +The `replyId` comes from the `requestReplyId` field of a `notification` event. + +**Response data:** none + +#### `findmyphone` (also aliased as `ring`) + +Make a paired phone ring loudly. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** none + +#### `lock` + +Lock a paired device's screen. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** none + +#### `unlock` + +Unlock a paired device's screen (if the device supports it). + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** none + +#### `run_list` + +Request a device's list of configured run commands. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** none (results arrive via `kdeconnect.runcommand` response +packet). + +#### `run_exec` + +Execute a command on a device by its command key. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_...", "key": "my_command_key"} +``` + +**Response data:** none + +#### `sftp_info` + +Get SFTP connection details for a device. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** `SftpInfo` + +```json +{ + "ok": true, + "data": { + "ip": "192.168.1.42", + "port": "31588", + "user": "u0_a123", + "password": "sftp-password-here", + "path": "/storage/emulated/0", + "volumes": [ + {"name": "Internal shared storage", "path": "/storage/emulated/0"} + ] + } +} +``` + +#### `sftp_volumes` + +List storage volumes available on a device. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** `[]StorageVolume` (array of `{"name": "...", "path": "..."}`) + +#### `sftp_mount` + +Mount a device's storage via SFTP (requires `sshfs`). + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** none + +#### `sftp_mount_local` + +Mount a device's storage at a local temporary path. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** `{"path": "/tmp/kcd-sftp-abcdef123456"}` + +#### `sftp_unmount` + +Unmount a previously mounted SFTP filesystem. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** none + +#### `sftp_browse` + +Request fresh SFTP credentials and either list available storage volumes or +mount a specific one. + +**Request payload:** + +- Without volume (list mode): `{"deviceId": "a1b2c3d4e5f6_..."}` +- With volume (mount mode): `{"deviceId": "a1b2c3d4e5f6_...", "volume": "/storage/emulated/0"}` + +The `volume` field accepts an index (0-based), volume name, or path. The +daemon resolves it against the device's reported volumes after receiving +fresh credentials. + +**Response data:** `SftpBrowseResponse` + +```json +{ + "ok": true, + "data": { + "path": "/home/user/Downloads/kcd/mnt/kcd-sftp-a1b2c3d4", + "volumes": [ + {"name": "Internal shared storage", "path": "/storage/emulated/0"}, + {"name": "SD card", "path": "/storage/ABCD-1234"} + ] + } +} +``` + +In list mode (`volume` omitted or empty), `path` is empty and `volumes` is +populated. In mount mode, `path` contains the local sshfs mount point. + +**Error cases:** + +- `"device not found"` — the device ID is unknown or was removed +- `"sftp plugin not enabled"` — SFTP is disabled in config +- `"timed out after 20s waiting for SFTP response"` — phone did not respond + +#### `mpris_status` + +Get MPRIS watcher status (which local players are tracked). + +**Request payload:** none + +**Response data:** `MprisDebugStatus` + +```json +{ + "ok": true, + "data": { + "watcherRunning": true, + "deviceCount": 2, + "players": ["spotify", "firefox"], + "playerMappings": { + "a1b2c3d4e5f6_...": "spotify" + } + } +} +``` + +#### `mpris_action` + +Send an MPRIS control action to a device. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_...", "action": "play"} +``` + +Supported actions: `"play"`, `"pause"`, `"playpause"`, `"next"`, `"previous"`, +`"stop"`, `"raise"`, `"quit"`. Volume can be set with `"setVolume"` (requires +an integer value field). Seek with `"seek"` (int64, offset in ms) or +`"setPosition"` (int64, absolute position in ms). + +**Response data:** none + +#### `mpris_remote` + +List remote MPRIS players (players on paired devices). + +**Request payload:** none + +**Response data:** `MprisRemoteResponse` + +```json +{ + "ok": true, + "data": { + "players": [ + {"deviceId": "a1b2c3d4e5f6_...", "player": "spotify"} + ] + } +} +``` + +--- + +## 4. Watch Protocol + +The `watch` command establishes a persistent connection that streams live +events as NDJSON. Unlike all other commands, the connection stays open. + +### Request + +```json +{"cmd": "watch", "payload": {"events": ["battery.update", "notification"]}} +``` + +The `payload.events` array is optional. If omitted, **all** event types are +streamed. If present, only events whose type string matches one of the entries +are delivered. + +### Initial Sequence + +1. **Ack line:** The daemon sends one `{"ok": true}` immediately upon + accepting the watch request. The client must read (and discard) this line + before processing events. + +2. **State dump:** For each **connected** device, in arbitrary order: + + **2a. `device.connected`:** + ```json + {"type":"device.connected","deviceId":"...","timestamp":"2026-05-27T10:00:00Z","payload":{"id":"...","name":"Pixel 9","type":"phone"}} + ``` + + **2b. `battery.update`:** + ```json + {"type":"battery.update","deviceId":"...","timestamp":"...","payload":{"charge":85,"charging":true}} + ``` + + **2c. `mpris.update`** (only if MPRIS plugin is registered AND the cached + state is less than 10 seconds old): + ```json + {"type":"mpris.update","deviceId":"...","timestamp":"...","payload":{...NowPlaying...}} + ``` + +3. **Live stream:** All matching events are streamed as they occur, one per + line, until the client disconnects or the daemon shuts down. + +### Event Wire Format + +```json +{ + "type": "battery.update", + "timestamp": "2026-05-27T10:00:00Z", + "deviceId": "a1b2c3d4e5f6_...", + "payload": { ... } +} +``` + +| Field | Type | Description | +|---|---|---| +| `type` | string | Event type identifier | +| `timestamp` | string (RFC3339) | UTC time when the event was published | +| `deviceId` | string | The device that triggered the event (empty for daemon-level events) | +| `payload` | JSON | Per-type payload, see section 4.2 | + +### Event Filters + +The `events` filter is applied on the server side. Only events whose type +string exactly matches one of the filters are delivered. Invalid/unknown +filter strings are silently ignored — they simply match nothing. + +### Reconnection + +The official `kcd watch` CLI client implements automatic reconnection with +exponential backoff (1 second initial, 30 second maximum, randomized jitter). +Client authors are encouraged to adopt a similar strategy. + +--- + +## 5. Event Types + +### 5.1 Device Events + +#### `device.added` + +A new device was discovered on the network. + +**Payload:** `string` (the device name) + +#### `device.removed` + +A device disappeared from the network (or was manually removed). + +**Payload:** none (`null`) + +#### `device.connected` + +A TCP connection was established with a device. + +**Payload:** + +```json +{"id": "...", "name": "Pixel 9", "type": "phone"} +``` + +#### `device.disconnected` + +A TCP connection was lost. + +**Payload:** none (`null`) + +### 5.2 Pairing Events + +#### `pair.requested` + +A remote device is requesting pairing. + +**Payload:** + +```json +{"name": "Pixel 9", "type": "phone", "verificationKey": "ABCD1234EFGH5678"} +``` + +The 16-character verification key should be displayed to the user to confirm +the same key is shown on the remote device. + +#### `pair.accepted` + +A pairing was accepted. + +**Payload:** + +```json +{"name": "Pixel 9", "type": "phone"} +``` + +#### `pair.rejected` + +A pairing request was rejected, or a device was unpaired. + +**Payload:** + +```json +{"name": "Pixel 9", "type": "phone"} +``` + +### 5.3 Battery Events + +#### `battery.update` + +Battery state changed or was requested. + +**Payload:** + +```json +{"charge": 85, "charging": true} +``` + +| Field | Type | Description | +|---|---|---| +| `charge` | number | Battery percentage (0–100) | +| `charging` | bool | Whether the device is currently charging | + +#### `battery.threshold` + +A battery threshold event was received from the device (e.g. low battery +warning). + +**Payload:** + +```json +{"charge": 15, "charging": false, "event": 1} +``` + +### 5.4 Notification Events + +#### `notification` + +A notification was received from a device. + +**Payload:** + +```json +{ + "appName": "WhatsApp", + "title": "John Doe", + "text": "See you at 5", + "requestReplyId": "reply-123", + "id": "notif-456" +} +``` + +| Field | Type | Description | +|---|---|---| +| `appName` | string | Application name (Android `appName` field) | +| `title` | string | Notification title (Android `title` field) | +| `text` | string | Notification body text (Android `ticker` field) | +| `requestReplyId` | string | Present if the notification supports inline replies | +| `id` | string | Notification identifier | + +#### `notification.canceled` + +A notification was dismissed by the device. + +**Payload:** + +```json +{"id": "notif-456"} +``` + +### 5.5 Share Events + +#### `share.progress` + +A file transfer is in progress. + +**Payload:** + +```json +{"file": "photo.jpg", "current": 524288, "total": 2097152} +``` + +| Field | Type | Description | +|---|---|---| +| `file` | string | Filename being transferred | +| `current` | number | Bytes received so far | +| `total` | number | Total file size in bytes | + +#### `share.complete` + +A file transfer completed (or failed). + +**Payload (success):** + +```json +{"file": "photo.jpg", "success": true} +``` + +**Payload (failure):** + +```json +{"file": "photo.jpg", "success": false, "error": "permission denied"} +``` + +#### `share.text` + +Text was shared from the device. + +**Payload:** + +```json +{"text": "Hello, check this out!"} +``` + +#### `share.url` + +A URL was shared from the device. + +**Payload:** + +```json +{"url": "https://example.com"} +``` + +### 5.6 Ping Events + +#### `ping.received` + +A ping was received from a device. + +**Payload:** + +```json +{"message": "Ping!"} +``` + +### 5.7 Telephony Events + +#### `telephony.ringing` + +An incoming call is ringing. + +**Payload:** + +```json +{"event": "ringing", "contactName": "John Doe", "phoneNumber": "+1234567890", "isCancel": false} +``` + +#### `telephony.talking` + +A call is in progress. + +**Payload:** same shape as `telephony.ringing` with `"event": "talking"`. + +#### `telephony.missed` + +A call was missed. + +**Payload:** same shape with `"event": "missed"`. + +#### `telephony.canceled` + +A call was cancelled. + +**Payload:** full `TelephonyBody` struct (includes `event`, `contactName`, +`phoneNumber`). + +### 5.8 Connectivity Events + +#### `connectivity.update` + +Cellular/Wi-Fi signal strength report from the device. + +**Payload:** + +```json +{ + "signalStrengths": { + "wlan0": { + "networkType": "Wi-Fi", + "networkDetailedType": "Wi-Fi", + "signalStrength": 4 + }, + "rmnet0": { + "networkType": "mobile", + "networkDetailedType": "LTE", + "signalStrength": 3 + } + } +} +``` + +### 5.9 SFTP Events + +#### `sftp.mount` + +SFTP credentials received (success) or error. + +**Payload (success):** + +```json +{ + "uri": "sftp://192.168.1.42:31588", + "ip": "192.168.1.42", + "port": "31588", + "user": "u0_a123", + "password": "sftp-password-here", + "path": "/storage/emulated/0", + "volumes": [{"name": "Internal shared storage", "path": "/storage/emulated/0"}] +} +``` + +**Payload (error):** + +```json +{"error": "SFTP server rejected credentials"} +``` + +### 5.10 Volume Events + +#### `volume.update` + +Device volume level changed. + +**Payload:** + +```json +{"name": "media", "volume": 70, "muted": false} +``` + +| Field | Type | Description | +|---|---|---| +| `name` | string | Audio stream name | +| `volume` | number | Volume level (0–100) | +| `muted` | bool | Whether the stream is muted | + +### 5.11 SMS Events + +#### `sms.incoming` + +An SMS or MMS message was received. + +**Payload:** + +```json +{ + "body": "Hello!", + "sender": "+1234567890", + "date": 1716800000000, + "type": 1, + "thread_id": 42, + "read": false, + "event": 0, + "u_id": 98765, + "sub_id": 0, + "addresses": [{"address": "+1234567890"}], + "attachments": [{"part_id": 1, "mime_type": "image/jpeg", "unique_identifier": "..."}] +} +``` + +#### `sms.attachment` + +An MMS attachment has been downloaded. + +**Payload:** + +```json +{"filename": "image.jpg", "path": "/tmp/kcd-sms-attachment-...", "thread_id": 42} +``` + +### 5.12 Ring Events + +#### `ring.received` + +A ring/find-my-phone request was received from a device (the phone is asking +this daemon to ring). + +**Payload:** none (`null`) + +### 5.13 MPRIS Events + +#### `mpris.update` + +Now-playing state from a device's media player. + +**Payload:** + +```json +{ + "player": "spotify", + "title": "Song Title", + "artist": "Artist Name", + "album": "Album Name", + "albumArtUrl": "https://i.scdn.co/image/...", + "url": "spotify:track:...", + "length": 240000, + "pos": 45000, + "isPlaying": true, + "volume": 80, + "canControl": true, + "canGoNext": true, + "canGoPrevious": true, + "canPause": true, + "canPlay": true, + "canSeek": true, + "playbackStatus": "Playing", + "shuffle": false, + "loopStatus": "None" +} +``` + +--- + +## 6. Outbound Packet Reference + +Packets that the daemon sends to remote devices. These are relevant for +understanding protocol-level interactions but are generally abstracted by the +IPC interface. Included here for completeness and for advanced client authors +who may want to implement a full network-level implementation. + +| Packet Type | Direction (Daemon → Device) | Trigger | +|---|---|---| +| `kdeconnect.identity` | Handshake | TLS identity exchange (sent twice: plaintext pre-TLS + full after TLS) | +| `kdeconnect.pair` | Pairing | Pair/unpair/reject actions | +| `kdeconnect.battery` | Battery | Reply to battery request + push on connect | +| `kdeconnect.battery.request` | Battery | Request phone battery state on connect | +| `kdeconnect.clipboard` | Clipboard | Push clipboard content to phone | +| `kdeconnect.clipboard.connect` | Clipboard | Push clipboard with timestamp on connect | +| `kdeconnect.mousepad.keyboardstate` | Mousepad | Advertise keyboard capability on connect | +| `kdeconnect.mpris` | MPRIS | Player list, NowPlaying state, seek positions, album art (broadcast + request-reply) | +| `kdeconnect.mpris.request` | MPRIS | Request player list, now-playing, volume; send control actions | +| `kdeconnect.notification.reply` | Notification | Reply to a notification with inline reply support | +| `kdeconnect.notification` | RunCommand | Command output notification pushed to phone | +| `kdeconnect.runcommand` | RunCommand | Send command list to phone | +| `kdeconnect.runcommand.request` | RunCommand | Request phone's command list / execute command | +| `kdeconnect.share.request` | Share | File transfer invitation (side-channel) | +| `kdeconnect.sftp.request` | SFTP | Request the phone to start its SFTP server | +| `kdeconnect.sms.request` | SMS | Send an SMS | +| `kdeconnect.sms.request_conversations` | SMS | Request conversation list | +| `kdeconnect.sms.request_conversation` | SMS | Request a specific thread's messages | +| `kdeconnect.sms.request_attachment` | SMS | Request an MMS attachment file | +| `kdeconnect.telephony.request_mute` | Telephony | Mute incoming call ringer | +| `kdeconnect.systemvolume` | SystemVolume | Push local sink list to phone | +| `kdeconnect.findmyphone.request` | FindMyPhone | Trigger phone ringer | +| `kdeconnect.connectivity_report.request` | Connectivity | Request signal strength report | +| `kdeconnect.ping` | Ping | Send a ping | + +--- + +## 7. Inbound Packet Reference + +Packets that the daemon handles from remote devices. Each entry shows which +plugin processes it and a link to the body struct definition. + +| Packet Type | Plugin | Body Struct | +|---|---|---| +| `kdeconnect.pair` | Pair | `PairBody{Pair bool, Timestamp int64}` | +| `kdeconnect.battery` | Battery | `BatteryBody{CurrentCharge int, IsCharging bool, ThresholdEvent int}` | +| `kdeconnect.battery.request` | Battery | (empty, triggers a battery reply) | +| `kdeconnect.notification` | Notification | `NotificationBody{ID, AppName, Title, Text, IsCancel, IsClearable, Silent, RequestReplyId string}` | +| `kdeconnect.share.request` | Share | `ShareBody{Filename, NumberOfFiles, TotalPayloadSize, LastModified, CreationTime, Text, Url}` | +| `kdeconnect.sftp` | SFTP | `SftpBody{IP, Port, User, Password, Path, MultiPaths, PathNames, ErrorMessage}` | +| `kdeconnect.ping` | Ping | `PingBody{Message string}` | +| `kdeconnect.mousepad.request` | Mousepad | `MousepadBody{Dx, Dy, X, Y, SingleClick, DoubleClick, MiddleClick, RightClick, SingleHold, SingleRel, Scroll, Key, SpecialKey, Shift, Ctrl, Alt, Super}` | +| `kdeconnect.systemvolume.request` | SystemVolume | `VolumeBody{RequestSinks, Name, Volume, Muted, MaxVolume}` | +| `kdeconnect.telephony` | Telephony | `TelephonyBody{Event, ContactName, PhoneNumber, IsCancel}` | +| `kdeconnect.sms.messages` | SMS | `SMSMessagesPacket{Version, Messages []SMSMessage}` | +| `kdeconnect.sms.attachment_file` | SMS | `AttachmentFileBody{Filename, ThreadID}` | +| `kdeconnect.findmyphone.request` | FindMyPhone | (empty, triggers ring event) | +| `kdeconnect.connectivity_report` | Connectivity | `ConnectivityBody{SignalStrengths map[string]SignalStrength}` | +| `kdeconnect.clipboard` | Clipboard | `ClipboardBody{Content string, Timestamp int64}` | +| `kdeconnect.clipboard.connect` | Clipboard | `ClipboardBody{Content string, Timestamp int64}` | +| `kdeconnect.clipboard.file` | Clipboard | `ClipboardFileBody{Filename string}` | +| `kdeconnect.lock` | LockDevice | `LockBody{RequestLocked, SetLocked, IsLocked}` | +| `kdeconnect.lock.request` | LockDevice | `LockBody{}` (triggers lock/unlock) | +| `kdeconnect.mpris` | MPRIS | `MPRISRequest{RequestPlayerList, RequestNowPlaying, RequestVolume, Player, Action, ...}` | +| `kdeconnect.mpris.request` | MPRIS | `MPRISRequest{}` (same struct, different semantics) | +| `kdeconnect.runcommand.request` | RunCommand | `RequestBody{RequestCommandList bool, Key string}` | +| `kdeconnect.presenter` | Presenter | `PresenterBody{Dx, Dy *float64, Stop *bool}` | + +--- + +## 8. Complete Walkthroughs + +### 8.1 List Devices (nc) + +```bash +$ echo '{"cmd":"devices"}' | nc -U /run/user/1000/kcd/kcd.sock +{"ok":true,"data":[{"id":"a1b2c3d4e5f6_...","name":"Pixel 9","type":"phone","state":"PAIRED","cert_fp":"","last_seen":"0001-01-01T00:00:00Z","connected":true}]} +``` + +### 8.2 Watch Events (socat) + +```bash +$ socat - UNIX-CONNECT:/run/user/1000/kcd/kcd.sock +{"cmd":"watch","payload":{"events":["battery.update","notification"]}} +``` + +After sending the request, read the ack line, then process events: + +```bash +# With jq for formatting: +$ echo '{"cmd":"watch","payload":{"events":["battery.update"]}}' | socat - UNIX-CONNECT:/run/user/1000/kcd/kcd.sock | while read -r line; do + [ "$line" = '{"ok":true}' ] && continue + echo "$line" | jq . +done +``` + +### 8.3 Pairing Flow + +1. **Start listening on the daemon:** + ```bash + $ echo '{"cmd":"pair_listen"}' | nc -U /run/user/1000/kcd/kcd.sock + ``` + + This blocks until a pair request arrives. The output: + ```json + {"ok":true,"data":{"deviceId":"a1b2...","deviceName":"Pixel 9","verificationKey":"ABCD1234EFGH5678"}} + ``` + +2. **In another terminal, accept the pairing:** + ```bash + $ echo '{"cmd":"pair","payload":{"deviceId":"a1b2...","accept":true}}' | nc -U /run/user/1000/kcd/kcd.sock + {"ok":true} + ``` + +3. **The phone shows the same verification key.** User confirms on both ends. + +### 8.4 Ping a Device + +```bash +$ echo '{"cmd":"ping","payload":{"deviceId":"a1b2..."}}' | nc -U /run/user/1000/kcd/kcd.sock +{"ok":true} +``` + +--- + +## 9. Error Handling + +### Error Response + +All commands return errors in the same format: + +```json +{"ok": false, "error": "device not found"} +``` + +Common error strings: + +| Error string | Meaning | +|---|---| +| `"device not found"` | The device ID is unknown or not paired | +| `"device not connected"` | The device is paired but not currently connected | +| `"plugin not enabled"` | The required plugin is disabled in config | +| `"invalid payload"` | The request payload could not be deserialized | +| `"command not found"` | The requested IPC command does not exist | + +### Connection Loss + +- If the daemon is not running, the socket connect will fail with `ENOENT` or + `ECONNREFUSED`. +- The daemon creates the socket directory and socket file on startup, and + removes the socket file on shutdown. The state directory (`$XDG_STATE_HOME`) + is preserved. +- For the `watch` protocol, the daemon does not send explicit keepalive + pings. A client can detect disconnection by a read returning 0 bytes or an + error. Implement reconnection with exponential backoff. + +### Concurrency + +The IPC server is single-threaded per connection; each connection is handled +in its own goroutine. Long-running commands (`pair_listen`, `watch`) block +that connection but do not block other connections. Send one request per +connection and use a separate connection for concurrent commands. diff --git a/packaging/kcd.fish-completion b/packaging/kcd.fish-completion index 22c3f1c..ff37d5b 100644 --- a/packaging/kcd.fish-completion +++ b/packaging/kcd.fish-completion @@ -157,11 +157,16 @@ share.text\t'Text shared from phone' share.url\t'URL shared from phone' ping.received\t'Ping received' telephony.ringing\t'Incoming call' +telephony.talking\t'Call in progress' telephony.missed\t'Missed call' telephony.canceled\t'Call ended' connectivity.update\t'Cell signal changed' +volume.update\t'System volume changed' sftp.mount\t'SFTP filesystem mounted' -volume.update\t'System volume changed'" +mpris.update\t'Now playing changed' +sms.incoming\t'SMS/MMS received' +sms.attachment\t'MMS attachment downloaded' +ring.received\t'Phone wants PC to ring'" # --------------------------------------------------------------------------- # sftp request|info|volumes|mount|unmount From b5694fffdfeb473d60c0c35baaaa44861e4766b0 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Wed, 27 May 2026 21:47:00 +0300 Subject: [PATCH 06/25] ci: remove kcd-bin.install from AUR config The install hook printed a message on every package update which was annoying for users. Removing the reference skips it entirely. --- .goreleaser.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.goreleaser.yaml b/.goreleaser.yaml index a6a9017..6f67a37 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -87,7 +87,6 @@ aurs: - "xdotool: for X11 mousepad support" - "wtype: for Wayland keyboard emulation" - "python-nautilus: for Nautilus file manager integration" - install: packaging/kcd-bin.install package: |- install -Dm755 "./kcd" "${pkgdir}/usr/bin/kcd" From 867e034e4c05f2d7ed3486d0a5281495f28d9367 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Tue, 9 Jun 2026 00:06:39 +0300 Subject: [PATCH 07/25] feat: auto-prune stale unpaired devices with configurable threshold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds automatic cleanup of stale devices from the registry. Devices that are neither paired nor connected are removed when their last-seen timestamp exceeds the threshold (default: 15m). This prevents old discovery entries from accumulating in `kcd devices` and `devices.json`. - LastSeen is now refreshed on every TCP connection (handleNewConnection) so active/reconnecting devices stay alive - Prune runs at daemon startup and before every saveDevices write - New config key: prune_stale_threshold (Go duration format, '0' to disable) - Silent cleanup — no events published for pruned devices --- internal/config/config.go | 13 ++++++++++++- internal/daemon/daemon.go | 8 +++++++- internal/daemon/transport.go | 1 + internal/device/registry.go | 23 +++++++++++++++++++++++ internal/ipc/handler.go | 29 ++++++++++++++++------------- internal/ipc/ipc_test.go | 2 +- packaging/kcd.example.toml | 7 +++++++ 7 files changed, 67 insertions(+), 16 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index c5acad1..3e11f88 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "strings" + "time" "github.com/BurntSushi/toml" ) @@ -38,7 +39,9 @@ type Config struct { Pairing PairingConfig `toml:"pairing"` Mousepad MousepadConfig `toml:"mousepad"` SMS SMSConfig `toml:"sms"` - ConfigPath string `toml:"-"` // populated at load time, never written to disk + PruneStaleThreshold string `toml:"prune_stale_threshold"` // auto-remove stale unpaired devices; Go duration, "0" = disable + + ConfigPath string `toml:"-"` // populated at load time, never written to disk } // PluginConfig toggles individual plugins on or off. @@ -72,6 +75,7 @@ func Defaults() *Config { c.Pairing.Defaults() c.Mousepad.Defaults() c.SMS.Defaults() + c.PruneStaleThreshold = "15m" return c } @@ -131,6 +135,13 @@ func (c *Config) Validate() error { return fmt.Errorf("config: share port_min (%d) cannot be greater than port_max (%d)", c.Share.PortMin, c.Share.PortMax) } + // Prune threshold validation + if c.PruneStaleThreshold != "" { + if _, err := time.ParseDuration(c.PruneStaleThreshold); err != nil { + return fmt.Errorf("config: invalid prune_stale_threshold %q: %w", c.PruneStaleThreshold, err) + } + } + // Mousepad backend validation switch c.Mousepad.Backend { case "auto", "ydotool", "xdotool", "uinput": diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 90d4652..2365d4c 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -87,8 +87,14 @@ func Run(ctx context.Context, cfg *config.Config) error { logger.Warn("failed to load devices", zap.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)) + } + // Helper to save device state saveDevices := func() { + devices.Prune(pruneThreshold) devs := devices.List() infos := make([]device.DeviceInfo, 0, len(devs)) for _, dev := range devs { @@ -125,7 +131,7 @@ func Run(ctx context.Context, cfg *config.Config) error { bc := discovery.NewBroadcasterController(identity, 30*time.Second, logger, devices.AllPairedDevicesConnected) // 5. IPC Server - handler := ipc.NewHandler(devices, plugins, pairPlugin, statePath, bus) + handler := ipc.NewHandler(devices, plugins, pairPlugin, statePath, bus, pruneThreshold) registerIPCRoutes(handler, cfg, devices, plugins, bc, ctx, tlsCfg, logger, startedAt) diff --git a/internal/daemon/transport.go b/internal/daemon/transport.go index fb0e606..75296ba 100644 --- a/internal/daemon/transport.go +++ b/internal/daemon/transport.go @@ -189,6 +189,7 @@ func handleNewConnection(ctx context.Context, conn *transport.Conn, identity *pr } else { dev.SetName(safeDeviceName) } + dev.SetLastSeen(time.Now()) certFP := cert.Fingerprint(peerCert) if dev.State() == device.StatePaired && dev.CertFP != "" { diff --git a/internal/device/registry.go b/internal/device/registry.go index a8ddb61..59ebed8 100644 --- a/internal/device/registry.go +++ b/internal/device/registry.go @@ -90,6 +90,29 @@ func (r *Registry) AllPairedDevicesConnected() bool { return hasPaired && allConnected } +// Prune removes devices that are stale (unpaired, disconnected, past threshold). +// It skips Paired and Connected devices. Returns the number of removed devices. +// Silent — no events are published. +func (r *Registry) Prune(threshold time.Duration) int { + if threshold <= 0 { + return 0 + } + var pruned int + r.devices.Range(func(key, value any) bool { + dev := value.(*Device) + if dev.State() == StatePaired || dev.IsConnected() { + return true + } + lastSeen := dev.LastSeen() + if lastSeen.IsZero() || time.Since(lastSeen) > threshold { + r.devices.Delete(key) + pruned++ + } + return true + }) + return pruned +} + // ReconnectBackoff calculates exponential backoff duration based on attempts. // Caps out at maxDuration. func ReconnectBackoff(attempt int, maxDuration time.Duration) time.Duration { diff --git a/internal/ipc/handler.go b/internal/ipc/handler.go index bd21d8e..fdded29 100644 --- a/internal/ipc/handler.go +++ b/internal/ipc/handler.go @@ -13,23 +13,25 @@ import ( // Handler handles incoming IPC requests. type Handler struct { - devices *device.Registry - plugins *plugin.Registry - pairPlugin *pair.PairPlugin - statePath string - bus *events.Bus - routes map[string]func(Request) Response + devices *device.Registry + plugins *plugin.Registry + pairPlugin *pair.PairPlugin + statePath string + bus *events.Bus + routes map[string]func(Request) Response + pruneThreshold time.Duration } // NewHandler creates a new IPC command handler. -func NewHandler(devices *device.Registry, plugins *plugin.Registry, pairPlugin *pair.PairPlugin, statePath string, bus *events.Bus) *Handler { +func NewHandler(devices *device.Registry, plugins *plugin.Registry, pairPlugin *pair.PairPlugin, statePath string, bus *events.Bus, pruneThreshold time.Duration) *Handler { return &Handler{ - devices: devices, - plugins: plugins, - pairPlugin: pairPlugin, - statePath: statePath, - bus: bus, - routes: make(map[string]func(Request) Response), + devices: devices, + plugins: plugins, + pairPlugin: pairPlugin, + statePath: statePath, + bus: bus, + routes: make(map[string]func(Request) Response), + pruneThreshold: pruneThreshold, } } @@ -85,6 +87,7 @@ func (h *Handler) saveDevices() { if h.statePath == "" { return } + h.devices.Prune(h.pruneThreshold) devs := h.devices.List() infos := make([]device.DeviceInfo, 0, len(devs)) for _, dev := range devs { diff --git a/internal/ipc/ipc_test.go b/internal/ipc/ipc_test.go index d8accd8..082d2cb 100644 --- a/internal/ipc/ipc_test.go +++ b/internal/ipc/ipc_test.go @@ -25,7 +25,7 @@ func TestIPCRoundTrip(t *testing.T) { devReg.Add(dev1) // Pass nil for pairPlugin (uses fallback path) and empty statePath for tests - handler := ipc.NewHandler(devReg, pluginReg, nil, "", nil) + handler := ipc.NewHandler(devReg, pluginReg, nil, "", nil, 0) server := ipc.NewServer(sockPath, handler, logger) ctx, cancel := context.WithCancel(context.Background()) diff --git a/packaging/kcd.example.toml b/packaging/kcd.example.toml index 20d5269..cb55406 100644 --- a/packaging/kcd.example.toml +++ b/packaging/kcd.example.toml @@ -32,6 +32,13 @@ # # To initiate pairing to a specific device, run `kcd pair `. +# ─── Device Management ───────────────────────────────────────────────────────── +# Automatically remove stale (unpaired, disconnected) devices from the registry. +# This prevents old discovery entries from accumulating in `kcd devices`. +# Set to "0" to disable auto-pruning entirely. +# Default: "15m" +# prune_stale_threshold = "15m" + # ─── Paths ──────────────────────────────────────────────────────────────────── # Directory where files received from paired devices are saved. From 024b131f434ce695d6b47639bbbd8b7abc76991f Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Tue, 9 Jun 2026 00:22:53 +0300 Subject: [PATCH 08/25] fix: clean Nix build artifacts before GoReleaser Adds result/ to .gitignore and restores flake.lock after Nix build step to prevent git dirty state error in GoReleaser. --- .github/workflows/release.yml | 6 ++++++ .gitignore | 3 +++ 2 files changed, 9 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 673faaa..8e2e261 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -64,6 +64,12 @@ jobs: if: hashFiles('flake.nix') != '' run: nix build + - name: Clean up Nix artifacts before release + if: hashFiles('flake.nix') != '' + run: | + rm -rf result* + git checkout flake.lock + - name: Run GoReleaser uses: goreleaser/goreleaser-action@v6 with: diff --git a/.gitignore b/.gitignore index 41c798b..40d51ee 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,9 @@ __pycache__/ +# Nix build outputs +result* + # Code coverage profiles and other test artifacts *.out coverage.* From 41cb267aec908bcce4a15af0694599e31b2d062e Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Tue, 9 Jun 2026 00:26:32 +0300 Subject: [PATCH 09/25] fix: restore flake.lock from HEAD instead of index git checkout flake.lock only restores from the index, but if Nix also stages the file, the index copy is still dirty. Using git checkout HEAD -- flake.lock properly discards both staged and working-tree changes. --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8e2e261..8866de6 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -68,7 +68,7 @@ jobs: if: hashFiles('flake.nix') != '' run: | rm -rf result* - git checkout flake.lock + git checkout HEAD -- flake.lock - name: Run GoReleaser uses: goreleaser/goreleaser-action@v6 From 3cd24c3a2bdb69420fc1e30bba362416307bfe87 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Tue, 9 Jun 2026 00:32:58 +0300 Subject: [PATCH 10/25] refactor: move Nix verification to post-release workflow Nix build creates flake.lock and a result symlink, which dirties the git tree and blocks GoReleaser's --clean check. Moving Nix steps to a separate workflow triggered by workflow_run eliminates this conflict entirely. --- .github/workflows/nix-verify.yml | 30 ++++++++++++++++++++++++++++++ .github/workflows/release.yml | 20 -------------------- 2 files changed, 30 insertions(+), 20 deletions(-) create mode 100644 .github/workflows/nix-verify.yml diff --git a/.github/workflows/nix-verify.yml b/.github/workflows/nix-verify.yml new file mode 100644 index 0000000..fdfe744 --- /dev/null +++ b/.github/workflows/nix-verify.yml @@ -0,0 +1,30 @@ +name: Nix + +on: + workflow_run: + workflows: ["Release"] + types: + - completed + +permissions: + contents: read + +jobs: + verify: + name: Nix build package + runs-on: ubuntu-latest + if: ${{ github.event.workflow_run.conclusion == 'success' }} + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Install Nix + uses: cachix/install-nix-action@v31 + with: + github_access_token: ${{ secrets.GITHUB_TOKEN }} + + - name: Flake schema check + run: nix flake check --no-build + + - name: Nix build package + run: nix build diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8866de6..7b690b9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -50,26 +50,6 @@ jobs: - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 - - name: Install Nix - if: hashFiles('flake.nix') != '' - uses: cachix/install-nix-action@v31 - with: - github_access_token: ${{ secrets.GITHUB_TOKEN }} - - - name: Flake schema check - if: hashFiles('flake.nix') != '' - run: nix flake check --no-build - - - name: Nix build package - if: hashFiles('flake.nix') != '' - run: nix build - - - name: Clean up Nix artifacts before release - if: hashFiles('flake.nix') != '' - run: | - rm -rf result* - git checkout HEAD -- flake.lock - - name: Run GoReleaser uses: goreleaser/goreleaser-action@v6 with: From 99143e9f025cb005b1c07cece3203242a21c2511 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Tue, 9 Jun 2026 00:51:27 +0300 Subject: [PATCH 11/25] style: fix gofmt alignment in config.go --- internal/config/config.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 3e11f88..027c833 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -27,19 +27,19 @@ type Config struct { LogLevel string `toml:"log_level"` // "debug", "info", "warn", "error" (or "quiet") // AutoAcceptPairing was removed in favor of `kcd pair` (listen mode). // Old config values are silently ignored by the TOML parser. - Plugins PluginConfig `toml:"plugins"` - Commands map[string]string `toml:"commands"` - CommandsPerDevice map[string]map[string]string `toml:"commands_per_device"` - Notifications NotificationConfig `toml:"notifications"` - Battery BatteryConfig `toml:"battery"` - Notification NotificationPluginConfig `toml:"notification_plugin"` - Share ShareConfig `toml:"share"` - SFTP SFTPConfig `toml:"sftp"` - Ping PingConfig `toml:"ping"` - Pairing PairingConfig `toml:"pairing"` - Mousepad MousepadConfig `toml:"mousepad"` - SMS SMSConfig `toml:"sms"` - PruneStaleThreshold string `toml:"prune_stale_threshold"` // auto-remove stale unpaired devices; Go duration, "0" = disable + Plugins PluginConfig `toml:"plugins"` + Commands map[string]string `toml:"commands"` + CommandsPerDevice map[string]map[string]string `toml:"commands_per_device"` + Notifications NotificationConfig `toml:"notifications"` + Battery BatteryConfig `toml:"battery"` + Notification NotificationPluginConfig `toml:"notification_plugin"` + Share ShareConfig `toml:"share"` + SFTP SFTPConfig `toml:"sftp"` + Ping PingConfig `toml:"ping"` + Pairing PairingConfig `toml:"pairing"` + Mousepad MousepadConfig `toml:"mousepad"` + SMS SMSConfig `toml:"sms"` + PruneStaleThreshold string `toml:"prune_stale_threshold"` // auto-remove stale unpaired devices; Go duration, "0" = disable ConfigPath string `toml:"-"` // populated at load time, never written to disk } From a7970d5e27a4fec5731a3eac5f7da2792fbfc6b0 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Thu, 18 Jun 2026 00:39:55 +0300 Subject: [PATCH 12/25] fix: prevent CPU spin from auto-reconnect backoff overflow ReconnectBackoff shift overflowed int64 at attempt >= 54, returning 0 instead of capping, causing time.After(0) to fire immediately in a tight loop (25% CPU on one core). Fix 1: cap attempt at 31 before shift to avoid overflow entirely. Fix 2: add reconnecting guard to Device via atomic.Bool to prevent multiple concurrent reconnect goroutines from piling up. --- internal/daemon/transport.go | 8 ++++++++ internal/device/device.go | 18 ++++++++++++++++++ internal/device/registry.go | 13 ++++++++++--- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/internal/daemon/transport.go b/internal/daemon/transport.go index 75296ba..0ce77c5 100644 --- a/internal/daemon/transport.go +++ b/internal/daemon/transport.go @@ -227,6 +227,12 @@ func handleNewConnection(ctx context.Context, conn *transport.Conn, identity *pr if lastIP == nil { return } + // 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())) + return + } go reconnectWithBackoff(ctx, sender, lastIP, identity, cfg, devices, plugins, localDeviceID, logger) } @@ -256,6 +262,8 @@ func reconnectWithBackoff( const maxBackoff = 5 * time.Minute attempt := 0 + defer dev.ReconnectDone() + logger.Info("starting auto-reconnect", zap.String("device_id", dev.ID()), zap.String("device_name", dev.Name()), diff --git a/internal/device/device.go b/internal/device/device.go index 68f0953..a027eac 100644 --- a/internal/device/device.go +++ b/internal/device/device.go @@ -5,6 +5,7 @@ import ( "crypto/x509" "net" "sync" + "sync/atomic" "time" "github.com/bethropolis/kcd/internal/events" @@ -38,6 +39,10 @@ type Device struct { mu sync.RWMutex + // reconnecting is an atomic flag preventing multiple concurrent + // auto-reconnect goroutines for this device. + reconnecting atomic.Bool + // pluginDispatch routes incoming packets to registered plugins pluginDispatch func(ctx context.Context, dev *Device, pkt *protocol.Packet) bool onConnect func(dev *Device) @@ -296,3 +301,16 @@ func (d *Device) SetLastSeen(t time.Time) { defer d.mu.Unlock() d.lastSeen = t } + +// TryReconnect attempts to mark the device as reconnecting. +// Returns true if this goroutine should proceed; false if another +// reconnect goroutine is already running. +func (d *Device) TryReconnect() bool { + return d.reconnecting.CompareAndSwap(false, true) +} + +// ReconnectDone marks the device as no longer reconnecting. +// Must be called (typically via defer) after a reconnect goroutine exits. +func (d *Device) ReconnectDone() { + d.reconnecting.Store(false) +} diff --git a/internal/device/registry.go b/internal/device/registry.go index 59ebed8..0f4dbe4 100644 --- a/internal/device/registry.go +++ b/internal/device/registry.go @@ -117,10 +117,17 @@ func (r *Registry) Prune(threshold time.Duration) int { // Caps out at maxDuration. func ReconnectBackoff(attempt int, maxDuration time.Duration) time.Duration { if attempt <= 0 { - return time.Second * 2 + return 2 * time.Second } - dur := time.Duration(1< maxDuration || dur < 0 { // < 0 checks integer overflow + // Guard against shift overflow. On 64-bit, 1<= 54 (producing 0 due to modular multiplication), and + // on 32-bit, 1<<32 wraps to 0. Cap early — the backoff is already + // enormous at attempt=30 (~68 years if uncapped). + if attempt >= 31 { + return maxDuration + } + dur := time.Duration(1< maxDuration || dur <= 0 { return maxDuration } return dur From e64c60bcedcb904ff31ad4f6abc348b5fce4bb7d Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Thu, 18 Jun 2026 01:28:59 +0300 Subject: [PATCH 13/25] fix: detect clipboard backend at runtime instead of wayland-1 systemd user services don't inherit desktop env vars, so os.Getenv("WAYLAND_DISPLAY") was always empty, forcing xclip even under Wayland. Replace static isWayland bool with detectBackend() that probes exec.LookPath("wl-paste") -> exec.LookPath("xclip") at first use, cached via sync.Once. Also log Warn when wl-copy/xclip fails instead of silent discard. --- internal/plugins/clipboard/clipboard.go | 62 ++++++++++++++++++++----- 1 file changed, 51 insertions(+), 11 deletions(-) diff --git a/internal/plugins/clipboard/clipboard.go b/internal/plugins/clipboard/clipboard.go index 8fed684..041d2b4 100644 --- a/internal/plugins/clipboard/clipboard.go +++ b/internal/plugins/clipboard/clipboard.go @@ -20,12 +20,21 @@ import ( "go.uber.org/zap" ) +type clipboardBackend int + +const ( + backendUnknown clipboardBackend = iota + backendWayland + backendX11 +) + // ClipboardPlugin handles clipboard sync both directions. type ClipboardPlugin struct { lastTimestamp int64 tlsConfig *tls.Config logger *zap.Logger - isWayland bool + backend clipboardBackend + backendOnce sync.Once mu sync.Mutex lastContent string // last content received from phone (inbound) lastPushedContent string // last content sent to phone (outbound) @@ -36,10 +45,26 @@ func NewClipboardPlugin(tlsConfig *tls.Config, logger *zap.Logger) *ClipboardPlu return &ClipboardPlugin{ tlsConfig: tlsConfig, logger: logger.With(zap.String("plugin", "clipboard")), - isWayland: os.Getenv("WAYLAND_DISPLAY") != "", } } +// detectBackend probes for a working clipboard tool (wl-paste → xclip) +// and caches the result so LookPath is called at most once. +func (p *ClipboardPlugin) detectBackend() clipboardBackend { + p.backendOnce.Do(func() { + if _, err := exec.LookPath("wl-paste"); err == nil { + p.backend = backendWayland + p.logger.Debug("clipboard: using Wayland backend (wl-paste/wl-copy)") + } else if _, err := exec.LookPath("xclip"); err == nil { + p.backend = backendX11 + p.logger.Debug("clipboard: using X11 backend (xclip)") + } else { + p.logger.Warn("clipboard: no clipboard tool found (install wl-clipboard or xclip)") + } + }) + return p.backend +} + // ClipboardBody represents the content of a clipboard packet. type ClipboardBody struct { Content string `json:"content"` @@ -100,14 +125,19 @@ func (p *ClipboardPlugin) Handle(ctx context.Context, dev device.Sender, pkt *pr // Spawning goroutine as Handlers must not block. go func() { var cmd *exec.Cmd - if p.isWayland { + switch p.detectBackend() { + case backendWayland: cmd = exec.CommandContext(context.Background(), "wl-copy") - } else { + case backendX11: cmd = exec.CommandContext(context.Background(), "xclip", "-selection", "clipboard") + default: + return } cmd.Stdin = strings.NewReader(body.Content) - _ = cmd.Run() + if err := cmd.Run(); err != nil { + p.logger.Warn("clipboard: failed to set clipboard", zap.Error(err)) + } }() return nil @@ -175,10 +205,13 @@ func (p *ClipboardPlugin) handleClipboardFile(ctx context.Context, dev device.Se defer t.Close() var cmd *exec.Cmd - if p.isWayland { + switch p.detectBackend() { + case backendWayland: cmd = exec.CommandContext(context.Background(), "wl-copy", "--type", mimeType) - } else { + case backendX11: cmd = exec.CommandContext(context.Background(), "xclip", "-selection", "clipboard", "-t", mimeType, "-i") + default: + return } cmd.Stdin = t if out, err := cmd.CombinedOutput(); err != nil { @@ -221,10 +254,13 @@ func downloadToFile(ctx context.Context, ip net.IP, port int, size int64, dest s // Push copies the local clipboard to the remote device using wl-paste or xclip -o. func Push(ctx context.Context, dev device.Sender, p *ClipboardPlugin) error { var cmd *exec.Cmd - if p.isWayland { + switch p.detectBackend() { + case backendWayland: cmd = exec.CommandContext(ctx, "wl-paste", "-n") - } else { + case backendX11: cmd = exec.CommandContext(ctx, "xclip", "-selection", "clipboard", "-o") + default: + return fmt.Errorf("clipboard: no clipboard tool available") } out, err := cmd.Output() @@ -261,13 +297,17 @@ func Push(ctx context.Context, dev device.Sender, p *ClipboardPlugin) error { func (p *ClipboardPlugin) readClipboard() string { var cmd *exec.Cmd - if p.isWayland { + switch p.detectBackend() { + case backendWayland: cmd = exec.CommandContext(context.Background(), "wl-paste", "-n") - } else { + case backendX11: cmd = exec.CommandContext(context.Background(), "xclip", "-selection", "clipboard", "-o") + default: + return "" } out, err := cmd.Output() if err != nil { + p.logger.Debug("clipboard: read failed", zap.Error(err)) return "" } return string(out) From 69073355deb1523fbb9f6cbaafc56c8fad20f8cc Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Thu, 18 Jun 2026 02:51:12 +0300 Subject: [PATCH 14/25] feat: add RemoteSystemVolume plugin (kcd volume) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New plugin for controlling phone audio volume from the CLI — the inverse of the existing SystemVolume plugin (phone controls PC volume). - Handles incoming kdeconnect.systemvolume (sink list + per-sink updates from phone), sends kdeconnect.systemvolume.request (volume/mute commands to phone) - Caches sink list per-device via sync.Map; OnConnect auto-requests sink list - IPC: remote_volume_list, remote_volume_set, remote_volume_mute - CLI: kcd volume list/set/mute subcommands - Events: reuses volume.update for both per-sink and full sink-list payloads - Default: enabled (remotesystemvolume = true) --- cmd/kcd/cli_volume.go | 98 ++++++++++++ cmd/kcd/main.go | 1 + docs/CLI.md | 54 +++++++ docs/CLIENT_GUIDE.md | 26 +++- docs/IPC_PROTOCOL.md | 67 +++++++- internal/config/plugins.go | 38 ++--- internal/daemon/ipc_routes.go | 3 + internal/daemon/ipc_routes_remotevolume.go | 80 ++++++++++ internal/daemon/plugins.go | 4 + internal/ipc/proto.go | 3 + .../remotesystemvolume/remotesystemvolume.go | 146 ++++++++++++++++++ packaging/kcd.example.toml | 7 + pkg/client/client.go | 31 +++- 13 files changed, 536 insertions(+), 22 deletions(-) create mode 100644 cmd/kcd/cli_volume.go create mode 100644 internal/daemon/ipc_routes_remotevolume.go create mode 100644 internal/plugins/remotesystemvolume/remotesystemvolume.go diff --git a/cmd/kcd/cli_volume.go b/cmd/kcd/cli_volume.go new file mode 100644 index 0000000..fd1cc58 --- /dev/null +++ b/cmd/kcd/cli_volume.go @@ -0,0 +1,98 @@ +package main + +import ( + "encoding/json" + "fmt" + + "github.com/bethropolis/kcd/internal/plugins/remotesystemvolume" + "github.com/urfave/cli/v2" +) + +var volumeCmd = &cli.Command{ + Name: "volume", + Usage: "Control remote device volume", + Subcommands: []*cli.Command{ + { + Name: "list", + Usage: "List audio sinks on a remote device", + 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 + } + data, err := cl.RemoteVolumeList(c.Args().First()) + if err != nil { + return err + } + var sinks []remotesystemvolume.SinkInfo + if err := json.Unmarshal(data, &sinks); err != nil { + return fmt.Errorf("decode sinks: %w", err) + } + if len(sinks) == 0 { + fmt.Println("No sinks known — connect to the device first.") + return nil + } + for _, s := range sinks { + muted := "" + if s.Muted { + muted = " (MUTED)" + } + fmt.Printf("%s %d%%%s\n", s.Name, s.Volume, muted) + } + return nil + }, + }, + { + Name: "set", + Usage: "Set volume on a remote device sink", + ArgsUsage: " <0-100>", + Action: func(c *cli.Context) error { + if c.NArg() < 3 { + return fmt.Errorf("usage: kcd volume set <0-100>") + } + cl, err := getClient(c) + if err != nil { + return err + } + volume := c.Args().Get(2) + var vol int + if _, err := fmt.Sscanf(volume, "%d", &vol); err != nil || vol < 0 || vol > 100 { + return fmt.Errorf("volume must be 0-100") + } + if err := cl.RemoteVolumeSet(c.Args().First(), c.Args().Get(1), vol); err != nil { + return err + } + fmt.Println("Volume set.") + return nil + }, + }, + { + Name: "mute", + Usage: "Mute or unmute a remote device sink", + ArgsUsage: " ", + Action: func(c *cli.Context) error { + if c.NArg() < 3 { + return fmt.Errorf("usage: kcd volume mute ") + } + cl, err := getClient(c) + if err != nil { + return err + } + muted := c.Args().Get(2) == "true" + if err := cl.RemoteVolumeMute(c.Args().First(), c.Args().Get(1), muted); err != nil { + return err + } + state := "muted" + if !muted { + state = "unmuted" + } + fmt.Printf("Sink %s.\n", state) + return nil + }, + }, + }, +} diff --git a/cmd/kcd/main.go b/cmd/kcd/main.go index 3209eaa..2d4e07b 100644 --- a/cmd/kcd/main.go +++ b/cmd/kcd/main.go @@ -125,6 +125,7 @@ func main() { runCmd, smsCmd, mprisCmd, + volumeCmd, { Name: "doctor", diff --git a/docs/CLI.md b/docs/CLI.md index a5e66e2..9dc45c8 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -721,6 +721,60 @@ kcd sms attachment --- +## volume + +Control the remote device's audio volume (requires `remotesystemvolume` plugin). + +### volume list + +List audio sinks on a remote device and their current volume/mute state. + +``` +kcd volume list +``` + +**Example output** + +``` +media 75% +alarm 100% +``` + +### volume set + +Set a sink's volume (0–100) on a remote device. + +``` +kcd volume set <0-100> +``` + +**Examples** + +```bash +kcd volume set a1b2... media 50 +kcd volume set a1b2... alarm 80 +``` + +### volume mute + +Mute or unmute a sink on a remote device. + +``` +kcd volume mute +``` + +**Examples** + +```bash +kcd volume mute a1b2... media true # Mute +kcd volume mute a1b2... media false # Unmute +``` + +> Sink names and current volume can be discovered with `kcd volume list`. +> Volume changes made on the phone are also published as `volume.update` events. + +--- + ## watch Monitor real-time events from the daemon as an NDJSON stream. This is the primary way to observe what is happening across all devices. diff --git a/docs/CLIENT_GUIDE.md b/docs/CLIENT_GUIDE.md index 2f7af7d..347fd18 100644 --- a/docs/CLIENT_GUIDE.md +++ b/docs/CLIENT_GUIDE.md @@ -346,7 +346,31 @@ ipc_request(sock, "clipboard_push", {"deviceId": dev_id}) The daemon reads the local clipboard (`wl-paste`/`xclip`) and sends it. -### 5.9 Get SFTP Connection Info +### 5.9 Remote Volume Control + +```python +# List audio sinks +resp = ipc_request(sock, "remote_volume_list", {"deviceId": dev_id}) +if resp["ok"]: + for sink in resp["data"]: + print(f'{sink["name"]}: {sink["volume"]}% (muted: {sink["muted"]})') + +# Set volume +ipc_request(sock, "remote_volume_set", { + "deviceId": dev_id, "name": "media", "volume": 50 +}) + +# Mute/unmute +ipc_request(sock, "remote_volume_mute", { + "deviceId": dev_id, "name": "media", "muted": True +}) +``` + +Volume changes from the phone arrive as `volume.update` events. The event payload +is either `{"name", "volume", "muted"}` for a single sink change or `{"sinks": [...]}` +for the full sink list. + +### 5.10 Get SFTP Connection Info ```python resp = ipc_request(sock, "sftp_info", {"deviceId": dev_id}) diff --git a/docs/IPC_PROTOCOL.md b/docs/IPC_PROTOCOL.md index 033936f..50f8fe0 100644 --- a/docs/IPC_PROTOCOL.md +++ b/docs/IPC_PROTOCOL.md @@ -555,6 +555,57 @@ an integer value field). Seek with `"seek"` (int64, offset in ms) or **Response data:** none +#### `remote_volume_list` + +List the last known audio sinks for a device. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_..."} +``` + +**Response data:** `[]SinkInfo` + +```json +{ + "ok": true, + "data": [ + {"name": "media", "description": "Media", "volume": 75, "muted": false, "maxVolume": 100} + ] +} +``` + +Returns an empty array if the plugin has not yet received a sink list from the device (connect to the device first). + +**Error cases:** +- `"device not found"` — the device ID is unknown or was removed +- `"remotesystemvolume plugin not enabled"` — plugin disabled in config + +#### `remote_volume_set` + +Set the volume of a specific audio sink on a remote device (0–100). + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_...", "name": "media", "volume": 50} +``` + +**Response data:** none + +#### `remote_volume_mute` + +Mute or unmute a specific audio sink on a remote device. + +**Request payload:** + +```json +{"deviceId": "a1b2c3d4e5f6_...", "name": "media", "muted": true} +``` + +**Response data:** none + #### `mpris_remote` List remote MPRIS players (players on paired devices). @@ -933,9 +984,9 @@ SFTP credentials received (success) or error. #### `volume.update` -Device volume level changed. +Device volume level changed (sent in two shapes). -**Payload:** +**Payload (per-sink update):** ```json {"name": "media", "volume": 70, "muted": false} @@ -947,6 +998,16 @@ Device volume level changed. | `volume` | number | Volume level (0–100) | | `muted` | bool | Whether the stream is muted | +**Payload (full sink list):** + +```json +{ + "sinks": [ + {"name": "media", "description": "Media", "volume": 75, "muted": false, "maxVolume": 100} + ] +} +``` + ### 5.11 SMS Events #### `sms.incoming` @@ -1054,6 +1115,7 @@ who may want to implement a full network-level implementation. | `kdeconnect.sms.request_attachment` | SMS | Request an MMS attachment file | | `kdeconnect.telephony.request_mute` | Telephony | Mute incoming call ringer | | `kdeconnect.systemvolume` | SystemVolume | Push local sink list to phone | +| `kdeconnect.systemvolume.request` | RemoteSystemVolume | Set phone volume/mute or request sink list | | `kdeconnect.findmyphone.request` | FindMyPhone | Trigger phone ringer | | `kdeconnect.connectivity_report.request` | Connectivity | Request signal strength report | | `kdeconnect.ping` | Ping | Send a ping | @@ -1090,6 +1152,7 @@ plugin processes it and a link to the body struct definition. | `kdeconnect.mpris.request` | MPRIS | `MPRISRequest{}` (same struct, different semantics) | | `kdeconnect.runcommand.request` | RunCommand | `RequestBody{RequestCommandList bool, Key string}` | | `kdeconnect.presenter` | Presenter | `PresenterBody{Dx, Dy *float64, Stop *bool}` | +| `kdeconnect.systemvolume` | RemoteSystemVolume | `VolumeBody{SinkList, Name, Volume, Muted}` | --- diff --git a/internal/config/plugins.go b/internal/config/plugins.go index b6990a6..2a09fb3 100644 --- a/internal/config/plugins.go +++ b/internal/config/plugins.go @@ -6,24 +6,25 @@ import ( ) type PluginConfig struct { - Battery bool `toml:"battery"` - Clipboard bool `toml:"clipboard"` - Notification bool `toml:"notification"` - Share bool `toml:"share"` - RunCommand bool `toml:"runcommand"` - MPRIS bool `toml:"mpris"` - Ping bool `toml:"ping"` - Telephony bool `toml:"telephony"` - Connectivity bool `toml:"connectivity"` - Mousepad bool `toml:"mousepad"` - SFTP bool `toml:"sftp"` - FindMyPhone bool `toml:"findmyphone"` - LockDevice bool `toml:"lockdevice"` - SystemVolume bool `toml:"systemvolume"` - PauseMusic bool `toml:"pausemusic"` - SMS bool `toml:"sms"` - Presenter bool `toml:"presenter"` - FindThisDevice bool `toml:"findthisdevice"` + Battery bool `toml:"battery"` + Clipboard bool `toml:"clipboard"` + Notification bool `toml:"notification"` + Share bool `toml:"share"` + RunCommand bool `toml:"runcommand"` + MPRIS bool `toml:"mpris"` + Ping bool `toml:"ping"` + Telephony bool `toml:"telephony"` + Connectivity bool `toml:"connectivity"` + Mousepad bool `toml:"mousepad"` + SFTP bool `toml:"sftp"` + FindMyPhone bool `toml:"findmyphone"` + LockDevice bool `toml:"lockdevice"` + SystemVolume bool `toml:"systemvolume"` + PauseMusic bool `toml:"pausemusic"` + SMS bool `toml:"sms"` + Presenter bool `toml:"presenter"` + FindThisDevice bool `toml:"findthisdevice"` + RemoteSystemVolume bool `toml:"remotesystemvolume"` } type BatteryConfig struct { @@ -101,6 +102,7 @@ func (p *PluginConfig) Defaults() { p.SMS = true p.Presenter = true p.FindThisDevice = true + p.RemoteSystemVolume = true } func (c *BatteryConfig) Defaults() { diff --git a/internal/daemon/ipc_routes.go b/internal/daemon/ipc_routes.go index 5156b45..2757671 100644 --- a/internal/daemon/ipc_routes.go +++ b/internal/daemon/ipc_routes.go @@ -46,6 +46,9 @@ func registerIPCRoutes(handler *ipc.Handler, cfg *config.Config, devices *device if cfg.Plugins.MPRIS { registerMprisRoutes(handler, devices, plugins) } + if cfg.Plugins.RemoteSystemVolume { + registerRemoteVolumeRoutes(handler, devices, plugins) + } handler.Register(ipc.CmdConnect, func(req ipc.Request) ipc.Response { var p ipc.ConnectPayload diff --git a/internal/daemon/ipc_routes_remotevolume.go b/internal/daemon/ipc_routes_remotevolume.go new file mode 100644 index 0000000..24c991a --- /dev/null +++ b/internal/daemon/ipc_routes_remotevolume.go @@ -0,0 +1,80 @@ +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/plugins/remotesystemvolume" +) + +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} + }) + + 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} + }) + + 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} + }) +} + +func mustJSON(v any) []byte { + b, _ := json.Marshal(v) + return b +} diff --git a/internal/daemon/plugins.go b/internal/daemon/plugins.go index 465552d..e5d6df6 100644 --- a/internal/daemon/plugins.go +++ b/internal/daemon/plugins.go @@ -20,6 +20,7 @@ import ( "github.com/bethropolis/kcd/internal/plugins/pair" "github.com/bethropolis/kcd/internal/plugins/ping" "github.com/bethropolis/kcd/internal/plugins/presenter" + "github.com/bethropolis/kcd/internal/plugins/remotesystemvolume" "github.com/bethropolis/kcd/internal/plugins/runcommand" "github.com/bethropolis/kcd/internal/plugins/sftp" "github.com/bethropolis/kcd/internal/plugins/share" @@ -83,6 +84,9 @@ func setupPlugins(cfg *config.Config, bus *events.Bus, tlsCfg *tls.Config, logge if cfg.Plugins.SMS { plugins.Register(sms.NewSMSPlugin(cfg.SMS, bus, tlsCfg, logger)) } + if cfg.Plugins.RemoteSystemVolume { + plugins.Register(remotesystemvolume.NewRemoteSystemVolumePlugin(bus, logger)) + } return pairPlugin } diff --git a/internal/ipc/proto.go b/internal/ipc/proto.go index cc7f784..44e2c1f 100644 --- a/internal/ipc/proto.go +++ b/internal/ipc/proto.go @@ -39,6 +39,9 @@ const ( CmdMprisAction = "mpris_action" CmdMprisRemote = "mpris_remote" CmdSftpBrowse = "sftp_browse" + CmdRemoteVolumeList = "remote_volume_list" + CmdRemoteVolumeSet = "remote_volume_set" + CmdRemoteVolumeMute = "remote_volume_mute" ) // ConnectPayload carries the target IP for the CmdConnect command. diff --git a/internal/plugins/remotesystemvolume/remotesystemvolume.go b/internal/plugins/remotesystemvolume/remotesystemvolume.go new file mode 100644 index 0000000..235d3a5 --- /dev/null +++ b/internal/plugins/remotesystemvolume/remotesystemvolume.go @@ -0,0 +1,146 @@ +package remotesystemvolume + +import ( + "context" + "encoding/json" + "sync" + "time" + + "github.com/bethropolis/kcd/internal/device" + "github.com/bethropolis/kcd/internal/events" + "github.com/bethropolis/kcd/internal/protocol" + "go.uber.org/zap" +) + +type VolumeBody struct { + RequestSinks bool `json:"requestSinks,omitempty"` + Name string `json:"name,omitempty"` + Volume int `json:"volume,omitempty"` + Muted bool `json:"muted,omitempty"` + MaxVolume int `json:"maxVolume,omitempty"` + SinkList []SinkInfo `json:"sinkList,omitempty"` +} + +type SinkInfo struct { + Name string `json:"name"` + Description string `json:"description"` + Volume int `json:"volume"` + Muted bool `json:"muted"` + MaxVolume int `json:"maxVolume"` +} + +type RemoteSystemVolumePlugin struct { + logger *zap.Logger + bus *events.Bus + sinks sync.Map // device ID -> []SinkInfo +} + +func NewRemoteSystemVolumePlugin(bus *events.Bus, logger *zap.Logger) *RemoteSystemVolumePlugin { + return &RemoteSystemVolumePlugin{ + logger: logger.With(zap.String("plugin", "remotesystemvolume")), + bus: bus, + } +} + +func (p *RemoteSystemVolumePlugin) Name() string { return "RemoteSystemVolume" } +func (p *RemoteSystemVolumePlugin) Timeout() time.Duration { return 5 * time.Second } +func (p *RemoteSystemVolumePlugin) IncomingTypes() []string { + return []string{"kdeconnect.systemvolume"} +} +func (p *RemoteSystemVolumePlugin) OutgoingTypes() []string { + return []string{"kdeconnect.systemvolume.request"} +} + +func (p *RemoteSystemVolumePlugin) Handle(ctx context.Context, dev device.Sender, pkt *protocol.Packet) error { + var body VolumeBody + if err := json.Unmarshal(pkt.Body, &body); err != nil { + return err + } + + if body.SinkList != nil { + p.sinks.Store(dev.ID(), body.SinkList) + if p.bus != nil { + p.bus.Publish(events.TypeVolumeUpdate, dev.ID(), map[string]any{ + "sinks": body.SinkList, + }) + } + return nil + } + + if body.Name != "" { + if val, ok := p.sinks.Load(dev.ID()); ok { + sinks := val.([]SinkInfo) + for i, s := range sinks { + if s.Name == body.Name { + sinks[i].Volume = body.Volume + sinks[i].Muted = body.Muted + break + } + } + p.sinks.Store(dev.ID(), sinks) + } + if p.bus != nil { + p.bus.Publish(events.TypeVolumeUpdate, dev.ID(), map[string]any{ + "name": body.Name, + "volume": body.Volume, + "muted": body.Muted, + }) + } + } + + return nil +} + +func (p *RemoteSystemVolumePlugin) ListSinks(deviceID string) []SinkInfo { + if val, ok := p.sinks.Load(deviceID); ok { + return val.([]SinkInfo) + } + return nil +} + +func (p *RemoteSystemVolumePlugin) SetVolume(dev device.Sender, sinkName string, volume int) error { + pkt, err := protocol.NewPacket("kdeconnect.systemvolume.request", VolumeBody{ + Name: sinkName, + Volume: volume, + Muted: false, + }) + if err != nil { + return err + } + return dev.Send(pkt) +} + +func (p *RemoteSystemVolumePlugin) SetMuted(dev device.Sender, sinkName string, muted bool) error { + pkt, err := protocol.NewPacket("kdeconnect.systemvolume.request", VolumeBody{ + Name: sinkName, + Muted: muted, + }) + if err != nil { + return err + } + return dev.Send(pkt) +} + +func (p *RemoteSystemVolumePlugin) RequestSinkList(dev device.Sender) error { + pkt, err := protocol.NewPacket("kdeconnect.systemvolume.request", VolumeBody{ + RequestSinks: true, + }) + if err != nil { + return err + } + return dev.Send(pkt) +} + +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), + ) + } + }() +} +func (p *RemoteSystemVolumePlugin) OnDisconnect(dev device.Sender) { + p.sinks.Delete(dev.ID()) +} diff --git a/packaging/kcd.example.toml b/packaging/kcd.example.toml index cb55406..28a3232 100644 --- a/packaging/kcd.example.toml +++ b/packaging/kcd.example.toml @@ -145,6 +145,13 @@ sms = true # Screen size detection: requires xdpyinfo (X11) or wlr-randr (Wayland). presenter = true +# Control the remote device's audio volume from this machine. +# Commands: +# kcd volume list — List audio sinks on the phone +# kcd volume set <0-100> — Set volume for a sink +# kcd volume mute — Mute/unmute a sink +remotesystemvolume = true + # ─── RunCommand: exposed commands ───────────────────────────────────────────── # Shell commands that the RunCommand plugin makes available to your phone. # Trigger them from the KDE Connect Android app → your device → Run command. diff --git a/pkg/client/client.go b/pkg/client/client.go index 7f240b2..5444af8 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -397,7 +397,36 @@ func (c *Client) MprisRemote() (*ipc.MprisRemoteResponse, error) { return &resp, nil } -// WatchFile subscribes to daemon events and streams them to the given channel. +// RemoteVolumeList returns the last known sink list for a device. +func (c *Client) RemoteVolumeList(deviceID string) ([]byte, error) { + resp, err := c.Call(ipc.CmdRemoteVolumeList, ipc.DevicePayload{DeviceID: deviceID}) + if err != nil { + return nil, err + } + return resp.Data, nil +} + +// RemoteVolumeSet sets the volume for a specific sink on a remote device. +func (c *Client) RemoteVolumeSet(deviceID, sinkName string, volume int) error { + _, err := c.Call(ipc.CmdRemoteVolumeSet, struct { + DeviceID string `json:"deviceId"` + Name string `json:"name"` + Volume int `json:"volume"` + }{DeviceID: deviceID, Name: sinkName, Volume: volume}) + return err +} + +// RemoteVolumeMute sets the mute state for a specific sink on a remote device. +func (c *Client) RemoteVolumeMute(deviceID, sinkName string, muted bool) error { + _, err := c.Call(ipc.CmdRemoteVolumeMute, struct { + DeviceID string `json:"deviceId"` + Name string `json:"name"` + Muted bool `json:"muted"` + }{DeviceID: deviceID, Name: sinkName, Muted: muted}) + return err +} + +// Watch subscribes to daemon events and streams them to the given channel. func (c *Client) Watch(ctx context.Context, filter []string, ch chan<- events.Event) error { dialer := net.Dialer{} conn, err := dialer.DialContext(ctx, "unix", c.SocketPath) From fec0429ca4346962496a26dff9f4f8fe0d6775e9 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Wed, 15 Jul 2026 00:22:58 +0300 Subject: [PATCH 15/25] fix: clipboard backend detection when WAYLAND_DISPLAY is unset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under systemd user services, neither WAYLAND_DISPLAY nor DISPLAY is inherited from the login session. detectBackend() only checked exec.LookPath, found wl-paste, and committed to the Wayland backend — but wl-paste then defaulted to the wrong socket (wayland-0) and failed with exit code 1. Fix: check display environment variables first, then probe /run/user/1000 for any wayland-* socket before selecting the Wayland backend. X11 remains the fallback when no socket is found. --- internal/plugins/clipboard/clipboard.go | 57 +++++++++++++++++++++---- 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/internal/plugins/clipboard/clipboard.go b/internal/plugins/clipboard/clipboard.go index 041d2b4..4c99abe 100644 --- a/internal/plugins/clipboard/clipboard.go +++ b/internal/plugins/clipboard/clipboard.go @@ -49,16 +49,57 @@ func NewClipboardPlugin(tlsConfig *tls.Config, logger *zap.Logger) *ClipboardPlu } // detectBackend probes for a working clipboard tool (wl-paste → xclip) -// and caches the result so LookPath is called at most once. +// and caches the result so the probe runs at most once. +// +// Under systemd user services neither WAYLAND_DISPLAY nor DISPLAY +// is typically set. We probe by checking the display variable first, +// then falling back to scanning $XDG_RUNTIME_DIR for a Wayland socket. func (p *ClipboardPlugin) detectBackend() clipboardBackend { p.backendOnce.Do(func() { - if _, err := exec.LookPath("wl-paste"); err == nil { - p.backend = backendWayland - p.logger.Debug("clipboard: using Wayland backend (wl-paste/wl-copy)") - } else if _, err := exec.LookPath("xclip"); err == nil { - p.backend = backendX11 - p.logger.Debug("clipboard: using X11 backend (xclip)") - } else { + wlDisplay := os.Getenv("WAYLAND_DISPLAY") + xDisplay := os.Getenv("DISPLAY") + + switch { + case wlDisplay != "": + if _, err := exec.LookPath("wl-paste"); err == nil { + p.backend = backendWayland + p.logger.Debug("clipboard: backend=wayland (WAYLAND_DISPLAY set)") + } + case xDisplay != "": + if _, err := exec.LookPath("xclip"); err == nil { + p.backend = backendX11 + p.logger.Debug("clipboard: backend=x11 (DISPLAY set)") + } + default: + // Systemd user service — neither variable is set. + // Probe $XDG_RUNTIME_DIR for any Wayland socket. + rtDir := os.Getenv("XDG_RUNTIME_DIR") + hasWaylandSock := false + if rtDir != "" { + if entries, err := os.ReadDir(rtDir); err == nil { + for _, e := range entries { + if strings.HasPrefix(e.Name(), "wayland-") && !e.IsDir() { + hasWaylandSock = true + break + } + } + } + } + if hasWaylandSock { + if _, err := exec.LookPath("wl-paste"); err == nil { + p.backend = backendWayland + p.logger.Debug("clipboard: backend=wayland (socket probe)") + } + } + if p.backend == backendUnknown { + if _, err := exec.LookPath("xclip"); err == nil { + p.backend = backendX11 + p.logger.Debug("clipboard: backend=x11 (fallback)") + } + } + } + + if p.backend == backendUnknown { p.logger.Warn("clipboard: no clipboard tool found (install wl-clipboard or xclip)") } }) From 8366d5318223342d60db675b2b1d1d0dd29ec2d4 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Wed, 15 Jul 2026 00:56:41 +0300 Subject: [PATCH 16/25] feat: make clipboard push-on-connect opt-in (default off) Adds [clipboard] config section with push_on_connect option (default false). When true, the local clipboard is pushed to a device every time it connects/reconnects via OnConnect. When false (default), clipboard is only pushed explicitly via kcd clipboard. --- internal/config/config.go | 2 ++ internal/config/plugins.go | 10 ++++++++++ internal/daemon/plugins.go | 2 +- internal/plugins/clipboard/clipboard.go | 11 ++++++++--- packaging/kcd.example.toml | 6 ++++++ 5 files changed, 27 insertions(+), 4 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 027c833..dbf2da8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -36,6 +36,7 @@ type Config struct { Share ShareConfig `toml:"share"` SFTP SFTPConfig `toml:"sftp"` Ping PingConfig `toml:"ping"` + Clipboard ClipboardConfig `toml:"clipboard"` Pairing PairingConfig `toml:"pairing"` Mousepad MousepadConfig `toml:"mousepad"` SMS SMSConfig `toml:"sms"` @@ -74,6 +75,7 @@ func Defaults() *Config { c.Ping.Defaults() c.Pairing.Defaults() c.Mousepad.Defaults() + c.Clipboard.Defaults() c.SMS.Defaults() c.PruneStaleThreshold = "15m" diff --git a/internal/config/plugins.go b/internal/config/plugins.go index 2a09fb3..2ec05a5 100644 --- a/internal/config/plugins.go +++ b/internal/config/plugins.go @@ -120,6 +120,16 @@ func (c *NotificationPluginConfig) Defaults() { c.ExpireMS = -1 } +type ClipboardConfig struct { + // PushOnConnect sends the local clipboard to a device when it + // connects/reconnects. Off by default to avoid spurious pushes. + PushOnConnect bool `toml:"push_on_connect"` +} + +func (c *ClipboardConfig) Defaults() { + c.PushOnConnect = false +} + func (c *ShareConfig) Defaults() { c.PortMin = 1739 c.PortMax = 1764 diff --git a/internal/daemon/plugins.go b/internal/daemon/plugins.go index e5d6df6..1d66d40 100644 --- a/internal/daemon/plugins.go +++ b/internal/daemon/plugins.go @@ -40,7 +40,7 @@ func setupPlugins(cfg *config.Config, bus *events.Bus, tlsCfg *tls.Config, logge plugins.Register(notification.NewNotificationPlugin(cfg.Notification, bus, tlsCfg, logger)) } if cfg.Plugins.Clipboard { - plugins.Register(clipboard.NewClipboardPlugin(tlsCfg, logger)) + plugins.Register(clipboard.NewClipboardPlugin(tlsCfg, logger, cfg.Clipboard.PushOnConnect)) } if cfg.Plugins.Share { plugins.Register(share.NewSharePlugin(cfg.DownloadDir, cfg.Share, tlsCfg, bus, logger)) diff --git a/internal/plugins/clipboard/clipboard.go b/internal/plugins/clipboard/clipboard.go index 4c99abe..eed77b0 100644 --- a/internal/plugins/clipboard/clipboard.go +++ b/internal/plugins/clipboard/clipboard.go @@ -30,6 +30,7 @@ const ( // ClipboardPlugin handles clipboard sync both directions. type ClipboardPlugin struct { + pushOnConnect bool lastTimestamp int64 tlsConfig *tls.Config logger *zap.Logger @@ -41,10 +42,11 @@ type ClipboardPlugin struct { } // NewClipboardPlugin creates a clipboard plugin. -func NewClipboardPlugin(tlsConfig *tls.Config, logger *zap.Logger) *ClipboardPlugin { +func NewClipboardPlugin(tlsConfig *tls.Config, logger *zap.Logger, pushOnConnect bool) *ClipboardPlugin { return &ClipboardPlugin{ - tlsConfig: tlsConfig, - logger: logger.With(zap.String("plugin", "clipboard")), + tlsConfig: tlsConfig, + pushOnConnect: pushOnConnect, + logger: logger.With(zap.String("plugin", "clipboard")), } } @@ -355,6 +357,9 @@ func (p *ClipboardPlugin) readClipboard() string { } func (p *ClipboardPlugin) OnConnect(dev device.Sender) { + if !p.pushOnConnect { + return + } content := p.readClipboard() p.mu.Lock() diff --git a/packaging/kcd.example.toml b/packaging/kcd.example.toml index 28a3232..3d21c45 100644 --- a/packaging/kcd.example.toml +++ b/packaging/kcd.example.toml @@ -76,6 +76,12 @@ battery = true # X11: requires xclip. clipboard = true +[clipboard] +# Push the local clipboard to a device every time it connects +# or reconnects. Off by default to avoid spurious pushes on +# daemon restart or phone reconnection. +# push_on_connect = false + # Forward phone notifications to the desktop via notify-send. # Requires libnotify / a running notification daemon. notification = true From 099a120271fc3290ee1f99309ada3ef2b59d2ee4 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Fri, 7 Aug 2026 23:23:06 +0300 Subject: [PATCH 17/25] feat(mpris): keep now-playing fresh and cache album art --- AGENTS.md | 1 + docs/CLI.md | 9 +- docs/CLIENT_GUIDE.md | 20 +- docs/IPC_PROTOCOL.md | 46 +++- internal/plugins/mpris/artcache.go | 155 ++++++++++++++ internal/plugins/mpris/artcache_test.go | 113 ++++++++++ internal/plugins/mpris/mpris.go | 214 ++++++++++++++++++- internal/plugins/mpris/mpris_test.go | 268 ++++++++++++++++++++++++ 8 files changed, 816 insertions(+), 10 deletions(-) create mode 100644 internal/plugins/mpris/artcache.go create mode 100644 internal/plugins/mpris/artcache_test.go diff --git a/AGENTS.md b/AGENTS.md index 3fe4477..b9cb4a9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,6 +55,7 @@ These structural constraints must hold at all times: | Device state (persisted pairs) | `~/.local/state/kcd/devices.json` (`$XDG_STATE_HOME/kcd/devices.json`) | | TLS cert / key | `~/.config/kcd/cert.pem`, `~/.config/kcd/key.pem` | | IPC Unix socket | `/run/user//kcd/kcd.sock` (`$XDG_RUNTIME_DIR/kcd/kcd.sock`) | +| Album art cache | `~/.cache/kcd/art/` (`$XDG_CACHE_HOME/kcd/art`) — resolved `kdeconnect://` art URIs, keyed by `kdeArtHash` | | Downloaded files | `~/Downloads/kcd/` (overridable via `download_dir` in config) | | systemd user unit | `~/.config/systemd/user/kcd.service` | diff --git a/docs/CLI.md b/docs/CLI.md index 9dc45c8..b219266 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -424,7 +424,14 @@ kcd watch --events=mpris.update # bindsym XF86AudioNext exec kcd mpris next ``` -> **Note:** The `mpris.update` event fires whenever the phone sends a now-playing state change (track change, play/pause toggle). Subscribe with `kcd watch --events=mpris.update`. +> **Note:** The `mpris.update` event fires whenever the phone sends a now-playing state change (track change, play/pause toggle). Subscribe with `kcd watch --events=mpris.update`. The daemon also re-requests now-playing every 5 seconds from devices with an **actively-playing** player, so state stays fresh for pure-push clients without polling — but events are deduplicated, so `mpris.update` only fires on real changes. Stopped/paused players are not polled (a stopped-but-alive track stays listed so it can be resumed), and when the phone removes a player from its `playerList` (session destroyed) the cached state is dropped and an empty `mpris.update` is emitted so the widget falls back to "no media playing". +> +> **Note:** Phone album art URIs (`kdeconnect:/artUri?...`) are resolved by the +> daemon: it fetches the art bytes from the phone, caches them to +> `$XDG_CACHE_HOME/kcd/art/.`, and emits a loadable +> `file://` URL in `albumArtUrl` (both in `kcd mpris status --json` and in +> `mpris.update` events). A second `mpris.update` is published when the art +> arrives. --- diff --git a/docs/CLIENT_GUIDE.md b/docs/CLIENT_GUIDE.md index 347fd18..2a52fca 100644 --- a/docs/CLIENT_GUIDE.md +++ b/docs/CLIENT_GUIDE.md @@ -245,7 +245,6 @@ def on_mpris(device_id, payload): print(f'Now playing: {payload["title"]} by {payload["artist"]}') else: print("Paused/stopped") - w.start() # Keep main thread alive @@ -267,11 +266,28 @@ except KeyboardInterrupt: | `notification` | Push notification from device | | `share.progress` | File transfer progress update | | `share.complete` | File transfer finished | -| `mpris.update` | Now-playing state changed | +| `mpris.update` | Now-playing state changed (deduplicated — only on real changes) | | `sms.incoming` | SMS/MMS received | | `pair.requested` | Remote device wants to pair | | `ping.received` | Ping from device | +> **Freshness:** the daemon re-requests now-playing from devices with an +> actively-playing player every 5 seconds, so a pure-push client (a widget watching the +> event stream, with no polling) receives the current track within one poll +> interval of subscribing — including mid-track mount, thanks to the initial +> event dump. Events are deduplicated: `mpris.update` only fires when the +> state actually changed, so the stream stays quiet between track changes. +> Stopped/paused players are not polled, and when the phone removes a player +> from its `playerList` (session destroyed) the cached state is dropped with +> an empty `mpris.update` — showing "no media playing". + +> **Album art:** `mpris.update` payloads (and `kcd mpris status --json`) +> expose `albumArtUrl` as a loadable `file://` path once the daemon has +> fetched the art from the phone into `$XDG_CACHE_HOME/kcd/art/`. If the +> art is still being fetched or fails, the raw `kdeconnect:/artUri?...` +> URI is emitted instead — treat anything that isn't `http(s)://` or +> `file://` as "no art available" and show a placeholder. + See [`IPC_PROTOCOL.md §5`](IPC_PROTOCOL.md#5-event-types) for the full list. --- diff --git a/docs/IPC_PROTOCOL.md b/docs/IPC_PROTOCOL.md index 50f8fe0..e848035 100644 --- a/docs/IPC_PROTOCOL.md +++ b/docs/IPC_PROTOCOL.md @@ -666,6 +666,14 @@ are delivered. {"type":"mpris.update","deviceId":"...","timestamp":"...","payload":{...NowPlaying...}} ``` + The daemon keeps now-playing state fresh by re-requesting it every 5 + seconds from devices with an **actively-playing** player (see the + `mpris.update` section below), so this initial dump fires reliably for + mid-track state — a pure-push client can mount and see the current track + without polling. Stopped/paused players are deliberately not polled, so + they age past the 10-second gate and no ghost track is shown after a + reconnect. + 3. **Live stream:** All matching events are streamed as they occur, one per line, until the client disconnects or the daemon shuts down. @@ -1057,6 +1065,32 @@ this daemon to ring). Now-playing state from a device's media player. +> **Album art:** when the phone advertises a `kdeconnect:/artUri?...` URI, +> the daemon requests the art bytes over a side channel and caches them to +> `$XDG_CACHE_HOME/kcd/art/.`. Once fetched, `albumArtUrl` +> is emitted as a loadable `file://` path. A second `mpris.update` event is +> published when the art arrives. If the fetch is still in flight or fails, +> the original URI is left untouched — clients should fall back to a +> placeholder. + +> **Freshness:** the daemon re-requests now-playing from every connected +> device with an **actively-playing** player every 5 seconds +> (`kdeconnect.mpris.request` with `requestNowPlaying: true`). Responses are +> deduplicated — an event is only emitted when the state actually changes. +> This keeps `pos`/state current for pure-push clients (widgets, Waybar) +> that never poll the CLI. Devices that haven't reported a player yet, or +> whose player is stopped/paused, are not polled — a stopped-but-alive +> player keeps its last pushed track (so it can be resumed), but stops being +> refreshed and ages out of the 10-second initial-dump gate. + +> **Session teardown:** when the phone reports a `playerList` that no longer +> contains the device's currently-tracked player (the media session was +> destroyed, e.g. the app was swiped away), the daemon drops the cached +> state and publishes an empty `mpris.update` (`{...NowPlaying...}` with no +> title) so watchers fall back to "no media playing". An empty `playerList` +> (all sessions destroyed) has the same effect. A stopped-but-alive session +> is **not** cleared — only sessions removed from `playerList` are. + **Payload:** ```json @@ -1065,7 +1099,7 @@ Now-playing state from a device's media player. "title": "Song Title", "artist": "Artist Name", "album": "Album Name", - "albumArtUrl": "https://i.scdn.co/image/...", + "albumArtUrl": "file:///home/user/.cache/kcd/art/1141556203.jpg", "url": "spotify:track:...", "length": 240000, "pos": 45000, @@ -1083,6 +1117,10 @@ Now-playing state from a device's media player. } ``` +The extension is sniffed from the received bytes (JPEG/PNG/GIF/WebP), +defaulting to `.jpg`. The cache directory holds at most 500 files and is +cleared when it overflows. + --- ## 6. Outbound Packet Reference @@ -1102,7 +1140,7 @@ who may want to implement a full network-level implementation. | `kdeconnect.clipboard.connect` | Clipboard | Push clipboard with timestamp on connect | | `kdeconnect.mousepad.keyboardstate` | Mousepad | Advertise keyboard capability on connect | | `kdeconnect.mpris` | MPRIS | Player list, NowPlaying state, seek positions, album art (broadcast + request-reply) | -| `kdeconnect.mpris.request` | MPRIS | Request player list, now-playing, volume; send control actions | +| `kdeconnect.mpris.request` | MPRIS | Request player list, now-playing, volume, album art; send control actions | | `kdeconnect.notification.reply` | Notification | Reply to a notification with inline reply support | | `kdeconnect.notification` | RunCommand | Command output notification pushed to phone | | `kdeconnect.runcommand` | RunCommand | Send command list to phone | @@ -1148,8 +1186,8 @@ plugin processes it and a link to the body struct definition. | `kdeconnect.clipboard.file` | Clipboard | `ClipboardFileBody{Filename string}` | | `kdeconnect.lock` | LockDevice | `LockBody{RequestLocked, SetLocked, IsLocked}` | | `kdeconnect.lock.request` | LockDevice | `LockBody{}` (triggers lock/unlock) | -| `kdeconnect.mpris` | MPRIS | `MPRISRequest{RequestPlayerList, RequestNowPlaying, RequestVolume, Player, Action, ...}` | -| `kdeconnect.mpris.request` | MPRIS | `MPRISRequest{}` (same struct, different semantics) | +| `kdeconnect.mpris` | MPRIS | `MPRISRequest{RequestPlayerList, RequestNowPlaying, RequestVolume, Player, Action, AlbumArtUrl, TransferringAlbumArt, ...}` — inbound packets with `transferringAlbumArt: true` + `payloadTransferInfo` carry album art bytes (side channel) that the daemon caches to `$XDG_CACHE_HOME/kcd/art/` | +| `kdeconnect.mpris.request` | MPRIS | `MPRISRequest{}` (same struct, different semantics) — an outbound `kdeconnect.mpris.request` with `player` + `albumArtUrl` asks the phone to stream art back | | `kdeconnect.runcommand.request` | RunCommand | `RequestBody{RequestCommandList bool, Key string}` | | `kdeconnect.presenter` | Presenter | `PresenterBody{Dx, Dy *float64, Stop *bool}` | | `kdeconnect.systemvolume` | RemoteSystemVolume | `VolumeBody{SinkList, Name, Volume, Muted}` | diff --git a/internal/plugins/mpris/artcache.go b/internal/plugins/mpris/artcache.go new file mode 100644 index 0000000..e0d157d --- /dev/null +++ b/internal/plugins/mpris/artcache.go @@ -0,0 +1,155 @@ +package mpris + +import ( + "hash/fnv" + "io" + "net/url" + "os" + "path/filepath" + "strconv" + "strings" + "sync" + + "go.uber.org/zap" +) + +// maxAlbumArtBytes caps inbound album art payloads (matches the 5 MiB +// limit in the KDE Connect reference AlbumArtCache). +const maxAlbumArtBytes = 5 * 1024 * 1024 + +// maxArtCacheFiles bounds the on-disk cache. Exceeding it clears the +// directory, mirroring the reference implementation's startup purge. +const maxArtCacheFiles = 500 + +// ArtCache resolves kdeconnect:/artUri album art URIs to local file:// +// URLs. Bytes fetched from the phone are streamed to +// $XDG_CACHE_HOME/kcd/art/.. +type ArtCache struct { + dir string + mu sync.RWMutex + resolved map[string]string // raw albumArtUrl -> file:// path +} + +// NewArtCache creates the cache directory and returns an empty cache. +func NewArtCache(logger *zap.Logger) *ArtCache { + base, err := os.UserCacheDir() + if err != nil || base == "" { + base = filepath.Join(os.TempDir(), "kcd-cache") + } + dir := filepath.Join(base, "kcd", "art") + 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)) + } + purgeArtCacheDir(dir, logger) + return &ArtCache{ + dir: dir, + resolved: make(map[string]string), + } +} + +// purgeArtCacheDir clears the cache when it grows past maxArtCacheFiles. +func purgeArtCacheDir(dir string, logger *zap.Logger) { + entries, err := os.ReadDir(dir) + if err != nil { + return + } + if len(entries) <= maxArtCacheFiles { + return + } + logger.Debug("mpris: clearing oversized album art cache", + zap.Int("files", len(entries))) + for _, e := range entries { + if !e.IsDir() { + _ = os.Remove(filepath.Join(dir, e.Name())) + } + } +} + +// Dir returns the on-disk cache directory. +func (c *ArtCache) Dir() string { + return c.dir +} + +// Key derives the cache filename stem for an album art URI: the +// kdeArtHash query parameter when present, otherwise an FNV-1a hash of +// the full URI (matching the reference qHash-based naming). +func (c *ArtCache) Key(rawURL string) string { + if u, err := url.Parse(rawURL); err == nil { + if h := u.Query().Get("kdeArtHash"); h != "" { + return h + } + } + h := fnv.New32a() + _, _ = h.Write([]byte(rawURL)) + return strconv.FormatUint(uint64(h.Sum32()), 10) +} + +// Resolve returns a file:// URL for a cached album art URI, or "" when +// the art has not been downloaded (or is still in flight). +func (c *ArtCache) Resolve(rawURL string) string { + if !strings.HasPrefix(rawURL, "kdeconnect:/") { + return "" + } + c.mu.RLock() + path, ok := c.resolved[rawURL] + c.mu.RUnlock() + if ok { + return path + } + + // Disk fallback: a previously cached file survives a daemon restart. + matches, err := filepath.Glob(filepath.Join(c.dir, c.Key(rawURL)+".*")) + if err != nil || len(matches) == 0 { + return "" + } + path = "file://" + matches[0] + c.mu.Lock() + c.resolved[rawURL] = path + c.mu.Unlock() + return path +} + +// Commit moves a streamed temp file into the cache, sniffing its type +// from the first bytes, and returns the file:// URL for rawURL. +func (c *ArtCache) Commit(rawURL, tmpPath string) (string, error) { + f, err := os.Open(tmpPath) + if err != nil { + return "", err + } + buf := make([]byte, 16) + n, _ := io.ReadFull(f, buf) + f.Close() + + ext := sniffImageExt(buf[:n]) + dst := filepath.Join(c.dir, c.Key(rawURL)+ext) + if err := os.Rename(tmpPath, dst); err != nil { + _ = os.Remove(dst) + if err := os.Rename(tmpPath, dst); err != nil { + return "", err + } + } + + fileURL := "file://" + dst + c.mu.Lock() + c.resolved[rawURL] = fileURL + c.mu.Unlock() + return fileURL, nil +} + +// sniffImageExt guesses the file extension from image magic bytes. +// Falls back to .jpg, matching the reference implementation. +func sniffImageExt(data []byte) string { + switch { + case len(data) >= 3 && data[0] == 0xFF && data[1] == 0xD8 && data[2] == 0xFF: + return ".jpg" + case len(data) >= 8 && data[0] == 0x89 && data[1] == 'P' && data[2] == 'N' && data[3] == 'G': + return ".png" + case len(data) >= 6 && string(data[:4]) == "GIF8": + return ".gif" + case len(data) >= 12 && string(data[8:12]) == "WEBP": + return ".webp" + default: + return ".jpg" + } +} diff --git a/internal/plugins/mpris/artcache_test.go b/internal/plugins/mpris/artcache_test.go new file mode 100644 index 0000000..2434b4d --- /dev/null +++ b/internal/plugins/mpris/artcache_test.go @@ -0,0 +1,113 @@ +package mpris + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSniffImageExt(t *testing.T) { + cases := []struct { + name string + data []byte + want string + }{ + {"jpeg", []byte{0xFF, 0xD8, 0xFF, 0xE0, 0x00}, ".jpg"}, + {"png", []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, ".png"}, + {"gif", []byte("GIF89a..."), ".gif"}, + {"webp", []byte("RIFF\x00\x00\x00\x00WEBPVP8 "), ".webp"}, + {"unknown-falls-back-jpg", []byte("not an image"), ".jpg"}, + {"empty", nil, ".jpg"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := sniffImageExt(tc.data); got != tc.want { + t.Fatalf("sniffImageExt(%v) = %q, want %q", tc.data, got, tc.want) + } + }) + } +} + +func TestArtCacheKey(t *testing.T) { + c := &ArtCache{} + // kdeArtHash query param wins. + got := c.Key("kdeconnect:/artUri?title=Test&kdeArtHash=1141556203") + if got != "1141556203" { + t.Fatalf("Key with kdeArtHash = %q, want 1141556203", got) + } + // No kdeArtHash -> stable FNV fallback, same for identical URLs. + a := c.Key("kdeconnect:/artUri?title=OnlyTitle") + b := c.Key("kdeconnect:/artUri?title=OnlyTitle") + if a == "" || a != b { + t.Fatalf("expected stable non-empty key, got %q / %q", a, b) + } + // Different URLs -> different keys (extremely unlikely to collide). + c2 := c.Key("kdeconnect:/artUri?title=Other") + if a == c2 { + t.Fatalf("expected different keys, both %q", a) + } +} + +func TestArtCacheResolveAndCommit(t *testing.T) { + dir := t.TempDir() + c := &ArtCache{dir: dir, resolved: make(map[string]string)} + + rawURL := "kdeconnect:/artUri?title=Test&kdeArtHash=42" + if got := c.Resolve(rawURL); got != "" { + t.Fatalf("expected miss before commit, got %q", got) + } + + // Stream a tiny JPEG into a temp file, then commit. + tmp := filepath.Join(dir, "tmp-art") + png := []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A, 0x00} + if err := os.WriteFile(tmp, png, 0600); err != nil { + t.Fatal(err) + } + + fileURL, err := c.Commit(rawURL, tmp) + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(fileURL, "file://") { + t.Fatalf("expected file:// prefix, got %q", fileURL) + } + path := strings.TrimPrefix(fileURL, "file://") + if !strings.HasSuffix(path, "42.png") { + t.Fatalf("expected extension sniffed to .png with key 42, got %q", path) + } + if _, err := os.Stat(path); err != nil { + t.Fatalf("cached file missing: %v", err) + } + + if got := c.Resolve(rawURL); got != fileURL { + t.Fatalf("expected resolve to return %q, got %q", fileURL, got) + } + + // Disk fallback: a fresh cache (restart) finds the file again. + c2 := &ArtCache{dir: dir, resolved: make(map[string]string)} + if got := c2.Resolve(rawURL); got != fileURL { + t.Fatalf("expected disk fallback resolve %q, got %q", fileURL, got) + } +} + +func TestArtCacheCommitRenamesTemp(t *testing.T) { + dir := t.TempDir() + c := &ArtCache{dir: dir, resolved: make(map[string]string)} + + tmp := filepath.Join(dir, "tmp-art") + jpeg := bytes.Repeat([]byte{0xFF, 0xD8, 0xFF}, 32) + if err := os.WriteFile(tmp, jpeg, 0600); err != nil { + t.Fatal(err) + } + if _, err := c.Commit("kdeconnect:/artUri?kdeArtHash=7", tmp); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(tmp); !os.IsNotExist(err) { + t.Fatalf("expected temp file removed after commit, err=%v", err) + } + if _, err := os.Stat(filepath.Join(dir, "7.jpg")); err != nil { + t.Fatalf("expected cached 7.jpg: %v", err) + } +} diff --git a/internal/plugins/mpris/mpris.go b/internal/plugins/mpris/mpris.go index 52d26f7..327d93a 100644 --- a/internal/plugins/mpris/mpris.go +++ b/internal/plugins/mpris/mpris.go @@ -48,6 +48,8 @@ type MPRISPlugin struct { pauseMusic bool callPausedPlayers []string // names of local players paused during a call + + artCache *ArtCache } type trackIdentity struct { @@ -83,6 +85,7 @@ func NewMPRISPlugin(tlsConfig *tls.Config, bus *events.Bus, pauseMusic bool, log remoteStateTimes: make(map[string]time.Time), positionTrackers: make(map[string]*remotePositionTracker), callPausedPlayers: make([]string, 0), + artCache: NewArtCache(logger), } // Start the watcher immediately (like C++ does in constructor). @@ -91,6 +94,7 @@ func NewMPRISPlugin(tlsConfig *tls.Config, bus *events.Bus, pauseMusic bool, log p.watchCancel = cancel p.watching = true p.startWatcher(watchCtx) + p.startRemoteStatePoller(watchCtx) // Subscribe to telephony events for pause-music-on-call. if p.pauseMusic && p.dbus != nil { @@ -129,6 +133,9 @@ type MPRISRequest struct { // Album art AlbumArtUrl string `json:"albumArtUrl,omitempty"` + // Set when this packet carries album art bytes over a side channel. + TransferringAlbumArt bool `json:"transferringAlbumArt,omitempty"` + // Player list from remote device PlayerList []string `json:"playerList,omitempty"` @@ -206,13 +213,48 @@ func (p *MPRISPlugin) Handle(ctx context.Context, dev device.Sender, pkt *protoc return nil } + // Inbound album art payload — the phone responds to requestAlbumArt + // with a side-channel transfer carrying the art bytes. + if body.TransferringAlbumArt && pkt.PayloadSize > 0 && pkt.PayloadTransferInfo != nil { + go p.receiveAlbumArt(ctx, dev, body.Player, body.AlbumArtUrl, + pkt.PayloadSize, pkt.PayloadTransferInfo.Port) + return nil + } + if body.RequestPlayerList { return p.sendPlayerList(dev) } - // Incoming playerList from remote device — request status for each player - if len(body.PlayerList) > 0 { + // Incoming playerList from remote device — prune players that no longer + // 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)) + pruned := false + p.mu.Lock() + if prev := p.remoteStates[dev.ID()]; prev != nil { + inList := false + for _, name := range body.PlayerList { + if name == prev.Player { + inList = true + break + } + } + if !inList { + delete(p.remoteStates, dev.ID()) + delete(p.remoteStateTimes, dev.ID()) + delete(p.positionTrackers, dev.ID()) + pruned = true + } + } + p.mu.Unlock() + + if pruned && p.bus != nil { + // The tracked player's session is gone — emit an empty update so + // watchers fall back to "no media playing" for this device. + p.bus.Publish(events.TypeMprisUpdate, dev.ID(), &NowPlaying{}) + } + for _, player := range body.PlayerList { player := player go p.requestPlayerStatus(dev, player) @@ -274,8 +316,21 @@ func (p *MPRISPlugin) Handle(ctx context.Context, dev device.Sender, pkt *protoc p.remoteStates[dev.ID()] = state p.remoteStateTimes[dev.ID()] = time.Now() p.mu.Unlock() + + // Request art bytes from the phone when the advertsed album art is + // a kdeconnect:// URI we have not cached yet. Resolve any already + // cached art before publishing so watch clients get a loadable URL. + if p.artCache != nil && p.artCache.Resolve(state.AlbumArtUrl) == "" { + go p.requestAlbumArt(dev, state.Player, state.AlbumArtUrl) + } if shouldPublish && p.bus != nil { - p.bus.Publish(events.TypeMprisUpdate, dev.ID(), state) + pub := state.DeepCopy() + if p.artCache != nil { + if resolved := p.artCache.Resolve(pub.AlbumArtUrl); resolved != "" { + pub.AlbumArtUrl = resolved + } + } + p.bus.Publish(events.TypeMprisUpdate, dev.ID(), pub) } return nil } @@ -474,6 +529,87 @@ func (p *MPRISPlugin) sendAlbumArt(ctx context.Context, dev device.Sender, playe } } +// requestAlbumArt asks the remote device to stream the album art bytes +// referenced by a kdeconnect:/artUri URI over a side channel. +func (p *MPRISPlugin) requestAlbumArt(dev device.Sender, player, artUrl string) { + if player == "" || artUrl == "" { + return + } + p.mu.Lock() + reqKey := "req|" + dev.ID() + "|" + artUrl + if lastReq, exists := p.artRequests[reqKey]; exists && time.Since(lastReq) < 10*time.Second { + p.mu.Unlock() + return + } + p.artRequests[reqKey] = time.Now() + p.mu.Unlock() + + body := MPRISRequest{ + Player: player, + AlbumArtUrl: artUrl, + } + pkt, err := protocol.NewPacket("kdeconnect.mpris.request", body) + if err != nil { + return + } + if err := dev.Send(pkt); err != nil { + p.logger.Debug("mpris: album art request failed", zap.Error(err)) + } +} + +// receiveAlbumArt streams an inbound album art payload into the cache +// and re-publishes the device state with a resolved file:// URL so watch +// clients and kcd mpris status surface the loadable location. +func (p *MPRISPlugin) receiveAlbumArt(_ context.Context, dev device.Sender, player, artUrl string, size int64, port int) { + remoteIP := dev.RemoteIP() + if remoteIP == nil { + return + } + if p.artCache == nil || size <= 0 || size > maxAlbumArtBytes { + return + } + + p.logger.Debug("mpris: receiving album art from remote", + zap.String("player", player), + zap.String("device_id", dev.ID()), + zap.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)) + return + } + tmpPath := tmp.Name() + tmp.Close() + defer os.Remove(tmpPath) + + // The Handle ctx is canceled as soon as Handle returns; use an + // independent context so the side-channel dial isn't aborted. + dlCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + if err := share.ReceiveSideChannel(dlCtx, remoteIP, port, size, tmpPath, p.tlsConfig, nil, p.logger); err != nil { + p.logger.Warn("mpris: album art transfer failed", zap.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)) + return + } + + p.mu.Lock() + state := p.remoteStates[dev.ID()] + if state != nil { + state = state.DeepCopy() + state.AlbumArtUrl = fileURL + } + p.mu.Unlock() + if state != nil && p.bus != nil { + p.bus.Publish(events.TypeMprisUpdate, dev.ID(), state) + } +} + func (p *MPRISPlugin) OnConnect(dev device.Sender) { p.logger.Info("mpris: device connected, requesting player list", zap.String("device_id", dev.ID())) go p.requestPlayerListPeriodic(dev) @@ -498,6 +634,64 @@ func (p *MPRISPlugin) requestPlayerListPeriodic(dev device.Sender) { p.requestPlayerList(dev) } +// remoteStatePollInterval is how often the daemon re-requests now-playing +// from devices with an active remote player. Clients are then pure-push: +// fresh state arrives within one interval of connect, and position stays +// current without any client-side polling. +const remoteStatePollInterval = 5 * time.Second + +// startRemoteStatePoller periodically re-requests now-playing from every +// connected device that has a known active player. The responses flow back +// through Handle, where shouldPublishRemoteState dedupes them, so an +// mpris.update is only republished when the state actually changes — not +// on every poll. This closes the "watch client misses mid-track state" +// gap from the initial dump's 10s freshness gate. +func (p *MPRISPlugin) startRemoteStatePoller(ctx context.Context) { + go func() { + ticker := time.NewTicker(remoteStatePollInterval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + p.pollRemoteStates() + } + } + }() +} + +// pollRemoteStates requests a now-playing refresh from devices that have a +// cached, actively-playing player. Devices without a cached state (never +// reported a player) or whose player is stopped/paused are skipped — stopped +// players are intentionally left to go stale instead of keeping a ghost track +// perpetually fresh. +func (p *MPRISPlugin) pollRemoteStates() { + p.mu.RLock() + type target struct { + dev device.Sender + player string + } + var targets []target + for id, dev := range p.devices { + if !dev.IsConnected() { + continue + } + state := p.remoteStates[id] + if state == nil || state.Player == "" || !state.IsPlaying { + continue + } + targets = append(targets, target{dev: dev, player: state.Player}) + } + p.mu.RUnlock() + + 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)) + } + } +} + func (p *MPRISPlugin) OnDisconnect(dev device.Sender) { p.mu.Lock() defer p.mu.Unlock() @@ -657,6 +851,7 @@ func (p *MPRISPlugin) RemoteState(deviceID string) *NowPlaying { return nil } copy := state.DeepCopy() + copy.AlbumArtUrl = p.resolveArtURL(copy.AlbumArtUrl) if tracker, ok := p.positionTrackers[deviceID]; ok && tracker.playing { elapsed := time.Since(tracker.lastPositionAt).Milliseconds() copy.Pos = tracker.lastPosition + elapsed @@ -687,6 +882,7 @@ func (p *MPRISPlugin) RemoteStates() map[string]*NowPlaying { continue } copy := state.DeepCopy() + copy.AlbumArtUrl = p.resolveArtURL(copy.AlbumArtUrl) if tracker, ok := p.positionTrackers[id]; ok && tracker.playing { elapsed := time.Since(tracker.lastPositionAt).Milliseconds() copy.Pos = tracker.lastPosition + elapsed @@ -696,6 +892,18 @@ func (p *MPRISPlugin) RemoteStates() map[string]*NowPlaying { return result } +// resolveArtURL maps a cached kdeconnect:// album art URI to a loadable +// file:// URL. Non-kdeconnect URIs and not-yet-cached art pass through. +func (p *MPRISPlugin) resolveArtURL(raw string) string { + if raw == "" || p.artCache == nil { + return raw + } + if resolved := p.artCache.Resolve(raw); resolved != "" { + return resolved + } + return raw +} + // watchTelephony subscribes to telephony events and pauses/resumes // local MPRIS players when calls start/end. func (p *MPRISPlugin) watchTelephony(ctx context.Context) { diff --git a/internal/plugins/mpris/mpris_test.go b/internal/plugins/mpris/mpris_test.go index d1e2b6e..0c00d1c 100644 --- a/internal/plugins/mpris/mpris_test.go +++ b/internal/plugins/mpris/mpris_test.go @@ -3,7 +3,9 @@ package mpris import ( "context" "crypto/x509" + "encoding/json" "net" + "sync" "testing" "time" @@ -106,3 +108,269 @@ func expectNoEvent(t *testing.T, sub *events.Subscriber) { case <-time.After(50 * time.Millisecond): } } + +type recordingSender struct { + id string + mu sync.Mutex + packets []*protocol.Packet +} + +func (s *recordingSender) ID() string { return s.id } +func (s *recordingSender) Name() string { return "" } +func (s *recordingSender) SetName(string) {} +func (s *recordingSender) State() device.PairingState { return device.StateUnpaired } +func (s *recordingSender) SetState(device.PairingState) {} +func (s *recordingSender) Send(p *protocol.Packet) error { + s.mu.Lock() + defer s.mu.Unlock() + s.packets = append(s.packets, p) + return nil +} +func (s *recordingSender) IsConnected() bool { return true } +func (s *recordingSender) RemoteIP() net.IP { return nil } +func (s *recordingSender) PeerCert() *x509.Certificate { return nil } +func (s *recordingSender) HasCapability(string) bool { return false } +func (s *recordingSender) UpdateBattery(charge int, charging bool) {} +func (s *recordingSender) GetBattery() (int, bool) { return 0, false } + +func (s *recordingSender) sent() []*protocol.Packet { + s.mu.Lock() + defer s.mu.Unlock() + out := make([]*protocol.Packet, len(s.packets)) + copy(out, s.packets) + return out +} + +func TestHandleRequestsAlbumArtForKdeconnectURI(t *testing.T) { + plugin := NewMPRISPlugin(nil, events.NewBus(zap.NewNop()), false, zap.NewNop()) + if plugin.watchCancel != nil { + defer plugin.watchCancel() + } + dev := &recordingSender{id: "device-art"} + + state := MPRISRequest{ + Player: "Spotify", + Title: "Some Track", + Artist: "Some Artist", + IsPlaying: true, + AlbumArtUrl: "kdeconnect:/artUri?title=Some+Track&kdeArtHash=12345", + } + if err := plugin.Handle(context.Background(), dev, newMPRISPacket(t, state)); err != nil { + t.Fatal(err) + } + + // requestAlbumArt runs async — wait for the packet to be sent. + deadline := time.Now().Add(time.Second) + for time.Now().Before(deadline) && len(dev.sent()) == 0 { + time.Sleep(5 * time.Millisecond) + } + plugin.mu.Lock() + _, requested := plugin.artRequests["req|device-art|"+state.AlbumArtUrl] + plugin.mu.Unlock() + if !requested { + t.Fatal("expected an album art request to be recorded") + } + + if len(dev.sent()) == 0 { + t.Fatal("expected at least one packet sent to the device") + } + pkt := dev.sent()[0] + if pkt.Type != "kdeconnect.mpris.request" { + t.Fatalf("expected kdeconnect.mpris.request, got %s", pkt.Type) + } + var body MPRISRequest + if err := json.Unmarshal(pkt.Body, &body); err != nil { + t.Fatal(err) + } + if body.Player != "Spotify" || body.AlbumArtUrl != state.AlbumArtUrl { + t.Fatalf("unexpected request body: %+v", body) + } +} + +func TestHandleIgnoresEmptyAlbumArtPayload(t *testing.T) { + plugin := NewMPRISPlugin(nil, events.NewBus(zap.NewNop()), false, zap.NewNop()) + if plugin.watchCancel != nil { + defer plugin.watchCancel() + } + dev := &recordingSender{id: "device-art-empty"} + + // A transferringAlbumArt packet with no payload must not crash and + // must not attempt a download (RemoteIP() is nil in the test sender). + pkt := newMPRISPacket(t, MPRISRequest{ + Player: "Spotify", + TransferringAlbumArt: true, + AlbumArtUrl: "kdeconnect:/artUri?kdeArtHash=1", + }) + if err := plugin.Handle(context.Background(), dev, pkt); err != nil { + t.Fatal(err) + } + time.Sleep(50 * time.Millisecond) // let any goroutine finish +} + +func TestPollRemoteStatesOnlyTargetsKnownPlayers(t *testing.T) { + plugin := NewMPRISPlugin(nil, events.NewBus(zap.NewNop()), false, zap.NewNop()) + if plugin.watchCancel != nil { + defer plugin.watchCancel() + } + + withPlayer := &recordingSender{id: "device-with-player"} + stoppedPlayer := &recordingSender{id: "device-stopped-player"} + noPlayer := &recordingSender{id: "device-no-player"} + + // Seed cached remote state for the device with a known active player. + state := MPRISRequest{Player: "Spotify", Title: "T", Artist: "A", IsPlaying: true} + if err := plugin.Handle(context.Background(), withPlayer, newMPRISPacket(t, state)); err != nil { + t.Fatal(err) + } + + // Seed cached state for a device whose player is stopped/paused — the + // poller must not keep refreshing it. + stopped := MPRISRequest{Player: "Namida", Title: "S", Artist: "B", IsPlaying: false} + if err := plugin.Handle(context.Background(), stoppedPlayer, newMPRISPacket(t, stopped)); err != nil { + t.Fatal(err) + } + + plugin.mu.Lock() + plugin.devices[withPlayer.id] = withPlayer + plugin.devices[stoppedPlayer.id] = stoppedPlayer + plugin.devices[noPlayer.id] = noPlayer + plugin.mu.Unlock() + + // Clear the recorded art-request packets from Handle so we can assert + // exactly what the poller sends. + withPlayer.sent() + stoppedPlayer.sent() + + plugin.pollRemoteStates() + + sent := withPlayer.sent() + if len(sent) == 0 { + t.Fatal("expected poller to request state from device with known active player") + } + var body MPRISRequest + if err := json.Unmarshal(sent[0].Body, &body); err != nil { + t.Fatal(err) + } + if !body.RequestNowPlaying { + t.Fatalf("expected requestNowPlaying=true in poll request, got %+v", body) + } + if body.Player != "Spotify" { + t.Fatalf("expected poll request for player Spotify, got %q", body.Player) + } + if got := stoppedPlayer.sent(); len(got) != 0 { + t.Fatalf("expected no poll request to device with stopped player, got %d packets", len(got)) + } + if got := noPlayer.sent(); len(got) != 0 { + t.Fatalf("expected no poll request to device without cached player, got %d packets", len(got)) + } +} + +func newMPRISRawPacket(t *testing.T, body map[string]interface{}) *protocol.Packet { + t.Helper() + pkt, err := protocol.NewPacket("kdeconnect.mpris", body) + if err != nil { + t.Fatal(err) + } + return pkt +} + +func TestHandlePrunesRemovedPlayer(t *testing.T) { + bus := events.NewBus(zap.NewNop()) + sub := bus.Subscribe(4, events.TypeMprisUpdate) + defer sub.Close() + + plugin := NewMPRISPlugin(nil, bus, false, zap.NewNop()) + if plugin.watchCancel != nil { + defer plugin.watchCancel() + } + dev := &recordingSender{id: "device-prune"} + + state := MPRISRequest{Player: "Namida", Title: "Otonoke", Artist: "Creepy Nuts", IsPlaying: true} + if err := plugin.Handle(context.Background(), dev, newMPRISPacket(t, state)); err != nil { + t.Fatal(err) + } + expectEvent(t, sub) // initial state event + + // The phone destroys the session: playerList no longer contains Namida. + list := newMPRISPacket(t, MPRISRequest{PlayerList: []string{"Spotify"}}) + if err := plugin.Handle(context.Background(), dev, list); err != nil { + t.Fatal(err) + } + + if got := plugin.RemoteState(dev.ID()); got != nil { + t.Fatalf("expected remote state to be pruned, got %+v", got) + } + plugin.mu.Lock() + _, hasTracker := plugin.positionTrackers[dev.ID()] + _, hasTime := plugin.remoteStateTimes[dev.ID()] + plugin.mu.Unlock() + if hasTracker { + t.Fatal("expected position tracker to be pruned") + } + if hasTime { + t.Fatal("expected remote state time to be pruned") + } + + // Empty update published so watchers fall back to "no media playing". + expectEvent(t, sub) +} + +func TestHandlePrunesPlayerOnEmptyList(t *testing.T) { + bus := events.NewBus(zap.NewNop()) + sub := bus.Subscribe(4, events.TypeMprisUpdate) + defer sub.Close() + + plugin := NewMPRISPlugin(nil, bus, false, zap.NewNop()) + if plugin.watchCancel != nil { + defer plugin.watchCancel() + } + dev := &recordingSender{id: "device-empty-list"} + + state := MPRISRequest{Player: "Namida", Title: "Otonoke", IsPlaying: true} + if err := plugin.Handle(context.Background(), dev, newMPRISPacket(t, state)); err != nil { + t.Fatal(err) + } + expectEvent(t, sub) // initial state event + + // All sessions destroyed — Android sends an explicit empty playerList. + // (MPRISRequest marshals []string{} to "playerList":[]; the omitempty tag + // would drop the key, so inject a raw body to exercise the presence check.) + empty := newMPRISRawPacket(t, map[string]interface{}{"playerList": []string{}}) + if err := plugin.Handle(context.Background(), dev, empty); err != nil { + t.Fatal(err) + } + + if got := plugin.RemoteState(dev.ID()); got != nil { + t.Fatalf("expected remote state to be pruned, got %+v", got) + } + expectEvent(t, sub) // empty update published +} + +func TestHandleKeepsListedPlayer(t *testing.T) { + bus := events.NewBus(zap.NewNop()) + sub := bus.Subscribe(4, events.TypeMprisUpdate) + defer sub.Close() + + plugin := NewMPRISPlugin(nil, bus, false, zap.NewNop()) + if plugin.watchCancel != nil { + defer plugin.watchCancel() + } + dev := &recordingSender{id: "device-keep"} + + state := MPRISRequest{Player: "Spotify", Title: "T", IsPlaying: true} + if err := plugin.Handle(context.Background(), dev, newMPRISPacket(t, state)); err != nil { + t.Fatal(err) + } + expectEvent(t, sub) // initial state event + + // playerList still contains the tracked player — state must survive. + list := newMPRISPacket(t, MPRISRequest{PlayerList: []string{"Spotify", "Namida"}}) + if err := plugin.Handle(context.Background(), dev, list); err != nil { + t.Fatal(err) + } + + if got := plugin.RemoteState(dev.ID()); got == nil || got.Player != "Spotify" { + t.Fatalf("expected state to survive for listed player, got %+v", got) + } + expectNoEvent(t, sub) // no spurious empty update +} From 14b5165236603c90c49ddbeec393d82743e5cacc Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Fri, 7 Aug 2026 23:23:18 +0300 Subject: [PATCH 18/25] feat(notification): collapse re-posted popups and default to icon-less --- docs/IPC_PROTOCOL.md | 10 ++ internal/config/plugins.go | 9 + internal/plugins/notification/notification.go | 77 +++++--- .../plugins/notification/notification_test.go | 168 +++++++++++++++++- packaging/kcd.example.toml | 2 + 5 files changed, 244 insertions(+), 22 deletions(-) diff --git a/docs/IPC_PROTOCOL.md b/docs/IPC_PROTOCOL.md index e848035..ab2c596 100644 --- a/docs/IPC_PROTOCOL.md +++ b/docs/IPC_PROTOCOL.md @@ -830,6 +830,16 @@ A notification was received from a device. | `requestReplyId` | string | Present if the notification supports inline replies | | `id` | string | Notification identifier | +> **Desktop popups:** the daemon shows each phone notification via `notify-send`. +> Popups render **without an icon by default** (`show_icons = false`); set +> `show_icons = true` to display the phone's app icon (downloaded via +> `fetch_icons` and reused across re-posts). Because Android re-posts a +> notification on every update with a stable `id` (e.g. a scrobbler's +> now-playing notification), the daemon replaces the existing desktop popup +> in place (`--replace-id`) so repeated updates collapse to one popup instead +> of flooding the screen — mirroring the reference desktop's +> `Notification::update()`. Disable with `replace_notifications = false`. + #### `notification.canceled` A notification was dismissed by the device. diff --git a/internal/config/plugins.go b/internal/config/plugins.go index 2ec05a5..dcb210d 100644 --- a/internal/config/plugins.go +++ b/internal/config/plugins.go @@ -43,6 +43,14 @@ type NotificationPluginConfig struct { MaxBodyLength int `toml:"max_body_length"` ExpireMS int `toml:"expire_ms"` SkipNonClearable bool `toml:"skip_non_clearable"` + // ReplaceNotifications replaces the previous desktop popup when the phone + // re-posts a notification with the same id (e.g. media/scrobble updates), + // instead of creating a new popup each time. On by default. + ReplaceNotifications bool `toml:"replace_notifications"` + // ShowIcons shows the app icon on desktop popups. Off by default — + // kcd notifications render without an icon. When enabled, the phone's + // icon is downloaded (subject to FetchIcons) and reused across re-posts. + ShowIcons bool `toml:"show_icons"` } type ShareConfig struct { @@ -118,6 +126,7 @@ func (c *NotificationPluginConfig) Defaults() { c.FetchIcons = true c.MaxBodyLength = 0 c.ExpireMS = -1 + c.ReplaceNotifications = true } type ClipboardConfig struct { diff --git a/internal/plugins/notification/notification.go b/internal/plugins/notification/notification.go index 7c286ad..cc9b118 100644 --- a/internal/plugins/notification/notification.go +++ b/internal/plugins/notification/notification.go @@ -28,12 +28,13 @@ type NotificationPlugin struct { bus *events.Bus tlsConfig *tls.Config logger *zap.Logger - notifIDs sync.Map // maps body.ID (string) -> desktop notify-send ID (string) + notifIDs sync.Map // maps deviceID|body.ID -> desktop notify-send ID (string) iconDir string // temp dir for cached notification icons cfg config.NotificationPluginConfig canCloseNotifs bool // whether notify-send supports --print-id mu sync.RWMutex filters config.NotificationConfig + newExec func(ctx context.Context, name string, args ...string) *exec.Cmd } // NewNotificationPlugin creates a NotificationPlugin. @@ -45,6 +46,7 @@ func NewNotificationPlugin(cfg config.NotificationPluginConfig, bus *events.Bus, bus: bus, tlsConfig: tlsConfig, logger: logger.With(zap.String("plugin", "notification")), + newExec: exec.CommandContext, } // Probe --print-id support by checking --help output. @@ -128,9 +130,9 @@ func (p *NotificationPlugin) Handle(ctx context.Context, dev device.Sender, pkt // Handle cancellation — close the corresponding desktop notification. if body.IsCancel { if body.ID != "" { - if desktopID, ok := p.notifIDs.LoadAndDelete(body.ID); ok { + if desktopID, ok := p.notifIDs.LoadAndDelete(p.notifKey(dev.ID(), body.ID)); ok { go func() { - _ = exec.CommandContext(context.Background(), "gdbus", "call", "--session", + _ = p.newExec(context.Background(), "gdbus", "call", "--session", "--dest", "org.freedesktop.Notifications", "--object-path", "/org/freedesktop/Notifications", "--method", "org.freedesktop.Notifications.CloseNotification", @@ -212,13 +214,22 @@ func (p *NotificationPlugin) Handle(ctx context.Context, dev device.Sender, pkt // Handlers must not block — all I/O in a goroutine. go func() { - iconPath := p.fetchIcon(ctx, appName, body.ID, remoteIP, payloadPort, payloadSize, hasIcon) - p.sendDesktopNotification(appName, body.ID, body.Title, text, iconPath) + var iconPath string + if p.cfg.ShowIcons { + iconPath = p.fetchIcon(ctx, appName, body.ID, remoteIP, payloadPort, payloadSize, hasIcon) + } + p.sendDesktopNotification(dev.ID(), appName, body.ID, body.Title, text, iconPath) }() return nil } +// notifKey scopes a notification id to its device so two paired phones with +// colliding Android notification keys don't replace each other's popups. +func (p *NotificationPlugin) notifKey(devID, id string) string { + return devID + "|" + id +} + // fetchIcon downloads the notification icon payload and returns the path to the // saved file, or an empty string if unavailable. func (p *NotificationPlugin) fetchIcon( @@ -229,7 +240,7 @@ func (p *NotificationPlugin) fetchIcon( size int64, hasIcon bool, ) string { - if !hasIcon || !p.cfg.FetchIcons || p.tlsConfig == nil || p.iconDir == "" { + if !p.cfg.FetchIcons || p.tlsConfig == nil || p.iconDir == "" { // Fall back to icon name derived from app name. return "" } @@ -239,11 +250,18 @@ func (p *NotificationPlugin) fetchIcon( safeName := nonAlphaNumeric.ReplaceAllString(appName, "_") iconPath := filepath.Join(p.iconDir, fmt.Sprintf("%s-%s.png", safeName, notifID)) - // Already cached from a previous notification from this app. + // Reuse the cached icon even when the phone re-posts the notification + // without an icon payload (Android only sends the bytes when the icon + // hash changes). Without this, every re-post would fall back to a theme + // icon name that doesn't exist, showing a placeholder image. if _, err := os.Stat(iconPath); err == nil { return iconPath } + if !hasIcon { + return "" + } + addr := fmt.Sprintf("%s:%d", remoteIP, port) dialer := &tls.Dialer{ NetDialer: &net.Dialer{Timeout: 10 * time.Second}, @@ -272,18 +290,10 @@ func (p *NotificationPlugin) fetchIcon( } // sendDesktopNotification calls notify-send with the collected parameters. -func (p *NotificationPlugin) sendDesktopNotification(appName, id, title, text, iconPath string) { - // Derive a fallback icon name from the app name when no payload icon is available. - iconArg := strings.ToLower(strings.ReplaceAll(appName, " ", "-")) - if iconPath != "" { - iconArg = iconPath - } - if iconArg == "" { - iconArg = "smartphone" - } - +func (p *NotificationPlugin) sendDesktopNotification(devID, appName, id, title, text, iconPath string) { // Dunst / mako / swaync: stack notifications from the same app so they - // replace each other instead of flooding the screen. + // replace each other instead of flooding the screen. Daemons that ignore + // this hint (e.g. Quickshell) are covered by --replace-id below. groupHint := "string:x-dunst-stack-tag:kcd-" + appName args := []string{"-a", appName} @@ -293,18 +303,45 @@ func (p *NotificationPlugin) sendDesktopNotification(appName, id, title, text, i if p.cfg.ExpireMS >= 0 { args = append(args, "-t", strconv.Itoa(p.cfg.ExpireMS)) } + + // No icon by default. Pass an explicit empty icon so daemons (e.g. + // Quickshell) don't fall back to deriving an icon name from the app name + // and render a placeholder. When show_icons is enabled, pass the phone's + // downloaded icon, falling back to a name derived from the app. + iconArg := "" + if p.cfg.ShowIcons { + iconArg = strings.ToLower(strings.ReplaceAll(appName, " ", "-")) + if iconPath != "" { + iconArg = iconPath + } + if iconArg == "" { + iconArg = "smartphone" + } + } args = append(args, "-i", iconArg, "-h", groupHint) if p.canCloseNotifs && id != "" { + // Android re-posts notifications on every update with a stable id + // (e.g. media/scrobble now-playing). Replace the existing desktop + // popup via D-Bus replaces_id so repeated updates collapse to one, + // mirroring the reference desktop's Notification::update(). Disable + // via `replace_notifications = false`. + if p.cfg.ReplaceNotifications { + if prevID, ok := p.notifIDs.Load(p.notifKey(devID, id)); ok { + if s, ok := prevID.(string); ok && s != "" { + args = append(args, "-r", s) + } + } + } args = append(args, "--print-id", title, text) } else { args = append(args, title, text) } - out, err := exec.CommandContext(context.Background(), "notify-send", args...).Output() + out, err := p.newExec(context.Background(), "notify-send", args...).Output() if err == nil && p.canCloseNotifs && id != "" { if desktopID := strings.TrimSpace(string(out)); desktopID != "" { - p.notifIDs.Store(id, desktopID) + p.notifIDs.Store(p.notifKey(devID, id), desktopID) } } } diff --git a/internal/plugins/notification/notification_test.go b/internal/plugins/notification/notification_test.go index 641420a..9bc5953 100644 --- a/internal/plugins/notification/notification_test.go +++ b/internal/plugins/notification/notification_test.go @@ -2,6 +2,13 @@ package notification import ( "context" + "crypto/tls" + "fmt" + "os" + "os/exec" + "path/filepath" + "strconv" + "sync" "testing" "github.com/bethropolis/kcd/internal/config" @@ -20,6 +27,10 @@ func newPlugin(t *testing.T) *NotificationPlugin { // tlsConfig is nil — icon fetching is skipped in unit tests. p := NewNotificationPlugin(cfg, bus, nil, logger) t.Cleanup(p.Close) + // Never touch the real notification daemon from tests. + p.newExec = func(_ context.Context, name string, args ...string) *exec.Cmd { + return exec.CommandContext(context.Background(), "true") + } return p } @@ -45,7 +56,7 @@ func TestNotificationPlugin_Handle_Cancel(t *testing.T) { dev := device.NewDevice("dev1", "Test", "phone", logger) // Store a fake desktop ID so the cancel path can look it up. - p.notifIDs.Store("notif-abc", "42") + p.notifIDs.Store(p.notifKey(dev.ID(), "notif-abc"), "42") body := NotificationBody{ ID: "notif-abc", @@ -57,7 +68,7 @@ func TestNotificationPlugin_Handle_Cancel(t *testing.T) { } // Entry should have been removed. - if _, ok := p.notifIDs.Load("notif-abc"); ok { + if _, ok := p.notifIDs.Load(p.notifKey(dev.ID(), "notif-abc")); ok { t.Error("expected notifIDs entry to be removed after cancel") } } @@ -78,3 +89,156 @@ func TestNotificationPlugin_Handle_Silent(t *testing.T) { t.Fatalf("Handle returned error: %v", err) } } + +// fakeNotifier records the notify-send invocations and fakes a print-id. +type fakeNotifier struct { + mu sync.Mutex + calls [][]string + nextID int +} + +func (f *fakeNotifier) command(name string, args ...string) *exec.Cmd { + f.mu.Lock() + defer f.mu.Unlock() + f.calls = append(f.calls, append([]string{name}, args...)) + f.nextID++ + id := strconv.Itoa(f.nextID) + // Simulate notify-send --print-id printing the new desktop id. + return exec.CommandContext(context.Background(), "sh", "-c", "printf '%s' '"+id+"'") +} + +func (f *fakeNotifier) argFor(call int, flag string) string { + for i, a := range f.calls[call] { + if a == flag { + if i+1 < len(f.calls[call]) { + return f.calls[call][i+1] + } + } + } + return "" +} + +func newFakePlugin(t *testing.T, replace bool) (*NotificationPlugin, *fakeNotifier) { + t.Helper() + logger := zaptest.NewLogger(t) + bus := events.NewBus(logger) + cfg := config.NotificationPluginConfig{} + cfg.Defaults() + cfg.ReplaceNotifications = replace + p := NewNotificationPlugin(cfg, bus, nil, logger) + t.Cleanup(p.Close) + p.canCloseNotifs = true + f := &fakeNotifier{} + p.newExec = func(_ context.Context, name string, args ...string) *exec.Cmd { + return f.command(name, args...) + } + return p, f +} + +func TestNotificationPlugin_ReplaceByID(t *testing.T) { + p, f := newFakePlugin(t, true) + dev := device.NewDevice("dev1", "Test", "phone", zaptest.NewLogger(t)) + id := "0|com.arn.scrobble|0|com.msob7y.namida|10247" + + // First post — no replace, stores the printed desktop id. + p.sendDesktopNotification(dev.ID(), "Pano Scrobbler", id, "Song", "Artist", "") + if r := f.argFor(0, "-r"); r != "" { + t.Fatalf("expected no replace on first post, got -r %q", r) + } + key := p.notifKey(dev.ID(), id) + if v, ok := p.notifIDs.Load(key); !ok || v != "1" { + t.Fatalf("expected stored desktop id 1 under %q, got %v/%v", key, ok, v) + } + + // Second post, same id — must replace the previous popup. + p.sendDesktopNotification(dev.ID(), "Pano Scrobbler", id, "Song", "Artist", "") + if r := f.argFor(1, "-r"); r != "1" { + t.Fatalf("expected -r 1 on update, got %q", r) + } + if v, ok := p.notifIDs.Load(key); !ok || v != "2" { + t.Fatalf("expected stored desktop id 2, got %v/%v", ok, v) + } + + // Same id from a different device must not replace this device's popup. + other := device.NewDevice("dev2", "Other", "phone", zaptest.NewLogger(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) + } + if v, ok := p.notifIDs.Load(p.notifKey(other.ID(), id)); !ok || v != "3" { + t.Fatalf("expected stored desktop id 3 under dev2 key, got %v/%v", ok, v) + } + + // After a cancel, the entry is gone and the next post is fresh again. + p.notifIDs.Delete(key) + p.sendDesktopNotification(dev.ID(), "Pano Scrobbler", id, "Song", "Artist", "") + if r := f.argFor(3, "-r"); r != "" { + t.Fatalf("expected no replace after cancel, got -r %q", r) + } +} + +func TestNotificationPlugin_ReplaceByIDDisabled(t *testing.T) { + p, f := newFakePlugin(t, false) + dev := device.NewDevice("dev1", "Test", "phone", zaptest.NewLogger(t)) + id := "0|com.arn.scrobble|0|com.msob7y.namida|10247" + + // Seed a stored id — with replacement disabled it must be ignored. + p.notifIDs.Store(p.notifKey(dev.ID(), id), "7") + + p.sendDesktopNotification(dev.ID(), "Pano Scrobbler", id, "Song", "Artist", "") + if r := f.argFor(0, "-r"); r != "" { + t.Fatalf("expected no replace when config disabled, got -r %q", r) + } + // The print-id is still captured so cancel/close keeps working. + if v, ok := p.notifIDs.Load(p.notifKey(dev.ID(), id)); !ok || v != "1" { + t.Fatalf("expected stored desktop id 1, got %v/%v", ok, v) + } +} + +func TestNotificationPlugin_FetchIconReusesCacheOnRepost(t *testing.T) { + p := newPlugin(t) + p.tlsConfig = &tls.Config{} + p.iconDir = t.TempDir() + app := "Pano Scrobbler" + id := "0|com.arn.scrobble|0|com.msob7y.namida|10247" + + // Seed the cached icon for this (app, id) — simulates the first post + // having downloaded the phone's icon. + cachedPath := filepath.Join(p.iconDir, fmt.Sprintf("%s-%s.png", + nonAlphaNumeric.ReplaceAllString(app, "_"), id)) + if err := os.WriteFile(cachedPath, []byte("png"), 0o600); err != nil { + t.Fatal(err) + } + + // Re-post without an icon payload must reuse the cached file so the + // popup keeps the real app icon instead of a placeholder. + got := p.fetchIcon(context.Background(), app, id, nil, 0, 0, false) + if got != cachedPath { + t.Fatalf("expected cached icon reuse %q, got %q", cachedPath, got) + } + + // Unknown id, no payload, no cache → empty (theme fallback downstream). + if got := p.fetchIcon(context.Background(), app, "unknown-id", nil, 0, 0, false); got != "" { + t.Fatalf("expected empty icon for uncached payload-less repost, got %q", got) + } +} + +func TestNotificationPlugin_ShowIconsGating(t *testing.T) { + dev := device.NewDevice("dev1", "Test", "phone", zaptest.NewLogger(t)) + + // Default (show_icons = false): an explicit empty icon — popups render + // icon-less and daemons won't derive a placeholder from the app name. + p, f := newFakePlugin(t, true) + p.sendDesktopNotification(dev.ID(), "Pano Scrobbler", "id-1", "Song", "Artist", "") + if got := f.argFor(0, "-i"); got != "" { + t.Fatalf("expected empty -i by default, got -i %q", got) + } + + // show_icons = true: -i carries the icon path/name. + p2, f2 := newFakePlugin(t, true) + p2.cfg.ShowIcons = true + p2.sendDesktopNotification(dev.ID(), "Pano Scrobbler", "id-2", "Song", "Artist", "/tmp/icon.png") + if got := f2.argFor(0, "-i"); got != "/tmp/icon.png" { + t.Fatalf("expected -i /tmp/icon.png with icons enabled, got %q", got) + } +} diff --git a/packaging/kcd.example.toml b/packaging/kcd.example.toml index 3d21c45..3ff4167 100644 --- a/packaging/kcd.example.toml +++ b/packaging/kcd.example.toml @@ -209,6 +209,8 @@ suspend = "systemctl suspend" # max_body_length = 0 # 0 = no limit (truncates with … if set) # expire_ms = -1 # notification timeout in milliseconds, -1 = default # skip_non_clearable = false # skip non-dismissible notifications (media playback, etc.) +# replace_notifications = true # replace the same notification in place on re-posts (e.g. scrobble now-playing) instead of flooding new popups +# show_icons = false # show app icons on desktop popups (downloaded from the phone); off by default # ─── Share: file transfer settings ──────────────────────────────────────────── From 2eed0dd8d52fd1701862d033df79d3ade77ecf11 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Fri, 7 Aug 2026 23:23:23 +0300 Subject: [PATCH 19/25] fix: watch json-mode reconnect + clipboard nil-logger guard --- cmd/kcd/cli_watch.go | 2 +- internal/plugins/clipboard/clipboard.go | 3 +++ internal/plugins/clipboard/clipboard_test.go | 6 +++--- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/cmd/kcd/cli_watch.go b/cmd/kcd/cli_watch.go index 84f77af..cadbd1c 100644 --- a/cmd/kcd/cli_watch.go +++ b/cmd/kcd/cli_watch.go @@ -128,7 +128,7 @@ var watchCmd = &cli.Command{ err := cl.Watch(ctx, c.StringSlice("events"), ch) // If err != nil, the connection failed or disconnected - if !isJSON && err != nil { + if err != nil { if err == context.Canceled { return nil } diff --git a/internal/plugins/clipboard/clipboard.go b/internal/plugins/clipboard/clipboard.go index eed77b0..8f8585e 100644 --- a/internal/plugins/clipboard/clipboard.go +++ b/internal/plugins/clipboard/clipboard.go @@ -43,6 +43,9 @@ type ClipboardPlugin struct { // NewClipboardPlugin creates a clipboard plugin. func NewClipboardPlugin(tlsConfig *tls.Config, logger *zap.Logger, pushOnConnect bool) *ClipboardPlugin { + if logger == nil { + logger = zap.NewNop() + } return &ClipboardPlugin{ tlsConfig: tlsConfig, pushOnConnect: pushOnConnect, diff --git a/internal/plugins/clipboard/clipboard_test.go b/internal/plugins/clipboard/clipboard_test.go index 47db2ce..b24755b 100644 --- a/internal/plugins/clipboard/clipboard_test.go +++ b/internal/plugins/clipboard/clipboard_test.go @@ -6,13 +6,13 @@ import ( "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/protocol" - "go.uber.org/zap/zaptest" + "go.uber.org/zap" ) func TestClipboardPlugin_Handle(t *testing.T) { - logger := zaptest.NewLogger(t) + logger := zap.NewNop() dev := device.NewDevice("dev1", "Test", "phone", logger) - p := &ClipboardPlugin{} + p := NewClipboardPlugin(nil, logger, false) body := ClipboardBody{ Content: "Hello world!", From 4bb4680d17caa367397d148914501601d4e809ee Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Sat, 8 Aug 2026 03:02:18 +0300 Subject: [PATCH 20/25] feat(notification): debounce cancel/re-post to stop popup flicker --- docs/IPC_PROTOCOL.md | 9 ++ internal/config/plugins.go | 6 + internal/plugins/notification/notification.go | 64 ++++++++-- .../plugins/notification/notification_test.go | 114 ++++++++++++++++++ packaging/kcd.example.toml | 1 + 5 files changed, 184 insertions(+), 10 deletions(-) diff --git a/docs/IPC_PROTOCOL.md b/docs/IPC_PROTOCOL.md index ab2c596..4250b19 100644 --- a/docs/IPC_PROTOCOL.md +++ b/docs/IPC_PROTOCOL.md @@ -839,6 +839,15 @@ A notification was received from a device. > in place (`--replace-id`) so repeated updates collapse to one popup instead > of flooding the screen — mirroring the reference desktop's > `Notification::update()`. Disable with `replace_notifications = false`. +> +> When the phone **cancels** a notification (e.g. a scrobbler's now-playing +> popup torn down on pause), the daemon defers closing the desktop popup by +> `cancel_grace_ms` (default `1500`). If the same notification id is re-posted +> within that window (rapid play/pause toggling), the popup is updated in +> place instead of flickering closed and open. A real dismissal — no re-post — +> closes the popup after the grace window. Set `cancel_grace_ms = 0` to close +> immediately on cancel. The `notification.canceled` event is always emitted +> immediately on the cancel packet, regardless of the grace window. #### `notification.canceled` diff --git a/internal/config/plugins.go b/internal/config/plugins.go index dcb210d..2323e8d 100644 --- a/internal/config/plugins.go +++ b/internal/config/plugins.go @@ -51,6 +51,11 @@ type NotificationPluginConfig struct { // kcd notifications render without an icon. When enabled, the phone's // icon is downloaded (subject to FetchIcons) and reused across re-posts. ShowIcons bool `toml:"show_icons"` + // CancelGraceMS defers closing the desktop popup after the phone cancels + // a notification. If the same notification is re-posted within the window + // (e.g. media now-playing toggling play/pause), the popup is updated in + // place instead of closing and re-opening. 0 closes immediately. + CancelGraceMS int `toml:"cancel_grace_ms"` } type ShareConfig struct { @@ -127,6 +132,7 @@ func (c *NotificationPluginConfig) Defaults() { c.MaxBodyLength = 0 c.ExpireMS = -1 c.ReplaceNotifications = true + c.CancelGraceMS = 1500 } type ClipboardConfig struct { diff --git a/internal/plugins/notification/notification.go b/internal/plugins/notification/notification.go index cc9b118..991481f 100644 --- a/internal/plugins/notification/notification.go +++ b/internal/plugins/notification/notification.go @@ -29,6 +29,7 @@ type NotificationPlugin struct { tlsConfig *tls.Config logger *zap.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 cfg config.NotificationPluginConfig canCloseNotifs bool // whether notify-send supports --print-id @@ -70,6 +71,12 @@ func (p *NotificationPlugin) Close() { if p.iconDir != "" { _ = os.RemoveAll(p.iconDir) } + p.pendingCloses.Range(func(k, v any) bool { + if t, ok := v.(*time.Timer); ok { + t.Stop() + } + return true + }) } // SetFilters atomically replaces the per-app notification filter map. @@ -130,15 +137,34 @@ func (p *NotificationPlugin) Handle(ctx context.Context, dev device.Sender, pkt // Handle cancellation — close the corresponding desktop notification. if body.IsCancel { if body.ID != "" { - if desktopID, ok := p.notifIDs.LoadAndDelete(p.notifKey(dev.ID(), body.ID)); ok { - go func() { - _ = p.newExec(context.Background(), "gdbus", "call", "--session", - "--dest", "org.freedesktop.Notifications", - "--object-path", "/org/freedesktop/Notifications", - "--method", "org.freedesktop.Notifications.CloseNotification", - desktopID.(string), - ).Run() - }() + key := p.notifKey(dev.ID(), body.ID) + if desktopID, ok := p.notifIDs.Load(key); ok { + if p.cfg.CancelGraceMS > 0 { + // Cancel-grace: hold the popup open so a same-id re-post + // (media now-playing toggling play/pause) updates it in + // place instead of closing and re-opening. If no re-post + // arrives, close after the grace window. + if t, ok := p.pendingCloses.Load(key); ok { + t.(*time.Timer).Stop() + } + want := desktopID.(string) + p.pendingCloses.Store(key, time.AfterFunc( + time.Duration(p.cfg.CancelGraceMS)*time.Millisecond, + func() { + if cur, ok := p.notifIDs.LoadAndDelete(key); ok { + // Only close the popup we scheduled — if a + // re-post replaced it, leave the new one. + if cur.(string) == want { + p.closeNotification(want) + } + } + p.pendingCloses.Delete(key) + }, + )) + } else { + p.notifIDs.Delete(key) + p.closeNotification(desktopID.(string)) + } } } if p.bus != nil { @@ -289,6 +315,18 @@ func (p *NotificationPlugin) fetchIcon( return iconPath } +// closeNotification closes a previously-shown desktop popup by id. +func (p *NotificationPlugin) closeNotification(desktopID string) { + go func() { + _ = p.newExec(context.Background(), "gdbus", "call", "--session", + "--dest", "org.freedesktop.Notifications", + "--object-path", "/org/freedesktop/Notifications", + "--method", "org.freedesktop.Notifications.CloseNotification", + desktopID, + ).Run() + }() +} + // sendDesktopNotification calls notify-send with the collected parameters. func (p *NotificationPlugin) sendDesktopNotification(devID, appName, id, title, text, iconPath string) { // Dunst / mako / swaync: stack notifications from the same app so they @@ -327,7 +365,13 @@ func (p *NotificationPlugin) sendDesktopNotification(devID, appName, id, title, // mirroring the reference desktop's Notification::update(). Disable // via `replace_notifications = false`. if p.cfg.ReplaceNotifications { - if prevID, ok := p.notifIDs.Load(p.notifKey(devID, id)); ok { + key := p.notifKey(devID, id) + // Cancel-grace: if a cancel was deferred for this key, drop it so + // the popup is updated in place rather than closed and re-opened. + if t, ok := p.pendingCloses.LoadAndDelete(key); ok { + t.(*time.Timer).Stop() + } + if prevID, ok := p.notifIDs.Load(key); ok { if s, ok := prevID.(string); ok && s != "" { args = append(args, "-r", s) } diff --git a/internal/plugins/notification/notification_test.go b/internal/plugins/notification/notification_test.go index 9bc5953..bcf81e9 100644 --- a/internal/plugins/notification/notification_test.go +++ b/internal/plugins/notification/notification_test.go @@ -10,6 +10,7 @@ import ( "strconv" "sync" "testing" + "time" "github.com/bethropolis/kcd/internal/config" "github.com/bethropolis/kcd/internal/device" @@ -52,6 +53,8 @@ func TestNotificationPlugin_Handle_Normal(t *testing.T) { 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) dev := device.NewDevice("dev1", "Test", "phone", logger) @@ -242,3 +245,114 @@ func TestNotificationPlugin_ShowIconsGating(t *testing.T) { t.Fatalf("expected -i /tmp/icon.png with icons enabled, got %q", got) } } + +// gdbusCalls returns the recorded CloseNotification invocations. +func (f *fakeNotifier) gdbusCalls() [][]string { + f.mu.Lock() + defer f.mu.Unlock() + var out [][]string + for _, c := range f.calls { + if len(c) > 0 && c[0] == "gdbus" { + out = append(out, c) + } + } + return out +} + +func waitFor(t *testing.T, what string, cond func() bool) { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if cond() { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("timed out waiting for %s", what) +} + +func postAndCancel(t *testing.T, p *NotificationPlugin, dev device.Sender, id string) { + t.Helper() + p.sendDesktopNotification(dev.ID(), "Pano Scrobbler", id, "Song", "Artist", "") + body := NotificationBody{ID: id, IsCancel: true} + pkt, _ := protocol.NewPacket("kdeconnect.notification", body) + if err := p.Handle(context.Background(), dev, pkt); err != nil { + t.Fatalf("Handle(cancel) returned error: %v", err) + } +} + +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)) + id := "0|com.arn.scrobble|0|com.msob7y.namida|10247" + key := p.notifKey(dev.ID(), id) + + // Post then cancel — the close is deferred, the popup stays open. + postAndCancel(t, p, dev, id) + if len(f.gdbusCalls()) != 0 { + t.Fatalf("expected no immediate close under cancel-grace, got %d gdbus calls", len(f.gdbusCalls())) + } + if v, ok := p.notifIDs.Load(key); !ok || v != "1" { + t.Fatalf("expected desktop id 1 retained during grace, got %v/%v", ok, v) + } + + // Re-post within the window — must update in place, not close+reopen. + p.sendDesktopNotification(dev.ID(), "Pano Scrobbler", id, "Song", "Artist", "") + if r := f.argFor(1, "-r"); r != "1" { + t.Fatalf("expected -r 1 on re-post during grace, got %q", r) + } + + // Wait past the grace window: the deferred close must have been cancelled. + time.Sleep(250 * time.Millisecond) + if len(f.gdbusCalls()) != 0 { + t.Fatalf("expected deferred close cancelled on re-post, got %d gdbus calls", len(f.gdbusCalls())) + } + if v, ok := p.notifIDs.Load(key); !ok || v != "2" { + t.Fatalf("expected desktop id 2 after in-place update, got %v/%v", ok, v) + } +} + +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)) + id := "0|com.arn.scrobble|0|com.msob7y.namida|10247" + key := p.notifKey(dev.ID(), id) + + postAndCancel(t, p, dev, id) + if len(f.gdbusCalls()) != 0 { + t.Fatalf("expected no immediate close under cancel-grace, got %d gdbus calls", len(f.gdbusCalls())) + } + + // No re-post: after the grace window the popup is closed and the entry dropped. + waitFor(t, "deferred close", func() bool { + if len(f.gdbusCalls()) != 1 { + return false + } + _, ok := p.notifIDs.Load(key) + return !ok + }) + if got := f.gdbusCalls()[0][len(f.gdbusCalls()[0])-1]; got != "1" { + t.Fatalf("expected gdbus close of desktop id 1, got %q", got) + } +} + +func TestNotificationPlugin_CancelGraceDisabled(t *testing.T) { + p, f := newFakePlugin(t, true) + p.cfg.CancelGraceMS = 0 + dev := device.NewDevice("dev1", "Test", "phone", zaptest.NewLogger(t)) + id := "0|com.arn.scrobble|0|com.msob7y.namida|10247" + key := p.notifKey(dev.ID(), id) + + postAndCancel(t, p, dev, id) + waitFor(t, "immediate gdbus close", func() bool { + if _, ok := p.notifIDs.Load(key); ok { + return false + } + return len(f.gdbusCalls()) == 1 + }) + if got := f.gdbusCalls()[0][len(f.gdbusCalls()[0])-1]; got != "1" { + t.Fatalf("expected gdbus close of desktop id 1, got %q", got) + } +} diff --git a/packaging/kcd.example.toml b/packaging/kcd.example.toml index 3ff4167..c6f7334 100644 --- a/packaging/kcd.example.toml +++ b/packaging/kcd.example.toml @@ -211,6 +211,7 @@ suspend = "systemctl suspend" # skip_non_clearable = false # skip non-dismissible notifications (media playback, etc.) # replace_notifications = true # replace the same notification in place on re-posts (e.g. scrobble now-playing) instead of flooding new popups # show_icons = false # show app icons on desktop popups (downloaded from the phone); off by default +# cancel_grace_ms = 1500 # delay closing a popup after a phone cancel; a same-id re-post within this window updates in place instead of flickering (0 = close immediately) # ─── Share: file transfer settings ──────────────────────────────────────────── From 3f55c55c12be0af5391faed7488dedd3554f91dd Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Sat, 8 Aug 2026 16:08:22 +0300 Subject: [PATCH 21/25] fix(clipboard): re-probe backend and bind unit to graphical session Clipboard stopped working on daemon restarts because the backend was probed once via sync.Once at first use. When the daemon boots before the compositor (systemd user default.target), the probe resolved to unknown and stayed cached for the process lifetime. - probeBackend: prefer a live WAYLAND_DISPLAY socket, else scan $XDG_RUNTIME_DIR for wayland-* sockets, else X11 fallback. - getBackend: re-probe while unknown, cache only non-unknown results. - clipboardCmd: inject the probed WAYLAND_DISPLAY into subprocess env and bound subprocesses with a 2s timeout so a hung wl-paste cannot stall clipboard sync. - kcd-user.service: After/PartOf/WantedBy graphical-session.target so the daemon starts after the compositor exports the display env. --- internal/plugins/clipboard/clipboard.go | 146 ++++++++++------- internal/plugins/clipboard/clipboard_test.go | 161 +++++++++++++++++++ packaging/kcd-user.service | 10 +- 3 files changed, 254 insertions(+), 63 deletions(-) diff --git a/internal/plugins/clipboard/clipboard.go b/internal/plugins/clipboard/clipboard.go index 8f8585e..0aee839 100644 --- a/internal/plugins/clipboard/clipboard.go +++ b/internal/plugins/clipboard/clipboard.go @@ -35,7 +35,8 @@ type ClipboardPlugin struct { tlsConfig *tls.Config logger *zap.Logger backend clipboardBackend - backendOnce sync.Once + wlDisplay string // WAYLAND_DISPLAY value for spawned subprocesses + probe func() (clipboardBackend, string) mu sync.Mutex lastContent string // last content received from phone (inbound) lastPushedContent string // last content sent to phone (outbound) @@ -50,67 +51,91 @@ func NewClipboardPlugin(tlsConfig *tls.Config, logger *zap.Logger, pushOnConnect tlsConfig: tlsConfig, pushOnConnect: pushOnConnect, logger: logger.With(zap.String("plugin", "clipboard")), + probe: probeBackend, } } -// detectBackend probes for a working clipboard tool (wl-paste → xclip) -// and caches the result so the probe runs at most once. +// probeBackend determines the usable clipboard backend (wl-paste/xclip) by +// inspecting the environment and $XDG_RUNTIME_DIR. It is side-effect free and +// unit-testable. The returned string is the WAYLAND_DISPLAY value to inject +// into spawned subprocesses (empty for X11/unknown). // -// Under systemd user services neither WAYLAND_DISPLAY nor DISPLAY -// is typically set. We probe by checking the display variable first, -// then falling back to scanning $XDG_RUNTIME_DIR for a Wayland socket. -func (p *ClipboardPlugin) detectBackend() clipboardBackend { - p.backendOnce.Do(func() { - wlDisplay := os.Getenv("WAYLAND_DISPLAY") - xDisplay := os.Getenv("DISPLAY") - - switch { - case wlDisplay != "": +// A Wayland socket is preferred over DISPLAY: under a systemd user service +// WAYLAND_DISPLAY is often unset at startup, and treating DISPLAY as the +// backend on a Wayland session silently copies to the X clipboard where +// Wayland-native apps never see it. +func probeBackend() (clipboardBackend, string) { + rtDir := os.Getenv("XDG_RUNTIME_DIR") + + // Wayland: trust WAYLAND_DISPLAY only if its socket actually exists, + // otherwise scan the runtime dir for any live wayland-* socket. + if disp := os.Getenv("WAYLAND_DISPLAY"); disp != "" && rtDir != "" { + if _, err := os.Stat(filepath.Join(rtDir, disp)); err == nil { if _, err := exec.LookPath("wl-paste"); err == nil { - p.backend = backendWayland - p.logger.Debug("clipboard: backend=wayland (WAYLAND_DISPLAY set)") + return backendWayland, disp } - case xDisplay != "": - if _, err := exec.LookPath("xclip"); err == nil { - p.backend = backendX11 - p.logger.Debug("clipboard: backend=x11 (DISPLAY set)") - } - default: - // Systemd user service — neither variable is set. - // Probe $XDG_RUNTIME_DIR for any Wayland socket. - rtDir := os.Getenv("XDG_RUNTIME_DIR") - hasWaylandSock := false - if rtDir != "" { - if entries, err := os.ReadDir(rtDir); err == nil { - for _, e := range entries { - if strings.HasPrefix(e.Name(), "wayland-") && !e.IsDir() { - hasWaylandSock = true - break - } + } + } + if rtDir != "" { + if entries, err := os.ReadDir(rtDir); err == nil { + for _, e := range entries { + name := e.Name() + if !e.IsDir() && strings.HasPrefix(name, "wayland-") { + if _, err := exec.LookPath("wl-paste"); err == nil { + return backendWayland, name } } } - if hasWaylandSock { - if _, err := exec.LookPath("wl-paste"); err == nil { - p.backend = backendWayland - p.logger.Debug("clipboard: backend=wayland (socket probe)") - } - } - if p.backend == backendUnknown { - if _, err := exec.LookPath("xclip"); err == nil { - p.backend = backendX11 - p.logger.Debug("clipboard: backend=x11 (fallback)") - } - } } + } - if p.backend == backendUnknown { - p.logger.Warn("clipboard: no clipboard tool found (install wl-clipboard or xclip)") + // X11 fallback. + if os.Getenv("DISPLAY") != "" { + if _, err := exec.LookPath("xclip"); err == nil { + return backendX11, "" } - }) - return p.backend + } + return backendUnknown, "" +} + +// getBackend returns the cached backend, re-probing while it is unknown so an +// early failed probe (compositor not up yet, env not imported) does not stick +// for the lifetime of the process. Only non-unknown results are cached. +func (p *ClipboardPlugin) getBackend() (clipboardBackend, string) { + p.mu.Lock() + defer p.mu.Unlock() + if p.backend != backendUnknown { + return p.backend, p.wlDisplay + } + backend, disp := p.probe() + if backend != backendUnknown { + p.backend = backend + p.wlDisplay = disp + p.logger.Debug("clipboard: backend detected", + zap.Int("backend", int(backend)), zap.String("wl_display", disp)) + } + return backend, disp } +// clipboardCmd builds an exec.Cmd for a clipboard tool with a strict timeout +// (a hung wl-paste must not stall the daemon) and with WAYLAND_DISPLAY injected +// into the subprocess environment from the probed socket, so the tool works +// even when the daemon's own environment lacks the variable. +func (p *ClipboardPlugin) clipboardCmd(ctx context.Context, name string, args ...string) *exec.Cmd { + ctx, cancel := context.WithTimeout(ctx, clipboardTimeout) + cmd := exec.CommandContext(ctx, name, args...) + cmd.WaitDelay = time.Second + if _, disp := p.getBackend(); disp != "" { + cmd.Env = append(os.Environ(), "WAYLAND_DISPLAY="+disp) + } + _ = cancel // the context's timer owns the deadline; cancel fires on its own + return cmd +} + +// clipboardTimeout bounds every wl-copy/wl-paste/xclip subprocess so a hung +// clipboard tool cannot block clipboard sync indefinitely. +const clipboardTimeout = 2 * time.Second + // ClipboardBody represents the content of a clipboard packet. type ClipboardBody struct { Content string `json:"content"` @@ -171,12 +196,13 @@ func (p *ClipboardPlugin) Handle(ctx context.Context, dev device.Sender, pkt *pr // Spawning goroutine as Handlers must not block. go func() { var cmd *exec.Cmd - switch p.detectBackend() { + switch backend, _ := p.getBackend(); backend { case backendWayland: - cmd = exec.CommandContext(context.Background(), "wl-copy") + cmd = p.clipboardCmd(context.Background(), "wl-copy") case backendX11: - cmd = exec.CommandContext(context.Background(), "xclip", "-selection", "clipboard") + cmd = p.clipboardCmd(context.Background(), "xclip", "-selection", "clipboard") default: + p.logger.Debug("clipboard: no backend available, dropping inbound copy") return } @@ -251,11 +277,11 @@ func (p *ClipboardPlugin) handleClipboardFile(ctx context.Context, dev device.Se defer t.Close() var cmd *exec.Cmd - switch p.detectBackend() { + switch backend, _ := p.getBackend(); backend { case backendWayland: - cmd = exec.CommandContext(context.Background(), "wl-copy", "--type", mimeType) + cmd = p.clipboardCmd(context.Background(), "wl-copy", "--type", mimeType) case backendX11: - cmd = exec.CommandContext(context.Background(), "xclip", "-selection", "clipboard", "-t", mimeType, "-i") + cmd = p.clipboardCmd(context.Background(), "xclip", "-selection", "clipboard", "-t", mimeType, "-i") default: return } @@ -300,11 +326,11 @@ func downloadToFile(ctx context.Context, ip net.IP, port int, size int64, dest s // Push copies the local clipboard to the remote device using wl-paste or xclip -o. func Push(ctx context.Context, dev device.Sender, p *ClipboardPlugin) error { var cmd *exec.Cmd - switch p.detectBackend() { + switch backend, _ := p.getBackend(); backend { case backendWayland: - cmd = exec.CommandContext(ctx, "wl-paste", "-n") + cmd = p.clipboardCmd(ctx, "wl-paste", "-n") case backendX11: - cmd = exec.CommandContext(ctx, "xclip", "-selection", "clipboard", "-o") + cmd = p.clipboardCmd(ctx, "xclip", "-selection", "clipboard", "-o") default: return fmt.Errorf("clipboard: no clipboard tool available") } @@ -343,11 +369,11 @@ func Push(ctx context.Context, dev device.Sender, p *ClipboardPlugin) error { func (p *ClipboardPlugin) readClipboard() string { var cmd *exec.Cmd - switch p.detectBackend() { + switch backend, _ := p.getBackend(); backend { case backendWayland: - cmd = exec.CommandContext(context.Background(), "wl-paste", "-n") + cmd = p.clipboardCmd(context.Background(), "wl-paste", "-n") case backendX11: - cmd = exec.CommandContext(context.Background(), "xclip", "-selection", "clipboard", "-o") + cmd = p.clipboardCmd(context.Background(), "xclip", "-selection", "clipboard", "-o") default: return "" } diff --git a/internal/plugins/clipboard/clipboard_test.go b/internal/plugins/clipboard/clipboard_test.go index b24755b..53277d7 100644 --- a/internal/plugins/clipboard/clipboard_test.go +++ b/internal/plugins/clipboard/clipboard_test.go @@ -2,17 +2,178 @@ package clipboard import ( "context" + "os" + "path/filepath" "testing" + "time" "github.com/bethropolis/kcd/internal/device" "github.com/bethropolis/kcd/internal/protocol" "go.uber.org/zap" ) +// setProbe points the plugin's probe at fn for the lifetime of the plugin. +// It is safe to call before any async goroutine spawns (tests set it, then +// never write it again, so the fire-and-forget Handle goroutine only reads). +func setProbe(t *testing.T, p *ClipboardPlugin, fn func() (clipboardBackend, string)) { + t.Helper() + p.probe = fn +} + +// shimBin puts executable stand-ins for the given tools at the front of PATH, +// so probeBackend does not depend on what's installed on the host. +func shimBin(t *testing.T, names ...string) { + t.Helper() + dir := t.TempDir() + for _, n := range names { + p := filepath.Join(dir, n) + if err := os.WriteFile(p, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatal(err) + } + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) +} + +func TestProbeBackend_WaylandFromSocket(t *testing.T) { + // No WAYLAND_DISPLAY in the env, but a live socket in the runtime dir — + // the systemd-user-service boot case. Must resolve to Wayland. + t.Setenv("WAYLAND_DISPLAY", "") + t.Setenv("DISPLAY", ":0") + t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) + shimBin(t, "wl-paste") + sock := filepath.Join(os.Getenv("XDG_RUNTIME_DIR"), "wayland-1") + if err := os.WriteFile(sock, nil, 0o600); err != nil { + t.Fatal(err) + } + + backend, disp := probeBackend() + if backend != backendWayland { + t.Fatalf("expected wayland via socket probe, got %v", backend) + } + if disp != "wayland-1" { + t.Fatalf("expected wayland-1, got %q", disp) + } +} + +func TestProbeBackend_WaylandFromEnv(t *testing.T) { + t.Setenv("WAYLAND_DISPLAY", "wayland-0") + t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) + shimBin(t, "wl-paste") + sock := filepath.Join(os.Getenv("XDG_RUNTIME_DIR"), "wayland-0") + if err := os.WriteFile(sock, nil, 0o600); err != nil { + t.Fatal(err) + } + + backend, disp := probeBackend() + if backend != backendWayland || disp != "wayland-0" { + t.Fatalf("expected wayland-0, got backend=%v disp=%q", backend, disp) + } +} + +func TestProbeBackend_StaleEnvFallsBackToSocket(t *testing.T) { + // WAYLAND_DISPLAY points at a socket that no longer exists, but another + // live socket is present — the probe must not trust the stale var. + t.Setenv("WAYLAND_DISPLAY", "wayland-99") + t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) + shimBin(t, "wl-paste") + sock := filepath.Join(os.Getenv("XDG_RUNTIME_DIR"), "wayland-3") + if err := os.WriteFile(sock, nil, 0o600); err != nil { + t.Fatal(err) + } + + backend, disp := probeBackend() + if backend != backendWayland || disp != "wayland-3" { + t.Fatalf("expected fallback to wayland-3, got backend=%v disp=%q", backend, disp) + } +} + +func TestProbeBackend_X11Fallback(t *testing.T) { + t.Setenv("WAYLAND_DISPLAY", "") + t.Setenv("DISPLAY", ":0") + t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) + shimBin(t, "xclip") + + backend, disp := probeBackend() + if backend != backendX11 { + t.Fatalf("expected x11 fallback, got %v", backend) + } + if disp != "" { + t.Fatalf("expected empty display for x11, got %q", disp) + } +} + +func TestProbeBackend_None(t *testing.T) { + t.Setenv("WAYLAND_DISPLAY", "") + t.Setenv("DISPLAY", "") + t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) + shimBin(t, "wl-paste", "xclip") + + backend, _ := probeBackend() + if backend != backendUnknown { + t.Fatalf("expected unknown backend, got %v", backend) + } +} + +func TestClipboardPlugin_ReProbesAfterFailure(t *testing.T) { + p := NewClipboardPlugin(nil, zap.NewNop(), false) + + // First probe fails (compositor not up at boot) — nothing cached. + setProbe(t, p, func() (clipboardBackend, string) { return backendUnknown, "" }) + if b, _ := p.getBackend(); b != backendUnknown { + t.Fatalf("expected unknown on first probe, got %v", b) + } + + // Wayland becomes available later — the next call must re-probe and cache it. + setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-1" }) + if b, d := p.getBackend(); b != backendWayland || d != "wayland-1" { + t.Fatalf("expected wayland-1 after re-probe, got backend=%v disp=%q", b, d) + } + + // Now cached — a broken probe afterwards must not clobber it. + setProbe(t, p, func() (clipboardBackend, string) { return backendUnknown, "" }) + if b, d := p.getBackend(); b != backendWayland || d != "wayland-1" { + t.Fatalf("expected cached wayland-1, got backend=%v disp=%q", b, d) + } +} + +func TestClipboardPlugin_CmdInjectsWaylandEnv(t *testing.T) { + p := NewClipboardPlugin(nil, zap.NewNop(), false) + setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-9" }) + + cmd := p.clipboardCmd(context.Background(), "wl-copy") + found := false + for _, kv := range cmd.Env { + if kv == "WAYLAND_DISPLAY=wayland-9" { + found = true + break + } + } + if !found { + t.Fatalf("expected WAYLAND_DISPLAY=wayland-9 in subprocess env, got %v", cmd.Env) + } +} + +func TestClipboardPlugin_CmdHasTimeout(t *testing.T) { + p := NewClipboardPlugin(nil, zap.NewNop(), false) + setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-1" }) + + // A hung subprocess (sleep 30) must be killed by the 2s context timeout. + start := time.Now() + cmd := p.clipboardCmd(context.Background(), "sleep", "30") + if err := cmd.Run(); err == nil { + t.Fatal("expected a timeout error from a hung clipboard subprocess") + } + if wait := time.Since(start); wait > 3*clipboardTimeout { + t.Fatalf("expected subprocess killed within ~2s, took %v", wait) + } +} + func TestClipboardPlugin_Handle(t *testing.T) { logger := zap.NewNop() dev := device.NewDevice("dev1", "Test", "phone", logger) p := NewClipboardPlugin(nil, logger, false) + // Force the no-backend path so no real clipboard tool is invoked. + setProbe(t, p, func() (clipboardBackend, string) { return backendUnknown, "" }) body := ClipboardBody{ Content: "Hello world!", diff --git a/packaging/kcd-user.service b/packaging/kcd-user.service index 9a6f608..7a69bd2 100644 --- a/packaging/kcd-user.service +++ b/packaging/kcd-user.service @@ -1,9 +1,13 @@ [Unit] Description=kcd — Headless KDE Connect Daemon Documentation=https://github.com/bethropolis/kcd -# Wait for a network interface to be up before starting. -After=network-online.target +# Wait for the graphical session: the compositor exports WAYLAND_DISPLAY into +# the systemd user manager at startup, and kcd's clipboard plugin probes the +# Wayland socket to spawn wl-copy/wl-paste. Starting at default.target means +# kcd boots before the compositor and misses the display environment. +After=network-online.target graphical-session.target Wants=network-online.target +PartOf=graphical-session.target [Service] # sd_notify: systemd waits for the daemon to send READY=1 before marking @@ -43,4 +47,4 @@ TasksMax=64 PrivateNetwork=false [Install] -WantedBy=default.target +WantedBy=graphical-session.target From 73b32fafcc01e31344ba99bbe12aa17f3bb6d030 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Sat, 8 Aug 2026 16:08:31 +0300 Subject: [PATCH 22/25] fix(daemon): escalate reconnect backoff across connection flaps A peer that keeps connecting then dropping (e.g. a dying phone) previously spawned a fresh auto-reconnect loop at attempt 0 on every disconnect, pinning the backoff at the 2s floor forever and hammering the peer. Persist the attempt counter on the device: a successful dial bumps it, and the next cycle continues backing off if the connection drops before reconnectFlapThreshold (15s). A connection that survives that long counts as stable and resets the counter on drop. --- internal/daemon/transport.go | 21 +++++++++++- internal/device/device.go | 52 ++++++++++++++++++++++++++++++ internal/device/device_test.go | 58 ++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) diff --git a/internal/daemon/transport.go b/internal/daemon/transport.go index 0ce77c5..f36c80b 100644 --- a/internal/daemon/transport.go +++ b/internal/daemon/transport.go @@ -17,6 +17,12 @@ import ( "go.uber.org/zap" ) +// reconnectFlapThreshold is the minimum connection lifetime for it to count +// as genuinely stable. A connection that drops sooner is treated as a flap +// (e.g. a peer that keeps dying), so the auto-reconnect backoff continues +// escalating instead of resetting to the 2s floor on every drop. +const reconnectFlapThreshold = 15 * time.Second + // DialDevice manually connects to a device at the given IP and port. 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) { addr := fmt.Sprintf("%s:%d", targetIP, targetPort) @@ -227,6 +233,14 @@ func handleNewConnection(ctx context.Context, conn *transport.Conn, identity *pr if lastIP == nil { return } + // A connection that lasted long enough was genuinely stable, so the + // next drop should start the backoff over. A flap (connection dies + // shortly after a successful dial, e.g. a dying peer) keeps the + // counter so the backoff keeps escalating instead of hammering at the + // 2s floor forever. + if sender.ConnectionAge() >= reconnectFlapThreshold { + sender.ResetReconnectAttempt() + } // Prevent multiple concurrent reconnect goroutines for the same device. if !sender.TryReconnect() { logger.Debug("auto-reconnect: already reconnecting, skipping", @@ -260,7 +274,7 @@ func reconnectWithBackoff( logger *zap.Logger, ) { const maxBackoff = 5 * time.Minute - attempt := 0 + attempt := dev.ReconnectAttempt() defer dev.ReconnectDone() @@ -321,6 +335,11 @@ func reconnectWithBackoff( zap.String("device_id", dev.ID()), zap.Int("attempts", attempt+1), ) + // Persist the counter before returning: if this connection flaps, + // the next reconnect cycle continues backing off rather than + // resetting to the 2s floor. onDisconnect resets it if the + // connection proved stable. + dev.SetReconnectAttempt(attempt + 1) return } diff --git a/internal/device/device.go b/internal/device/device.go index a027eac..9afdac7 100644 --- a/internal/device/device.go +++ b/internal/device/device.go @@ -43,6 +43,17 @@ type Device struct { // auto-reconnect goroutines for this device. reconnecting atomic.Bool + // reconnectAttempt persists the auto-reconnect backoff counter across + // disconnect cycles. A connection that flaps (drops shortly after a + // successful dial) keeps the counter so the backoff escalates instead of + // resetting to the 2s floor; a stable connection resets it on drop. + reconnectAttempt int + + // connectStarted is when the most recent connection was established, + // used to detect flaps (connections that die too quickly to count as + // genuinely stable). + connectStarted time.Time + // pluginDispatch routes incoming packets to registered plugins pluginDispatch func(ctx context.Context, dev *Device, pkt *protocol.Packet) bool onConnect func(dev *Device) @@ -109,6 +120,12 @@ func (d *Device) Connect(ctx context.Context, conn *transport.Conn, dispatch fun d.mu.Unlock() } + // Record when the connection was established so a quick drop can be + // distinguished from a genuinely stable connection. + d.mu.Lock() + d.connectStarted = time.Now() + d.mu.Unlock() + go d.readLoop(ctx, conn) go d.writerLoop(ctx, conn) } @@ -314,3 +331,38 @@ func (d *Device) TryReconnect() bool { func (d *Device) ReconnectDone() { d.reconnecting.Store(false) } + +// ReconnectAttempt returns the persisted auto-reconnect backoff counter. +func (d *Device) ReconnectAttempt() int { + d.mu.RLock() + defer d.mu.RUnlock() + return d.reconnectAttempt +} + +// SetReconnectAttempt stores the auto-reconnect backoff counter so the next +// reconnect cycle (spawned when this connection drops) continues backing off +// instead of resetting to the initial floor. +func (d *Device) SetReconnectAttempt(n int) { + d.mu.Lock() + d.reconnectAttempt = n + d.mu.Unlock() +} + +// ResetReconnectAttempt clears the backoff counter after a stable connection +// (one that stayed up long enough to count as genuinely healthy) drops. +func (d *Device) ResetReconnectAttempt() { + d.mu.Lock() + d.reconnectAttempt = 0 + d.mu.Unlock() +} + +// ConnectionAge returns how long the current connection has been established, +// or 0 if the device has never connected in this process. +func (d *Device) ConnectionAge() time.Duration { + d.mu.RLock() + defer d.mu.RUnlock() + if d.connectStarted.IsZero() { + return 0 + } + return time.Since(d.connectStarted) +} diff --git a/internal/device/device_test.go b/internal/device/device_test.go index d61ab91..5c3b2df 100644 --- a/internal/device/device_test.go +++ b/internal/device/device_test.go @@ -1,9 +1,13 @@ package device import ( + "context" + "crypto/tls" + "net" "testing" "time" + "github.com/bethropolis/kcd/internal/transport" "go.uber.org/zap/zaptest" ) @@ -48,3 +52,57 @@ func TestReconnectBackoff(t *testing.T) { } } } + +func TestDevice_ReconnectAttempt(t *testing.T) { + logger := zaptest.NewLogger(t) + d := NewDevice("123", "Phone", "phone", logger) + + if got := d.ReconnectAttempt(); got != 0 { + t.Fatalf("expected initial attempt 0, got %d", got) + } + + d.SetReconnectAttempt(3) + if got := d.ReconnectAttempt(); got != 3 { + t.Fatalf("expected attempt 3, got %d", got) + } + + d.ResetReconnectAttempt() + if got := d.ReconnectAttempt(); got != 0 { + t.Fatalf("expected attempt 0 after reset, got %d", got) + } +} + +func TestDevice_ConnectionAge(t *testing.T) { + logger := zaptest.NewLogger(t) + d := NewDevice("123", "Phone", "phone", logger) + + if got := d.ConnectionAge(); got != 0 { + t.Fatalf("expected 0 for never-connected device, got %v", got) + } + + // Connect with a pipe-based TLS connection (no handshake required). + left, right := net.Pipe() + conn := transport.NewConn(tls.Client(left, &tls.Config{InsecureSkipVerify: true})) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + d.Connect(ctx, conn, nil, nil, nil) + + if got := d.ConnectionAge(); got <= 0 { + t.Fatalf("expected positive age after connect, got %v", got) + } + + time.Sleep(20 * time.Millisecond) + if got := d.ConnectionAge(); got < 20*time.Millisecond { + t.Errorf("expected age to grow past 20ms, got %v", got) + } + + // Tear down: closing the pipe unblocks readLoop, which clears the + // connection. Wait for it so no goroutine logs after the test returns. + _ = right.Close() + deadline := time.Now().Add(2 * time.Second) + for d.IsConnected() && time.Now().Before(deadline) { + time.Sleep(time.Millisecond) + } + d.Disconnect() +} From c0847737dbdcdc2d0d79f09c7090547bd355d5ea Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Sun, 9 Aug 2026 00:56:13 +0300 Subject: [PATCH 23/25] fix(clipboard): keep subprocess deadline alive and surface real tool errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clipboardCmd cancelled its 2s context immediately, so exec.CommandContext saw an already-cancelled context — its watchdog could SIGKILL wl-paste mid-flight, making clipboard pushes flaky. The timeout test passed for the wrong reason (instant kill, not the timer). - clipboardCmd no longer owns a deadline; runClipboard bounds execution with the 2s timeout and defers cancellation correctly. - runClipboard captures stderr and wraps it into the error, so failures report the real reason (e.g. 'Nothing is copied') instead of a bare 'exit status N'. - Push treats an empty/no-selection clipboard as nothing-to-push instead of a hard error, so kcd clipboard and --watch don't spam at startup before anything has been copied. - Tests: rework the timeout test to guard against instant-cancel regressions and cover stderr surfacing, no-selection detection, and empty/success push paths. --- internal/plugins/clipboard/clipboard.go | 102 +++++++++---- internal/plugins/clipboard/clipboard_test.go | 153 ++++++++++++++++++- 2 files changed, 219 insertions(+), 36 deletions(-) diff --git a/internal/plugins/clipboard/clipboard.go b/internal/plugins/clipboard/clipboard.go index 0aee839..f7181ec 100644 --- a/internal/plugins/clipboard/clipboard.go +++ b/internal/plugins/clipboard/clipboard.go @@ -1,6 +1,7 @@ package clipboard import ( + "bytes" "context" "crypto/tls" "encoding/json" @@ -117,21 +118,60 @@ func (p *ClipboardPlugin) getBackend() (clipboardBackend, string) { return backend, disp } -// clipboardCmd builds an exec.Cmd for a clipboard tool with a strict timeout -// (a hung wl-paste must not stall the daemon) and with WAYLAND_DISPLAY injected -// into the subprocess environment from the probed socket, so the tool works -// even when the daemon's own environment lacks the variable. -func (p *ClipboardPlugin) clipboardCmd(ctx context.Context, name string, args ...string) *exec.Cmd { - ctx, cancel := context.WithTimeout(ctx, clipboardTimeout) - cmd := exec.CommandContext(ctx, name, args...) +// clipboardCmd builds an exec.Cmd for a clipboard tool with WAYLAND_DISPLAY +// injected into the subprocess environment from the probed socket, so the +// tool works even when the daemon's own environment lacks the variable. +// The command itself carries no deadline; runClipboard applies the timeout +// around execution so a hung wl-paste can never stall the daemon. +func (p *ClipboardPlugin) clipboardCmd(name string, args ...string) *exec.Cmd { + // A background context: the deadline lives in runClipboard, which binds + // a bounded context around execution so a hung tool is killed there. + cmd := exec.CommandContext(context.Background(), name, args...) cmd.WaitDelay = time.Second if _, disp := p.getBackend(); disp != "" { cmd.Env = append(os.Environ(), "WAYLAND_DISPLAY="+disp) } - _ = cancel // the context's timer owns the deadline; cancel fires on its own return cmd } +// runClipboard runs a clipboard subprocess bounded by the clipboard timeout +// and captures any stderr the tool prints. On failure the stderr is wrapped +// into the returned error so the real reason (e.g. "No selection", a compositor +// error) is visible instead of a bare "exit status N". +func (p *ClipboardPlugin) runClipboard(ctx context.Context, cmd *exec.Cmd, stdin io.Reader) ([]byte, error) { + tctx, cancel := context.WithTimeout(ctx, clipboardTimeout) + defer cancel() + + timed := exec.CommandContext(tctx, cmd.Path, cmd.Args[1:]...) + timed.Env = cmd.Env + timed.WaitDelay = cmd.WaitDelay + timed.Stdin = stdin + + var stderr bytes.Buffer + timed.Stderr = &stderr + + out, err := timed.Output() + if err != nil { + if msg := strings.TrimSpace(stderr.String()); msg != "" { + err = fmt.Errorf("%w: %s", err, msg) + } + } + return out, err +} + +// isNoSelection reports whether a clipboard tool failure actually means the +// clipboard is empty (nothing to push) rather than a real problem with the +// tool or compositor. Matches wl-paste's and xclip's "nothing here" messages. +func isNoSelection(err error) bool { + if err == nil { + return false + } + s := strings.ToLower(err.Error()) + return strings.Contains(s, "no selection") || + strings.Contains(s, "nothing is copied") || + strings.Contains(s, "no data") +} + // clipboardTimeout bounds every wl-copy/wl-paste/xclip subprocess so a hung // clipboard tool cannot block clipboard sync indefinitely. const clipboardTimeout = 2 * time.Second @@ -195,20 +235,17 @@ func (p *ClipboardPlugin) Handle(ctx context.Context, dev device.Sender, pkt *pr // Spawning goroutine as Handlers must not block. go func() { - var cmd *exec.Cmd switch backend, _ := p.getBackend(); backend { case backendWayland: - cmd = p.clipboardCmd(context.Background(), "wl-copy") + if _, err := p.runClipboard(context.Background(), p.clipboardCmd("wl-copy"), strings.NewReader(body.Content)); err != nil { + p.logger.Warn("clipboard: failed to set clipboard", zap.Error(err)) + } case backendX11: - cmd = p.clipboardCmd(context.Background(), "xclip", "-selection", "clipboard") + if _, err := p.runClipboard(context.Background(), p.clipboardCmd("xclip", "-selection", "clipboard"), strings.NewReader(body.Content)); err != nil { + p.logger.Warn("clipboard: failed to set clipboard", zap.Error(err)) + } default: p.logger.Debug("clipboard: no backend available, dropping inbound copy") - return - } - - cmd.Stdin = strings.NewReader(body.Content) - if err := cmd.Run(); err != nil { - p.logger.Warn("clipboard: failed to set clipboard", zap.Error(err)) } }() @@ -279,15 +316,14 @@ func (p *ClipboardPlugin) handleClipboardFile(ctx context.Context, dev device.Se var cmd *exec.Cmd switch backend, _ := p.getBackend(); backend { case backendWayland: - cmd = p.clipboardCmd(context.Background(), "wl-copy", "--type", mimeType) + cmd = p.clipboardCmd("wl-copy", "--type", mimeType) case backendX11: - cmd = p.clipboardCmd(context.Background(), "xclip", "-selection", "clipboard", "-t", mimeType, "-i") + cmd = p.clipboardCmd("xclip", "-selection", "clipboard", "-t", mimeType, "-i") default: return } - cmd.Stdin = t - if out, err := cmd.CombinedOutput(); err != nil { - p.logger.Warn("clipboard file: failed to set clipboard", zap.Error(err), zap.String("output", string(out))) + if _, err := p.runClipboard(context.Background(), cmd, t); err != nil { + p.logger.Warn("clipboard file: failed to set clipboard", zap.Error(err)) } }() @@ -328,19 +364,29 @@ func Push(ctx context.Context, dev device.Sender, p *ClipboardPlugin) error { var cmd *exec.Cmd switch backend, _ := p.getBackend(); backend { case backendWayland: - cmd = p.clipboardCmd(ctx, "wl-paste", "-n") + cmd = p.clipboardCmd("wl-paste", "-n") case backendX11: - cmd = p.clipboardCmd(ctx, "xclip", "-selection", "clipboard", "-o") + cmd = p.clipboardCmd("xclip", "-selection", "clipboard", "-o") default: return fmt.Errorf("clipboard: no clipboard tool available") } - out, err := cmd.Output() + out, err := p.runClipboard(ctx, cmd, nil) if err != nil { + // An empty clipboard (fresh session, nothing copied yet) is not an + // error worth failing a push for — the tool exits non-zero with a + // "no selection"-style message. Treat it as nothing to push so the + // CLI and --watch don't spam errors until something is copied. + if isNoSelection(err) { + return nil + } return err } content := string(out) + if content == "" { + return nil + } p.mu.Lock() // Skip if content matches what we last received from the phone (lastContent) @@ -371,13 +417,13 @@ func (p *ClipboardPlugin) readClipboard() string { var cmd *exec.Cmd switch backend, _ := p.getBackend(); backend { case backendWayland: - cmd = p.clipboardCmd(context.Background(), "wl-paste", "-n") + cmd = p.clipboardCmd("wl-paste", "-n") case backendX11: - cmd = p.clipboardCmd(context.Background(), "xclip", "-selection", "clipboard", "-o") + cmd = p.clipboardCmd("xclip", "-selection", "clipboard", "-o") default: return "" } - out, err := cmd.Output() + out, err := p.runClipboard(context.Background(), cmd, nil) if err != nil { p.logger.Debug("clipboard: read failed", zap.Error(err)) return "" diff --git a/internal/plugins/clipboard/clipboard_test.go b/internal/plugins/clipboard/clipboard_test.go index 53277d7..d186f02 100644 --- a/internal/plugins/clipboard/clipboard_test.go +++ b/internal/plugins/clipboard/clipboard_test.go @@ -2,8 +2,14 @@ package clipboard import ( "context" + "crypto/x509" + "encoding/json" + "fmt" + "net" "os" + "os/exec" "path/filepath" + "strings" "testing" "time" @@ -140,7 +146,7 @@ func TestClipboardPlugin_CmdInjectsWaylandEnv(t *testing.T) { p := NewClipboardPlugin(nil, zap.NewNop(), false) setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-9" }) - cmd := p.clipboardCmd(context.Background(), "wl-copy") + cmd := p.clipboardCmd("wl-copy") found := false for _, kv := range cmd.Env { if kv == "WAYLAND_DISPLAY=wayland-9" { @@ -153,18 +159,149 @@ func TestClipboardPlugin_CmdInjectsWaylandEnv(t *testing.T) { } } -func TestClipboardPlugin_CmdHasTimeout(t *testing.T) { +func TestRunClipboard_Success(t *testing.T) { + p := NewClipboardPlugin(nil, zap.NewNop(), false) + + out, err := p.runClipboard(context.Background(), exec.CommandContext(context.Background(), "/bin/sh", "-c", "printf hello"), nil) + if err != nil { + t.Fatalf("expected success, got %v", err) + } + if string(out) != "hello" { + t.Fatalf("expected 'hello', got %q", out) + } +} + +func TestRunClipboard_WrapsStderr(t *testing.T) { p := NewClipboardPlugin(nil, zap.NewNop(), false) - setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-1" }) - // A hung subprocess (sleep 30) must be killed by the 2s context timeout. + _, err := p.runClipboard(context.Background(), exec.CommandContext(context.Background(), "/bin/sh", "-c", "echo boom >&2; exit 2"), nil) + if err == nil { + t.Fatal("expected an error") + } + // The real stderr reason must surface instead of a bare "exit status 2". + if !strings.Contains(err.Error(), "boom") { + t.Fatalf("expected stderr in error, got %v", err) + } +} + +func TestRunClipboard_TimesOutHungSubprocess(t *testing.T) { + p := NewClipboardPlugin(nil, zap.NewNop(), false) + + // A hung subprocess must be killed by the 2s timeout — and not instantly + // (that would regress to the old premature-cancel bug). start := time.Now() - cmd := p.clipboardCmd(context.Background(), "sleep", "30") - if err := cmd.Run(); err == nil { + _, err := p.runClipboard(context.Background(), exec.CommandContext(context.Background(), "/bin/sh", "-c", "sleep 30"), nil) + if err == nil { t.Fatal("expected a timeout error from a hung clipboard subprocess") } - if wait := time.Since(start); wait > 3*clipboardTimeout { - t.Fatalf("expected subprocess killed within ~2s, took %v", wait) + if elapsed := time.Since(start); elapsed < time.Second { + t.Fatalf("subprocess killed too early (instant cancel regression?), elapsed=%v", elapsed) + } else if elapsed > 3*clipboardTimeout { + t.Fatalf("expected subprocess killed within ~2s, took %v", elapsed) + } +} + +func TestIsNoSelection(t *testing.T) { + for _, msg := range []string{ + "exit status 1: wl-paste: No selection", + "exit status 1: Nothing is copied", + "exit status 1: Error: There is no data to be read", + } { + if !isNoSelection(fmt.Errorf("%s", msg)) { + t.Errorf("expected %q to be treated as no-selection", msg) + } + } + if isNoSelection(fmt.Errorf("exit status 1: failed to connect to compositor")) { + t.Error("compositor errors must remain hard failures") + } +} + +// fakeSender records packets like a device without touching the network. +type fakeSender struct { + sent []*protocol.Packet +} + +func (f *fakeSender) ID() string { return "fake" } +func (f *fakeSender) Name() string { return "fake" } +func (f *fakeSender) SetName(string) {} +func (f *fakeSender) State() device.PairingState { return device.StatePaired } +func (f *fakeSender) SetState(device.PairingState) { +} + +func (f *fakeSender) Send(p *protocol.Packet) error { + f.sent = append(f.sent, p) + return nil +} +func (f *fakeSender) IsConnected() bool { return true } +func (f *fakeSender) RemoteIP() net.IP { return net.ParseIP("127.0.0.1") } +func (f *fakeSender) PeerCert() *x509.Certificate { return nil } +func (f *fakeSender) HasCapability(string) bool { return false } +func (f *fakeSender) UpdateBattery(int, bool) {} +func (f *fakeSender) GetBattery() (int, bool) { return 0, false } + +// shimWith replaces PATH with a single scripted binary so Push exercises the +// real command execution without depending on the host's clipboard tools. +func shimWith(t *testing.T, name, script string) { + t.Helper() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, name), []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir) +} + +func TestPush_EmptyClipboardIsSoftFail(t *testing.T) { + p := NewClipboardPlugin(nil, zap.NewNop(), false) + setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-1" }) + + // Empty clipboard with nothing copied: wl-paste exits non-zero with a + // "no selection" diagnostic — push must not fail the CLI for this. + shimWith(t, "wl-paste", "#!/bin/sh\necho 'No selection' >&2\nexit 1\n") + dev := &fakeSender{} + + if err := Push(context.Background(), dev, p); err != nil { + t.Fatalf("expected soft pass on empty clipboard, got %v", err) + } + if len(dev.sent) != 0 { + t.Fatalf("expected no packet for empty clipboard, got %d", len(dev.sent)) + } +} + +func TestPush_EmptyOutputSendsNothing(t *testing.T) { + p := NewClipboardPlugin(nil, zap.NewNop(), false) + setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-1" }) + + // Tool reports success but no content — still nothing to push. + shimWith(t, "wl-paste", "#!/bin/sh\nexit 0\n") + dev := &fakeSender{} + + if err := Push(context.Background(), dev, p); err != nil { + t.Fatalf("expected success, got %v", err) + } + if len(dev.sent) != 0 { + t.Fatalf("expected no packet for empty output, got %d", len(dev.sent)) + } +} + +func TestPush_SendsContent(t *testing.T) { + p := NewClipboardPlugin(nil, zap.NewNop(), false) + setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-1" }) + + shimWith(t, "wl-paste", "#!/bin/sh\nprintf 'hello world'\n") + dev := &fakeSender{} + + if err := Push(context.Background(), dev, p); err != nil { + t.Fatalf("expected success, got %v", err) + } + if len(dev.sent) != 1 { + t.Fatalf("expected 1 packet, got %d", len(dev.sent)) + } + var body ClipboardBody + if err := json.Unmarshal(dev.sent[0].Body, &body); err != nil { + t.Fatalf("bad body: %v", err) + } + if body.Content != "hello world" { + t.Fatalf("expected 'hello world', got %q", body.Content) } } From 0945a616e3976a8615b8a23217a70b3fc18bc185 Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Mon, 10 Aug 2026 00:54:37 +0300 Subject: [PATCH 24/25] fix(clipboard): apply inbound clips and stop echoing them back to phone wl-copy forks a persistent manager that inherits the stdout/stderr pipes, so cmd.Output() blocked past the deadline and inbound copies were silently dropped; pushes then resubmitted stale or empty content to the phone, producing a bogus 'pasted from your clipboard' toast. Write with runCopy (fds pointed at the null device) so Run() returns once wl-copy exits. Also write clips with wl-copy -n and normalize trailing newlines in the dedup guard so a tooling newline cannot turn the phone's own content into an outbound echo. --- internal/plugins/clipboard/clipboard.go | 53 ++++++++++++++++---- internal/plugins/clipboard/clipboard_test.go | 53 ++++++++++++++++++-- 2 files changed, 94 insertions(+), 12 deletions(-) diff --git a/internal/plugins/clipboard/clipboard.go b/internal/plugins/clipboard/clipboard.go index f7181ec..cf77efa 100644 --- a/internal/plugins/clipboard/clipboard.go +++ b/internal/plugins/clipboard/clipboard.go @@ -138,14 +138,14 @@ func (p *ClipboardPlugin) clipboardCmd(name string, args ...string) *exec.Cmd { // and captures any stderr the tool prints. On failure the stderr is wrapped // into the returned error so the real reason (e.g. "No selection", a compositor // error) is visible instead of a bare "exit status N". -func (p *ClipboardPlugin) runClipboard(ctx context.Context, cmd *exec.Cmd, stdin io.Reader) ([]byte, error) { +func (p *ClipboardPlugin) runClipboard(ctx context.Context, cmd *exec.Cmd) ([]byte, error) { tctx, cancel := context.WithTimeout(ctx, clipboardTimeout) defer cancel() timed := exec.CommandContext(tctx, cmd.Path, cmd.Args[1:]...) timed.Env = cmd.Env timed.WaitDelay = cmd.WaitDelay - timed.Stdin = stdin + timed.Stdin = nil var stderr bytes.Buffer timed.Stderr = &stderr @@ -159,6 +159,26 @@ func (p *ClipboardPlugin) runClipboard(ctx context.Context, cmd *exec.Cmd, stdin return out, err } +// runCopy writes clipboard data via wl-copy/xclip -i. Unlike runClipboard it +// must NOT capture stdout/stderr through os.Pipe: wl-copy forks a persistent +// background manager that inherits the pipe fds, so the pipe never EOFs and +// cmd.Output()/cmd.Wait() would block until the manager dies. Pointing the +// child's fds at the null device instead makes Run() return as soon as the +// forking wl-copy process exits. wl-paste never forks, which is why the read +// path above can still use pipes. +func (p *ClipboardPlugin) runCopy(ctx context.Context, cmd *exec.Cmd, stdin io.Reader) error { + tctx, cancel := context.WithTimeout(ctx, clipboardTimeout) + defer cancel() + + timed := exec.CommandContext(tctx, cmd.Path, cmd.Args[1:]...) + timed.Env = cmd.Env + timed.WaitDelay = cmd.WaitDelay + timed.Stdin = stdin + timed.Stdout = nil + timed.Stderr = nil + return timed.Run() +} + // isNoSelection reports whether a clipboard tool failure actually means the // clipboard is empty (nothing to push) rather than a real problem with the // tool or compositor. Matches wl-paste's and xclip's "nothing here" messages. @@ -237,11 +257,15 @@ func (p *ClipboardPlugin) Handle(ctx context.Context, dev device.Sender, pkt *pr go func() { switch backend, _ := p.getBackend(); backend { case backendWayland: - if _, err := p.runClipboard(context.Background(), p.clipboardCmd("wl-copy"), strings.NewReader(body.Content)); err != nil { + // -n: wl-copy appends a trailing newline by default. Without it the + // local selection becomes content+"\n", which differs from the + // 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)) } case backendX11: - if _, err := p.runClipboard(context.Background(), p.clipboardCmd("xclip", "-selection", "clipboard"), strings.NewReader(body.Content)); err != nil { + 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)) } default: @@ -316,13 +340,13 @@ func (p *ClipboardPlugin) handleClipboardFile(ctx context.Context, dev device.Se var cmd *exec.Cmd switch backend, _ := p.getBackend(); backend { case backendWayland: - cmd = p.clipboardCmd("wl-copy", "--type", mimeType) + cmd = p.clipboardCmd("wl-copy", "-n", "--type", mimeType) case backendX11: cmd = p.clipboardCmd("xclip", "-selection", "clipboard", "-t", mimeType, "-i") default: return } - if _, err := p.runClipboard(context.Background(), cmd, t); err != nil { + if err := p.runCopy(context.Background(), cmd, t); err != nil { p.logger.Warn("clipboard file: failed to set clipboard", zap.Error(err)) } }() @@ -371,7 +395,7 @@ func Push(ctx context.Context, dev device.Sender, p *ClipboardPlugin) error { return fmt.Errorf("clipboard: no clipboard tool available") } - out, err := p.runClipboard(ctx, cmd, nil) + out, err := p.runClipboard(ctx, cmd) if err != nil { // An empty clipboard (fresh session, nothing copied yet) is not an // error worth failing a push for — the tool exits non-zero with a @@ -395,7 +419,11 @@ func Push(ctx context.Context, dev device.Sender, p *ClipboardPlugin) error { // lastContent guard: prevents sending the phone's own content back. // lastPushedContent guard: prevents duplicate pushes when the local // clipboard hasn't changed between two Push calls. - if content == p.lastContent || content == p.lastPushedContent { + // + // Comparisons are normalized of trailing newlines so a stray \n appended + // by some clipboard tooling cannot silently disable the guard and cause + // an echo of the phone's own content back to it. + if normClip(content) == normClip(p.lastContent) || normClip(content) == normClip(p.lastPushedContent) { p.mu.Unlock() return nil } @@ -413,6 +441,13 @@ func Push(ctx context.Context, dev device.Sender, p *ClipboardPlugin) error { return dev.Send(pkt) } +// normClip strips trailing newlines so content read back from wl-copy/xclip +// compares equal to the raw content we stored, regardless of which tool +// appended (or not) a trailing newline. +func normClip(s string) string { + return strings.TrimRight(s, "\n") +} + func (p *ClipboardPlugin) readClipboard() string { var cmd *exec.Cmd switch backend, _ := p.getBackend(); backend { @@ -423,7 +458,7 @@ func (p *ClipboardPlugin) readClipboard() string { default: return "" } - out, err := p.runClipboard(context.Background(), cmd, nil) + out, err := p.runClipboard(context.Background(), cmd) if err != nil { p.logger.Debug("clipboard: read failed", zap.Error(err)) return "" diff --git a/internal/plugins/clipboard/clipboard_test.go b/internal/plugins/clipboard/clipboard_test.go index d186f02..93b8f8a 100644 --- a/internal/plugins/clipboard/clipboard_test.go +++ b/internal/plugins/clipboard/clipboard_test.go @@ -162,7 +162,7 @@ func TestClipboardPlugin_CmdInjectsWaylandEnv(t *testing.T) { func TestRunClipboard_Success(t *testing.T) { p := NewClipboardPlugin(nil, zap.NewNop(), false) - out, err := p.runClipboard(context.Background(), exec.CommandContext(context.Background(), "/bin/sh", "-c", "printf hello"), nil) + out, err := p.runClipboard(context.Background(), exec.CommandContext(context.Background(), "/bin/sh", "-c", "printf hello")) if err != nil { t.Fatalf("expected success, got %v", err) } @@ -174,7 +174,7 @@ func TestRunClipboard_Success(t *testing.T) { func TestRunClipboard_WrapsStderr(t *testing.T) { p := NewClipboardPlugin(nil, zap.NewNop(), false) - _, err := p.runClipboard(context.Background(), exec.CommandContext(context.Background(), "/bin/sh", "-c", "echo boom >&2; exit 2"), nil) + _, err := p.runClipboard(context.Background(), exec.CommandContext(context.Background(), "/bin/sh", "-c", "echo boom >&2; exit 2")) if err == nil { t.Fatal("expected an error") } @@ -190,7 +190,7 @@ func TestRunClipboard_TimesOutHungSubprocess(t *testing.T) { // A hung subprocess must be killed by the 2s timeout — and not instantly // (that would regress to the old premature-cancel bug). start := time.Now() - _, err := p.runClipboard(context.Background(), exec.CommandContext(context.Background(), "/bin/sh", "-c", "sleep 30"), nil) + _, err := p.runClipboard(context.Background(), exec.CommandContext(context.Background(), "/bin/sh", "-c", "sleep 30")) if err == nil { t.Fatal("expected a timeout error from a hung clipboard subprocess") } @@ -305,6 +305,53 @@ func TestPush_SendsContent(t *testing.T) { } } +// TestPush_DoesNotEchoReceivedClipboard guards the echo bug: after the phone +// pushes content, the local clipboard holds it (possibly with a trailing +// 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) + setProbe(t, p, func() (clipboardBackend, string) { return backendWayland, "wayland-1" }) + + // Phone sent "hello"; wl-copy stored "hello\n" (no -n); wl-paste reads it + // back with the trailing newline still attached. + p.mu.Lock() + p.lastContent = "hello" + p.mu.Unlock() + shimWith(t, "wl-paste", "#!/bin/sh\nprintf 'hello\\n'\n") + dev := &fakeSender{} + + if err := Push(context.Background(), dev, p); err != nil { + t.Fatalf("expected success, got %v", err) + } + if len(dev.sent) != 0 { + t.Fatalf("expected no echo packet, got %d", len(dev.sent)) + } + + // Exact round-trip (what wl-copy -n produces) must also be suppressed. + p.mu.Lock() + p.lastContent = "new clip" + p.mu.Unlock() + shimWith(t, "wl-paste", "#!/bin/sh\nprintf 'new clip'\n") + dev = &fakeSender{} + + if err := Push(context.Background(), dev, p); err != nil { + t.Fatalf("expected success, got %v", err) + } + if len(dev.sent) != 0 { + t.Fatalf("expected no echo packet, got %d", len(dev.sent)) + } + + // Genuinely different content must still be pushed. + shimWith(t, "wl-paste", "#!/bin/sh\nprintf 'something else'\n") + if err := Push(context.Background(), dev, p); err != nil { + t.Fatalf("expected success, got %v", err) + } + if len(dev.sent) != 1 { + t.Fatalf("expected 1 packet, got %d", len(dev.sent)) + } +} + func TestClipboardPlugin_Handle(t *testing.T) { logger := zap.NewNop() dev := device.NewDevice("dev1", "Test", "phone", logger) From adb1bf832b39c1ae219d2e82ad326d008c4faf9b Mon Sep 17 00:00:00 2001 From: bethropolis <66518866+bethropolis@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:36:45 +0300 Subject: [PATCH 25/25] fix(clipboard): bound WaitDelay so forked clipboard helpers can't hang reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runClipboard/runCopy copy WaitDelay from the passed cmd, but a bare exec.CommandContext defaults it to 0 — which means cmd.Output() blocks on the stdout pipe until the last fd holder exits. A killed child that spawned a grandchild retaining the pipe (sh -c 'sleep 30 & wait' — and /bin/sh on CI runners) kept tests and reads pinned for the full 30s. Cap the delay at 1s so Wait always returns promptly after the 2s deadline. --- internal/plugins/clipboard/clipboard.go | 18 ++++++++++-- internal/plugins/clipboard/clipboard_test.go | 29 ++++++++++++++------ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/internal/plugins/clipboard/clipboard.go b/internal/plugins/clipboard/clipboard.go index cf77efa..4ab36eb 100644 --- a/internal/plugins/clipboard/clipboard.go +++ b/internal/plugins/clipboard/clipboard.go @@ -144,7 +144,7 @@ func (p *ClipboardPlugin) runClipboard(ctx context.Context, cmd *exec.Cmd) ([]by timed := exec.CommandContext(tctx, cmd.Path, cmd.Args[1:]...) timed.Env = cmd.Env - timed.WaitDelay = cmd.WaitDelay + timed.WaitDelay = capKillDelay(cmd.WaitDelay) timed.Stdin = nil var stderr bytes.Buffer @@ -172,13 +172,27 @@ func (p *ClipboardPlugin) runCopy(ctx context.Context, cmd *exec.Cmd, stdin io.R timed := exec.CommandContext(tctx, cmd.Path, cmd.Args[1:]...) timed.Env = cmd.Env - timed.WaitDelay = cmd.WaitDelay + timed.WaitDelay = capKillDelay(cmd.WaitDelay) timed.Stdin = stdin timed.Stdout = nil timed.Stderr = nil return timed.Run() } +// capKillDelay returns nonzero bound on how long Wait may block on the +// stdout/stderr pipes after the timeout kills the child. Callers built via +// clipboardCmd start with WaitDelay=1s, but a bare exec.CommandContext +// (tests, or any future caller) defaults to 0, which means Wait blocks until +// the pipes EOF — and a killed child that forked a grandchild holding those +// fds would pin runClipboard open until the grandchild exits (e.g. a whole +// 30s sleep). Ensure it always returns promptly after the deadline. +func capKillDelay(d time.Duration) time.Duration { + if d <= 0 { + return time.Second + } + return d +} + // isNoSelection reports whether a clipboard tool failure actually means the // clipboard is empty (nothing to push) rather than a real problem with the // tool or compositor. Matches wl-paste's and xclip's "nothing here" messages. diff --git a/internal/plugins/clipboard/clipboard_test.go b/internal/plugins/clipboard/clipboard_test.go index 93b8f8a..568071e 100644 --- a/internal/plugins/clipboard/clipboard_test.go +++ b/internal/plugins/clipboard/clipboard_test.go @@ -189,15 +189,26 @@ func TestRunClipboard_TimesOutHungSubprocess(t *testing.T) { // A hung subprocess must be killed by the 2s timeout — and not instantly // (that would regress to the old premature-cancel bug). - start := time.Now() - _, err := p.runClipboard(context.Background(), exec.CommandContext(context.Background(), "/bin/sh", "-c", "sleep 30")) - if err == nil { - t.Fatal("expected a timeout error from a hung clipboard subprocess") - } - if elapsed := time.Since(start); elapsed < time.Second { - t.Fatalf("subprocess killed too early (instant cancel regression?), elapsed=%v", elapsed) - } else if elapsed > 3*clipboardTimeout { - t.Fatalf("expected subprocess killed within ~2s, took %v", elapsed) + // + // Two shapes of "hung" are covered: a plain child (sh -c 'sleep 30') and + // a child that forks a grandchild still holding the stdout pipe + // (sh -c 'sleep 30 & wait'). The grandchild survives the parent's SIGKILL, + // so without a WaitDelay bound Output() would block on the pipe until the + // grandchild exits — pinning runClipboard open for the whole 30s. + for _, cmd := range []*exec.Cmd{ + exec.CommandContext(context.Background(), "/bin/sh", "-c", "sleep 30"), + exec.CommandContext(context.Background(), "/bin/sh", "-c", "sleep 30 & wait"), + } { + start := time.Now() + _, err := p.runClipboard(context.Background(), cmd) + if err == nil { + t.Fatal("expected a timeout error from a hung clipboard subprocess") + } + if elapsed := time.Since(start); elapsed < time.Second { + t.Fatalf("subprocess killed too early (instant cancel regression?), elapsed=%v", elapsed) + } else if elapsed > 3*clipboardTimeout { + t.Fatalf("expected subprocess killed within ~2s, took %v", elapsed) + } } }