From 2353350d79f14cd51e993ea2f1eec0fdd8c4f0bf Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 22:38:52 +0200 Subject: [PATCH 1/2] The clipboard keeps its history running and keeps it to itself (hq issues 743, 750) clipmenud is declared as the operator's user unit, enabled and running, with a launcher that finds the display. Copy reads the clipboard back and fails with the reason. History and paste answer counts, times and lengths, never content. --- modules/clipmenu/README.md | 20 +-- .../clipmenu/cmd/clipmenu-tools/clipboard.go | 115 ++++++++++++------ .../cmd/clipmenu-tools/clipboard_test.go | 112 +++++++++++++---- modules/clipmenu/cmd/clipmenu-tools/main.go | 34 +++--- .../cmd/clipmenu-tools/manifest_test.go | 98 +++++++++++++-- modules/clipmenu/module.json | 45 +++++-- 6 files changed, 319 insertions(+), 105 deletions(-) diff --git a/modules/clipmenu/README.md b/modules/clipmenu/README.md index 5b709125..b9f504c9 100644 --- a/modules/clipmenu/README.md +++ b/modules/clipmenu/README.md @@ -7,9 +7,15 @@ The clipboard manager as a module (novox/hq ADR 0208, research 026/04). - Claims the mesh's `node-clipboard` seat and serves its verbs `history` and `copy`. Requires `x11-display` on its own machine. - **Declares `rofi-greenclip` absent** (ADR 0180). This module replaces it. -- Starts `clipmenud` **once per session**, from the session's start (the `xinitrc` slot `normal`). - It is not a user unit as well. Its packaged unit needs user-scoped units (mesh-host #72, not - merged), and the session start alone is one starter. +- Declares `clipmenud.service` as a **user unit of the operator's account**, enabled and running + (`scope: user`, ADR 0177; issue 743), so `$mod+.` shows the history again. It is the only starter: the + session's `xinitrc` line is gone. A drop-in (`~/.config/systemd/user/clipmenud.service.d/mesh.conf`) + replaces `ExecStart` with the module's launcher, which finds the X display from its socket (the account's + service manager has no `DISPLAY`) and puts the text-only `xsel` first on the `PATH`; until the display is up + it waits (a quick exit would run into the start limit, which the drop-in also switches off, with `RestartSec=2s`). `DISPLAY` and `XAUTHORITY` come from the user manager when the session's xinitrc has handed them over (module xorg); with several X servers and no `DISPLAY` yet, it waits for them. The store is in the account's runtime directory (mode 0700), + so no other account reads it. + **After delivery the operator logs in again**: the running session keeps the `clipmenud` its xinitrc + started, and the new unit waits on clipmenud's session lock until that one ends. - Binds `$mod+period` to `clipmenu`, as its own i3 drop-in (`50-clipmenu.conf`). clipmenu shows the history through `dmenu`, the seat command of `node-launcher`, so it looks like every other menu. - Its settings are environment contributions (ADR 0203), read by the daemon, the menu and the tools @@ -23,11 +29,11 @@ The clipboard manager as a module (novox/hq ADR 0208, research 026/04). | tool | does | |---|---| -| `node-clipboard.history` | the history, newest first, each entry once with its id, first line, time, size and text (cut) | -| `node-clipboard.copy` | put text on the clipboard; it enters the history like any copy | -| `clipmenu_paste` | what the clipboard holds now | +| `node-clipboard.history` | the history, newest first: count, time and length of each entry, and whether the daemon collects. **Never a text, a first line or an id** (issue 750); the text is read through clipmenu on the operator's own screen | +| `node-clipboard.copy` | put text on the clipboard, then read it back; an error with the reason (no `DISPLAY`, `xsel` exited, other contents) when it does not hold the text. It enters the history like any copy | +| `clipmenu_paste` | how many bytes of text the clipboard holds now, never the text | | `clipmenu_clear` | forget the history; the daemon's locks stay | -| `clipmenu_delete` | forget one entry, by id or first line | +| `clipmenu_delete` | forget one entry, by its place in the history (1 is the newest) and the time the history gave it, refused if the history changed since; not by text or id | The history is read from clipmenu's own store, in the account's runtime directory, under clipmenu's own lock, so a copy arriving meanwhile is neither lost nor half-written. `copy` hands the text to an diff --git a/modules/clipmenu/cmd/clipmenu-tools/clipboard.go b/modules/clipmenu/cmd/clipmenu-tools/clipboard.go index b0250304..8eafed9f 100644 --- a/modules/clipmenu/cmd/clipmenu-tools/clipboard.go +++ b/modules/clipmenu/cmd/clipmenu-tools/clipboard.go @@ -15,8 +15,7 @@ import ( ) const ( - mostCopy = 1 << 20 - mostPaste = 64 << 10 + mostCopy = 1 << 20 // majorVersion is clipmenu's store layout: /clipmenu../. majorVersion = 6 ) @@ -71,15 +70,15 @@ func cksum(data []byte) string { func entryID(line string) string { return cksum([]byte(line + "\n")) } -// Entry is one clip in the history. +// Entry is one clip in the history as a caller sees it: when and how long, never what. The text of a +// clip, its first line and its id (a checksum of that line) stay in clipmenu's store, readable on the +// operator's own screen through clipmenu (issue 750). type Entry struct { - ID string `json:"id"` - Line string `json:"line"` - At string `json:"at"` - Bytes int `json:"bytes"` - Text string `json:"text,omitempty"` - Truncated bool `json:"truncated,omitempty"` - at int64 + At string `json:"at"` + Bytes int `json:"bytes"` + id string + line string + at int64 } // HistoryResult is what node-clipboard.history answers. @@ -119,14 +118,15 @@ func readStore(dir string) ([]Entry, error) { } out := make([]Entry, 0, len(latest)) for line, ns := range latest { - out = append(out, Entry{ID: entryID(line), Line: line, at: ns, At: time.Unix(0, ns).Format(time.RFC3339)}) + out = append(out, Entry{id: entryID(line), line: line, at: ns, At: time.Unix(0, ns).Format(time.RFC3339)}) } sort.Slice(out, func(i, j int) bool { return out[i].at > out[j].at }) return out, scan.Err() } -// History is the clipboard's history, newest first. -func History(limit, maxBytes int) (HistoryResult, error) { +// History is the clipboard's history, newest first: how many entries, when each was copied and how +// long each is, and whether the daemon is collecting. It never returns a copied text. +func History(limit int) (HistoryResult, error) { dir, err := storeDir(findEnvironment()) if err != nil { return HistoryResult{}, err @@ -140,15 +140,8 @@ func History(limit, maxBytes int) (HistoryResult, error) { if i == limit { break } - if info, err := os.Stat(filepath.Join(dir, e.ID)); err == nil { + if info, err := os.Stat(filepath.Join(dir, e.id)); err == nil { e.Bytes = int(info.Size()) - if maxBytes > 0 { - raw, _ := os.ReadFile(filepath.Join(dir, e.ID)) - if len(raw) > maxBytes { - raw, e.Truncated = raw[:maxBytes], true - } - e.Text = string(raw) - } } out.Entries = append(out.Entries, e) } @@ -159,6 +152,40 @@ func History(limit, maxBytes int) (HistoryResult, error) { type CopyResult struct { Bytes int `json:"bytes"` Unit string `json:"unit"` + // Verified says the clipboard was read back and held exactly the text. + Verified bool `json:"verified"` +} + +// readBackWait is the pause between reads of the clipboard while the owner starts; read-back tries 15 times. +var readBackWait = 200 * time.Millisecond + +// readBack waits for the clipboard to hold text and answers why when it does not. +func readBack(s Session, unit, text string) error { + var last string + for i := 0; i < 15; i++ { + time.Sleep(readBackWait) + r, err := s.run(3*time.Second, "", "xsel", "--output", "--clipboard") + switch { + case err != nil: + last = err.Error() + case r.Code != 0: + last = "xsel: " + strings.TrimSpace(r.Stderr) + case r.Stdout == text: + return nil + default: + last = fmt.Sprintf("it holds %d bytes of something else", len(r.Stdout)) + } + } + why := "" + if s.Display == "" { + why = "; the session has no DISPLAY, so the owner could not reach the screen" + } + if j, err := s.run(5*time.Second, "", "journalctl", "--user", "-u", unit+".service", "-n", "5", "--no-pager", "-o", "cat"); err == nil { + if tail := strings.TrimSpace(j.Stdout); tail != "" { + why += "; the owner said: " + tail + } + } + return fmt.Errorf("the clipboard does not hold the text (%d bytes): %s%s", len(text), last, why) } // Copy puts text on the clipboard. The clipboard is owned by a process until another copies, so the @@ -194,17 +221,20 @@ func Copy(text string) (CopyResult, error) { os.Remove(f.Name()) return CopyResult{}, err } - return CopyResult{Bytes: len(text), Unit: unit + ".service"}, nil + if err := readBack(s, unit, text); err != nil { + return CopyResult{}, err + } + return CopyResult{Bytes: len(text), Unit: unit + ".service", Verified: true}, nil } -// PasteResult is what clipmenu_paste answers. +// PasteResult is what clipmenu_paste answers: how much text the clipboard holds, never the text. The +// clipboard is the newest history entry, and a copied secret must not reach a caller (issue 750). type PasteResult struct { - Text string `json:"text"` - Bytes int `json:"bytes"` - Truncated bool `json:"truncated,omitempty"` + Bytes int `json:"bytes"` + Text bool `json:"holds_text"` } -// Paste reads the clipboard now. +// Paste reads the clipboard now and says how long it is. func Paste() (PasteResult, error) { s, err := findSession() if err != nil { @@ -217,11 +247,7 @@ func Paste() (PasteResult, error) { if r.Code != 0 { return PasteResult{}, fmt.Errorf("xsel: %s", strings.TrimSpace(r.Stderr)) } - out := PasteResult{Text: r.Stdout, Bytes: len(r.Stdout), Truncated: r.Truncated} - if len(out.Text) > mostPaste { - out.Text, out.Truncated = out.Text[:mostPaste], true - } - return out, nil + return PasteResult{Bytes: len(r.Stdout), Text: len(r.Stdout) > 0}, nil } // ChangeResult is what clear and delete answer. @@ -298,10 +324,12 @@ func Clear() (ChangeResult, error) { return out, err } -// Delete forgets one entry, by id or by its first line. -func Delete(id, line string) (ChangeResult, error) { - if (id == "") == (line == "") { - return ChangeResult{}, errors.New("give the entry's id or its line, one of the two") +// Delete forgets one entry, by its place in the history as node-clipboard.history lists it (1 is the +// newest) together with the time it reports, so a copy arriving since the listing cannot shift the +// place onto another entry. Not by its text or id: either would let a caller confirm a guess of a copied line. +func Delete(place int, at string) (ChangeResult, error) { + if place < 1 { + return ChangeResult{}, errors.New("give the entry's place in the history, 1 for the newest") } dir, err := storeOf() if err != nil { @@ -309,6 +337,17 @@ func Delete(id, line string) (ChangeResult, error) { } var out ChangeResult err = withStoreLock(dir, func() error { + entries, err := readStore(dir) + if err != nil { + return err + } + if place > len(entries) { + return fmt.Errorf("no entry %d: the clipboard history holds %d", place, len(entries)) + } + if entries[place-1].At != at { + return fmt.Errorf("entry %d was copied at %s, not at %s: the history changed since it was listed; list it again", place, entries[place-1].At, at) + } + line := entries[place-1].line raw, err := os.ReadFile(filepath.Join(dir, "line_cache")) if err != nil && !errors.Is(err, os.ErrNotExist) { return err @@ -319,7 +358,7 @@ func Delete(id, line string) (ChangeResult, error) { continue } _, text, _ := strings.Cut(l, " ") - if text == line || (id != "" && entryID(text) == id) { + if text == line { out.Removed++ _ = os.Remove(filepath.Join(dir, entryID(text))) continue @@ -327,7 +366,7 @@ func Delete(id, line string) (ChangeResult, error) { kept = append(kept, l) } if out.Removed == 0 { - return fmt.Errorf("no entry %s%s in the clipboard history", id, line) + return fmt.Errorf("no entry %d in the clipboard history", place) } content := strings.Join(kept, "\n") if content != "" { diff --git a/modules/clipmenu/cmd/clipmenu-tools/clipboard_test.go b/modules/clipmenu/cmd/clipmenu-tools/clipboard_test.go index 058b8ae9..69bc1d30 100644 --- a/modules/clipmenu/cmd/clipmenu-tools/clipboard_test.go +++ b/modules/clipmenu/cmd/clipmenu-tools/clipboard_test.go @@ -1,6 +1,7 @@ package main import ( + "encoding/json" "errors" "os" "os/exec" @@ -51,46 +52,84 @@ func store(t *testing.T, clips map[string]string, order ...string) string { return dir } -func TestTheHistoryIsNewestFirstOnceEachWithItsText(t *testing.T) { - store(t, map[string]string{"first": "first", "second (2 lines)": "second\nline two"}, "first", "second (2 lines)", "first") +func TestTheHistoryAnswersCountTimesAndLengthsNeverContent(t *testing.T) { + store(t, map[string]string{"first": "first", "sk-ant-api03-SECRETKEY (2 lines)": "sk-ant-api03-SECRETKEY\nline two"}, "first", "sk-ant-api03-SECRETKEY (2 lines)", "first") fakeProcess(t, nobody, "clipmenud") - h, err := History(10, 4096) + h, err := History(10) if err != nil { t.Fatal(err) } - if !h.Collecting || h.Total != 2 || h.Entries[0].Line != "first" || h.Entries[1].Text != "second\nline two" || h.Entries[1].Bytes != 15 { + if !h.Collecting || h.Total != 2 || len(h.Entries) != 2 || h.Entries[0].Bytes != 5 || h.Entries[1].Bytes != 31 || h.Entries[0].At == "" { t.Fatalf("%+v", h) } - if h, _ := History(1, 3); len(h.Entries) != 1 || h.Entries[0].Text != "fir" || !h.Entries[0].Truncated { - t.Fatalf("limited and cut: %+v", h) + raw, _ := json.Marshal(h) + for _, leak := range []string{"first", "SECRETKEY", "line two"} { + if strings.Contains(string(raw), leak) { + t.Fatalf("the history answers %q: %s", leak, raw) + } } - if h, _ := History(10, 0); h.Entries[0].Text != "" || h.Entries[0].Bytes != 5 { - t.Fatalf("without text: %+v", h) + if h, _ := History(1); len(h.Entries) != 1 { + t.Fatalf("limited: %+v", h) } } -func TestAnEntryIsDeletedByIdOrLineWithItsText(t *testing.T) { +// at is the time History reports for the entry at place. +func at(t *testing.T, place int) string { + t.Helper() + h, err := History(100) + if err != nil || place > len(h.Entries) { + t.Fatalf("%+v, %v", h, err) + } + return h.Entries[place-1].At +} + +func TestADeleteWhoseListingIsStaleIsRefused(t *testing.T) { + dir := store(t, map[string]string{"a": "a", "b": "b"}, "a", "b") + listed := at(t, 1) // b + f, _ := os.OpenFile(filepath.Join(dir, "line_cache"), os.O_APPEND|os.O_WRONLY, 0o600) + f.WriteString("1800000000000000000 a\n") // a copy arrives: place 1 is now a + f.Close() + if _, err := Delete(1, listed); err == nil || !strings.Contains(err.Error(), "list it again") { + t.Fatalf("the wrong entry was deleted: %v", err) + } + if h, _ := History(10); h.Total != 2 { + t.Fatalf("%+v", h) + } +} + +func TestAnEntryIsDeletedByItsPlaceWithItsTextAndNeverByItsText(t *testing.T) { dir := store(t, map[string]string{"a": "a", "b": "b", "c": "c"}, "a", "b", "c", "a") - r, err := Delete(entryID("a"), "") + // newest first: a, c, b + r, err := Delete(1, at(t, 1)) if err != nil || r.Removed != 2 || r.Remaining != 2 { - t.Fatalf("by id, both copies: %+v, %v", r, err) + t.Fatalf("by place, both copies: %+v, %v", r, err) } if _, err := os.Stat(filepath.Join(dir, entryID("a"))); !errors.Is(err, os.ErrNotExist) { t.Fatal("the text stayed") } - if r, err := Delete("", "b"); err != nil || r.Removed != 1 || r.Remaining != 1 { - t.Fatalf("by line: %+v, %v", r, err) + if r, err := Delete(2, at(t, 2)); err != nil || r.Removed != 1 || r.Remaining != 1 { + t.Fatalf("second place (b): %+v, %v", r, err) } - if _, err := Delete("", "zzz"); err == nil { - t.Fatal("a missing entry was reported deleted") + if _, err := Delete(5, "x"); err == nil { + t.Fatal("a missing place was reported deleted") } - if _, err := Delete("x", "y"); err == nil { - t.Fatal("both an id and a line were accepted") + if _, err := Delete(0, "x"); err == nil { + t.Fatal("place 0 was accepted") } cache, _ := os.ReadFile(filepath.Join(dir, "line_cache")) if !strings.HasSuffix(string(cache), " c\n") || strings.Count(string(cache), "\n") != 1 { t.Fatalf("line cache: %q", cache) } + for _, tool := range tools() { + if tool.Name == "clipmenu_delete" { + if _, ok := tool.Input["line"]; ok { + t.Fatal("delete still takes a line") + } + if _, ok := tool.Input["id"]; ok { + t.Fatal("delete still takes an id") + } + } + } } func TestClearForgetsEverythingButKeepsTheDaemonsLocks(t *testing.T) { @@ -112,13 +151,21 @@ func TestClearForgetsEverythingButKeepsTheDaemonsLocks(t *testing.T) { } } -func TestCopyHandsTheTextToAnOwnerUnderTheAccountsServiceManager(t *testing.T) { +func copyBed(t *testing.T, xsel string) string { + t.Helper() store(t, nil) fakeProcess(t, nobody, "i3", "DISPLAY=:1") - bin := fakeBinaries(t, map[string]string{"systemctl": "true", "systemd-run": `echo "$*" > "$LOG"; for last; do :; done; cat "$last" > "$LOG.text"`}) + bin := fakeBinaries(t, map[string]string{"systemctl": "true", "xsel": xsel, + "systemd-run": `echo "$*" > "$LOG"; for last; do :; done; cat "$last" > "$LOG.text"`}) t.Setenv("LOG", filepath.Join(bin, "log")) + readBackWait = 0 + return bin +} + +func TestCopyHandsTheTextToAnOwnerUnderTheAccountsServiceManagerAndReadsItBack(t *testing.T) { + bin := copyBed(t, `cat "$LOG.text"`) r, err := Copy("secret-free text") - if err != nil || r.Bytes != 16 || !strings.HasPrefix(r.Unit, "clipmenu-copy-") { + if err != nil || r.Bytes != 16 || !r.Verified || !strings.HasPrefix(r.Unit, "clipmenu-copy-") { t.Fatalf("%+v, %v", r, err) } asked, _ := os.ReadFile(filepath.Join(bin, "log")) @@ -134,7 +181,23 @@ func TestCopyHandsTheTextToAnOwnerUnderTheAccountsServiceManager(t *testing.T) { } } -func TestPasteReadsTheClipboardOrSaysThereIsNoSession(t *testing.T) { +func TestCopyFailsWhenTheClipboardHoldsSomethingElse(t *testing.T) { + copyBed(t, `printf 'old contents'`) + _, err := Copy("new text") + if err == nil || !strings.Contains(err.Error(), "does not hold") || strings.Contains(err.Error(), "old contents") { + t.Fatalf("a copy that did not land was reported: %v", err) + } +} + +func TestCopyFailsSayingWhyWhenTheOwnerCannotReachTheDisplay(t *testing.T) { + copyBed(t, `echo "xsel: Can't open display: (null)" >&2; exit 1`) + _, err := Copy("new text") + if err == nil || !strings.Contains(err.Error(), "Can't open display") { + t.Fatalf("the reason was not given: %v", err) + } +} + +func TestPasteSaysHowMuchTheClipboardHoldsNeverWhat(t *testing.T) { fakeMachine(t) if _, err := Paste(); !errors.Is(err, ErrNoSession) { t.Fatal(err) @@ -142,8 +205,9 @@ func TestPasteReadsTheClipboardOrSaysThereIsNoSession(t *testing.T) { fakeProcess(t, nobody, "i3", "DISPLAY=:1") fakeBinaries(t, map[string]string{"xsel": `[ "$*" = "--output --clipboard" ] && printf 'on the clipboard'`}) p, err := Paste() - if err != nil || p.Text != "on the clipboard" || p.Bytes != 16 { - t.Fatalf("%+v, %v", p, err) + raw, _ := json.Marshal(p) + if err != nil || p.Bytes != 16 || strings.Contains(string(raw), "on the clipboard") { + t.Fatalf("%s, %v", raw, err) } } @@ -157,7 +221,7 @@ func TestWithoutAStoreTheChangesSayWhy(t *testing.T) { if _, err := Clear(); err == nil || !strings.Contains(err.Error(), "clipmenud has not run") { t.Fatal(err) } - if h, err := History(5, 0); err != nil || h.Total != 0 || h.Collecting { + if h, err := History(5); err != nil || h.Total != 0 || h.Collecting { t.Fatalf("an empty history: %+v, %v", h, err) } } diff --git a/modules/clipmenu/cmd/clipmenu-tools/main.go b/modules/clipmenu/cmd/clipmenu-tools/main.go index e8e92129..822b99e7 100644 --- a/modules/clipmenu/cmd/clipmenu-tools/main.go +++ b/modules/clipmenu/cmd/clipmenu-tools/main.go @@ -22,28 +22,23 @@ func tools() []stdio.Tool { return []stdio.Tool{ { Name: "node-clipboard.history", - Description: "What the operator copied, newest first: each entry's id, its first line, when, its size " + - "and its text (each cut at max_bytes). Whether the clipboard daemon is collecting.", + Description: "What the operator copied, newest first: how many entries, when each was copied and how long " + + "it is, never what it says. Whether the clipboard daemon is collecting. The text is read on the operator's own screen, through clipmenu.", Input: map[string]any{ - "limit": map[string]any{"type": "integer", "description": "at most this many entries (default 20, at most 500)"}, - "max_bytes": map[string]any{"type": "integer", "description": "cut each entry's text at this many bytes; 0 leaves the text out (default 4096, at most 65536)"}, + "limit": map[string]any{"type": "integer", "description": "at most this many entries (default 20, at most 500)"}, }, Run: func(args map[string]any) (any, error) { limit, err := whole(args, "limit", 20, 1, 500) if err != nil { return nil, err } - most, err := whole(args, "max_bytes", 4096, 0, 65536) - if err != nil { - return nil, err - } - return History(limit, most) + return History(limit) }, }, { Name: "node-clipboard.copy", Description: "Put text on the operator's clipboard, as if they had copied it; it enters the history " + - "like any copy. Answers how many bytes.", + "like any copy. Reads the clipboard back and answers an error with the reason when it does not hold the text.", Input: map[string]any{ "type": "object", "properties": map[string]any{ @@ -61,7 +56,7 @@ func tools() []stdio.Tool { }, { Name: "clipmenu_paste", - Description: "What the operator's clipboard holds right now, as text (cut at 64 KiB, said in truncated).", + Description: "How many bytes of text the operator's clipboard holds right now, never the text itself.", Run: func(map[string]any) (any, error) { return Paste() }, }, { @@ -71,19 +66,22 @@ func tools() []stdio.Tool { }, { Name: "clipmenu_delete", - Description: "Forget one entry of the clipboard history, by its id as the history answers it, or by " + - "its first line exactly.", + Description: "Forget one entry of the clipboard history, by its place in node-clipboard.history's list " + + "(1 is the newest) and its time as the history gives it; refused when the two no longer match.", Input: map[string]any{ - "id": map[string]any{"type": "string", "description": "the entry's id"}, - "line": map[string]any{"type": "string", "description": "the entry's first line, exactly"}, + "place": map[string]any{"type": "integer", "description": "the entry's place in the history, 1 for the newest"}, + "at": map[string]any{"type": "string", "description": "the entry's time, as the history reports it"}, }, Run: func(args map[string]any) (any, error) { - id, err := text(args, "id", false) + place, err := whole(args, "place", 0, 0, 100000) if err != nil { return nil, err } - line, _ := args["line"].(string) - return Delete(id, line) + at, err := text(args, "at", true) + if err != nil { + return nil, err + } + return Delete(place, at) }, }, } diff --git a/modules/clipmenu/cmd/clipmenu-tools/manifest_test.go b/modules/clipmenu/cmd/clipmenu-tools/manifest_test.go index 3ab86569..265d199a 100644 --- a/modules/clipmenu/cmd/clipmenu-tools/manifest_test.go +++ b/modules/clipmenu/cmd/clipmenu-tools/manifest_test.go @@ -1,9 +1,14 @@ package main import ( + "context" + "net" + "os" + "os/exec" "reflect" "strings" "testing" + "time" ) // clipmenu's shape (novox/hq ADR 0208, research 026/04): it claims node-clipboard serving history @@ -27,20 +32,43 @@ func TestItClaimsTheClipboardSeatServingHistoryAndCopy(t *testing.T) { } } -func TestTheDaemonStartsOnceFromTheSessionsStart(t *testing.T) { +func TestTheDaemonIsTheOperatorsUserUnitEnabledAndRunning(t *testing.T) { m := readManifest(t) - if len(m.Shell) != 1 || m.Shell[0].For != "xinitrc" || m.Shell[0].Slot != "normal" { - t.Fatalf("%+v", m.Shell) + if len(m.Shell) != 0 { + t.Fatalf("a second start from the session: %+v", m.Shell) } - code := m.Shell[0].Code - // Started once, through the module's text-only xsel (a selection holding an image is never read). - if strings.Count(code, "clipmenud &\n") != 1 || !strings.Contains(code, `PATH="/usr/local/lib/mesh-clipmenu:$PATH" clipmenud &`) || - strings.Contains(code, "greenclip") { - t.Fatalf("%q", code) + svc := m.resource(t, "daemon") + want := map[string]any{"id": "daemon", "type": "service", "unit": "clipmenud.service", "scope": "user", + "user": "${machine:account}", "state": "running", "boot": "enabled", "reload-on": nil} + for k, v := range want { + if k != "reload-on" && svc[k] != v { + t.Fatalf("daemon %s = %v, want %v: %v", k, svc[k], v, svc) + } } - for _, r := range m.Resources { - if r["type"] == "service" || r["type"] == "process" { - t.Fatalf("a second start: %v", r) + dropin := m.resource(t, "daemon-dropin") + content, _ := dropin["content"].(string) + if dropin["path"] != "${machine:account-home}/.config/systemd/user/clipmenud.service.d/mesh.conf" || + dropin["owner"] != "${machine:account}" || + !strings.Contains(content, "ExecStart=/usr/local/lib/mesh-clipmenu/clipmenud-session") || + strings.Contains(content, "greenclip") { + t.Fatalf("drop-in: %v", dropin) + } + launcher, _ := m.resource(t, "launcher")["content"].(string) + if !strings.Contains(launcher, "DISPLAY") || !strings.Contains(launcher, "/tmp/.X11-unix") || + !strings.Contains(launcher, `PATH="/usr/local/lib/mesh-clipmenu:$PATH"`) || !strings.Contains(launcher, "exec clipmenud") { + t.Fatalf("launcher: %q", launcher) + } + // Waits for the display instead of exiting into the start limit; DISPLAY from the manager wins. + if !strings.Contains(launcher, "until [ -n \"$DISPLAY\" ]") || !strings.Contains(launcher, "sleep 1") || + strings.Contains(launcher, "exit 1") || strings.Contains(launcher, "exit 0") { + t.Fatalf("the launcher must wait, not exit: %q", launcher) + } + if !strings.Contains(content, "StartLimitIntervalSec=0") || !strings.Contains(content, "RestartSec=2s") { + t.Fatalf("drop-in lacks the safety net: %q", content) + } + for k, v := range m.Environment.Variables { + if !strings.Contains(content, "Environment="+k+"="+v) { + t.Fatalf("the unit lacks %s=%s", k, v) } } } @@ -64,3 +92,51 @@ func TestTheToolsAgreeWithTheManifest(t *testing.T) { checkTheToolsAgree(t, m) checkNoSecretsOrInstallationNames(t) } + +// Run for real: DISPLAY from the manager is used at once; several sockets and no DISPLAY wait. +func TestTheLauncherUsesTheManagersDisplayAndWaitsOtherwise(t *testing.T) { + m := readManifest(t) + script, _ := m.resource(t, "launcher")["content"].(string) + dir := t.TempDir() + file := dir + "/launcher" + // clipmenud and the socket directory are faked: the script's own paths are rewritten. + body := strings.ReplaceAll(script, "/tmp/.X11-unix", dir+"/x11") + body = strings.Replace(body, "exec clipmenud", `echo "DISPLAY=$DISPLAY"`, 1) + if err := os.WriteFile(file, []byte(body), 0o755); err != nil { + t.Fatal(err) + } + os.MkdirAll(dir+"/x11", 0o755) + fakeBin := dir + "/bin" + os.MkdirAll(fakeBin, 0o755) + // the manager's environment: empty until the file appears + os.WriteFile(fakeBin+"/systemctl", []byte("#!/bin/sh\n[ -f \""+dir+"/env\" ] && cat \""+dir+"/env\"\nexit 0\n"), 0o755) + run := func(env ...string) (string, bool) { + ctx, cancel := context.WithTimeout(context.Background(), 2500*time.Millisecond) + defer cancel() + cmd := exec.CommandContext(ctx, "sh", file) + cmd.Env = append([]string{"PATH=" + fakeBin + ":" + os.Getenv("PATH"), "HOME=" + dir}, env...) + out, _ := cmd.Output() + return strings.TrimSpace(string(out)), ctx.Err() == nil + } + if out, ok := run("DISPLAY=:7"); !ok || out != "DISPLAY=:7" { + t.Fatalf("with DISPLAY: %q %v", out, ok) + } + if _, ok := run(); ok { + t.Fatal("it exited with no X server") + } + for _, n := range []string{"X0", "X1"} { + l, _ := net.Listen("unix", dir+"/x11/"+n) + defer l.Close() + } + if _, ok := run(); ok { + t.Fatal("it chose one of two servers") + } + // the session hands the manager its display while the launcher waits + go func() { + time.Sleep(1200 * time.Millisecond) + os.WriteFile(dir+"/env", []byte("DISPLAY=:1\nOTHER=x\n"), 0o644) + }() + if out, ok := run(); !ok || out != "DISPLAY=:1" { + t.Fatalf("the display that arrived while waiting: %q %v", out, ok) + } +} diff --git a/modules/clipmenu/module.json b/modules/clipmenu/module.json index e157350c..ac1c6c9c 100644 --- a/modules/clipmenu/module.json +++ b/modules/clipmenu/module.json @@ -29,13 +29,6 @@ "CM_HISTLENGTH": "15" } }, - "shell": [ - { - "for": "xinitrc", - "slot": "normal", - "code": "# The clipboard's history (module clipmenu, novox/hq ADR 0208): clipmenud collects every copy from\n# here on, once per session. It keeps the history in the account's runtime directory, so a reboot\n# forgets it, and with it every password that was ever copied.\n# Through the module's xsel, which reads only a selection offering text (see the README).\nPATH=\"/usr/local/lib/mesh-clipmenu:$PATH\" clipmenud &\n" - } - ], "resources": [ { "id": "package", @@ -54,6 +47,44 @@ "path": "/usr/local/lib/mesh-clipmenu/xsel", "mode": "0755", "content": "#!/bin/sh\n# xsel as clipmenud sees it, written by the mesh (module clipmenu). Replaced at every push.\n#\n# clipmenud records text, and reads a selection with `timeout 1 xsel -o`. A selection holding an\n# image (a screenshot copied as image/png) is sent in pieces; one second is too short for megabytes,\n# timeout kills xsel half way, and the program owning the image waits for ever for a reader that is\n# gone. From then on nothing can ask the clipboard anything: pastes hang, and an Electron app that\n# asks on its main thread freezes. So a read goes ahead only when the selection offers text.\ncase \" $* \" in\n*\" -o \"* | *\" --output \"*)\n\tselection=clipboard\n\tcase \" $* \" in\n\t*\" --primary \"* | *\" -p \"*) selection=primary ;;\n\t*\" --secondary \"* | *\" -s \"*) selection=secondary ;;\n\tesac\n\ttargets=$(timeout 1 xclip -selection \"$selection\" -t TARGETS -o 2>/dev/null) || exit 1\n\tprintf '%s\\n' \"$targets\" | grep -qxE 'UTF8_STRING|STRING|TEXT|text/plain(;charset=utf-8)?' || exit 1\n\t;;\nesac\nexec /usr/bin/xsel \"$@\"\n" + }, + { + "id": "user-units-dir", + "type": "directory", + "path": "${machine:account-home}/.config/systemd/user", + "owner": "${machine:account}", + "mode": "0755" + }, + { + "id": "daemon-dropins", + "type": "directory", + "path": "${machine:account-home}/.config/systemd/user/clipmenud.service.d", + "owner": "${machine:account}", + "mode": "0755" + }, + { + "id": "daemon-dropin", + "type": "file", + "path": "${machine:account-home}/.config/systemd/user/clipmenud.service.d/mesh.conf", + "owner": "${machine:account}", + "mode": "0644", + "content": "# clipmenud in the operator's account (module clipmenu, novox/hq ADR 0208). Owned by the mesh: replaced at every push.\n# The package's unit starts clipmenud with no DISPLAY; the module's launcher finds the display and puts the\n# text-only xsel first on the PATH. The settings repeat the module's environment, which the account's\n# service manager does not read.\n[Unit]\nStartLimitIntervalSec=0\n\n[Service]\nRestartSec=2s\nEnvironment=CM_SELECTIONS=clipboard\nEnvironment=CM_MAX_CLIPS=500\nEnvironment=CM_HISTLENGTH=15\nExecStart=\nExecStart=/usr/local/lib/mesh-clipmenu/clipmenud-session\n" + }, + { + "id": "launcher", + "type": "file", + "path": "/usr/local/lib/mesh-clipmenu/clipmenud-session", + "mode": "0755", + "content": "#!/bin/sh\n# clipmenud as the operator's user unit starts it, written by the mesh (module clipmenu). Replaced at every push.\n#\n# The account's service manager starts before the X server does, and has a DISPLAY only once the session's\n# xinitrc has handed it over (module xorg). So this waits, quietly, until the display is known, and exits\n# only when the session ends with it: a quick exit would run the unit into its start limit. DISPLAY, when\n# the manager has it, wins. Without it, exactly one X socket names the display; none or several (a\n# second server beside the session) wait for the session's own to arrive.\n# A running process does not see what the session hands the manager later, so each pass asks the manager.\nuntil [ -n \"$DISPLAY\" ]; do\n\tfor pair in $(systemctl --user show-environment 2>/dev/null | grep -E '^(DISPLAY|XAUTHORITY)='); do\n\t\texport \"$pair\"\n\tdone\n\t[ -n \"$DISPLAY\" ] && break\n\tset -- /tmp/.X11-unix/X*\n\tif [ $# -eq 1 ] && [ -S \"$1\" ]; then\n\t\tDISPLAY=\":${1##*/X}\"\n\t\texport DISPLAY\n\telse\n\t\tsleep 1\n\tfi\ndone\nif [ -z \"$XAUTHORITY\" ] && [ -r \"$HOME/.Xauthority\" ]; then\n\tXAUTHORITY=\"$HOME/.Xauthority\"\n\texport XAUTHORITY\nfi\nPATH=\"/usr/local/lib/mesh-clipmenu:$PATH\"\nexport PATH\nexec clipmenud\n" + }, + { + "id": "daemon", + "type": "service", + "unit": "clipmenud.service", + "scope": "user", + "user": "${machine:account}", + "state": "running", + "boot": "enabled" } ], "build": { From c070bea782aa725be4b25963d144d57122dfb5b5 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 22:59:27 +0200 Subject: [PATCH 2/2] Declare how the clipboard daemon is ready, so the count of resources without health stays at 62 (ADR 0240 rule 8, issues 743, 750) --- modules/clipmenu/cmd/clipmenu-tools/manifest_test.go | 5 +++++ modules/clipmenu/module.json | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/modules/clipmenu/cmd/clipmenu-tools/manifest_test.go b/modules/clipmenu/cmd/clipmenu-tools/manifest_test.go index 265d199a..22378f46 100644 --- a/modules/clipmenu/cmd/clipmenu-tools/manifest_test.go +++ b/modules/clipmenu/cmd/clipmenu-tools/manifest_test.go @@ -45,6 +45,11 @@ func TestTheDaemonIsTheOperatorsUserUnitEnabledAndRunning(t *testing.T) { t.Fatalf("daemon %s = %v, want %v: %v", k, svc[k], v, svc) } } + // ADR 0240 rule 8: a long-running resource says how it is ready. The launcher keeps the unit active while + // it waits for the display and execs clipmenud, so the unit's state is the daemon's own liveness. + if h, _ := svc["health"].(map[string]any); h["kind"] != "unit" { + t.Fatalf("daemon health = %v, want kind unit", svc["health"]) + } dropin := m.resource(t, "daemon-dropin") content, _ := dropin["content"].(string) if dropin["path"] != "${machine:account-home}/.config/systemd/user/clipmenud.service.d/mesh.conf" || diff --git a/modules/clipmenu/module.json b/modules/clipmenu/module.json index ac1c6c9c..c62950c2 100644 --- a/modules/clipmenu/module.json +++ b/modules/clipmenu/module.json @@ -84,7 +84,10 @@ "scope": "user", "user": "${machine:account}", "state": "running", - "boot": "enabled" + "boot": "enabled", + "health": { + "kind": "unit" + } } ], "build": {