From 50e32f24dfb0645249408fdfba3aeb174205c5c4 Mon Sep 17 00:00:00 2001 From: Storm Dragon Date: Wed, 21 Oct 2026 15:11:19 -0400 Subject: [PATCH] Logging changes. Use --debug-file for a replaced log. Use --append to append to the log file if it exists. Also fixed a bug where cthulhu could try to restart without replacing the old running version. --- CONTRIBUTING.md | 12 +++- README.md | 5 ++ cmd/cthulhu/main.go | 8 ++- cmd/cthulhu/main_test.go | 30 +++++++++- cmd/cthulhu/replace.go | 5 +- cmd/cthulhu/replace_test.go | 8 ++- .../Arch-Linux/cthulhu-git/PKGBUILD | 2 +- internal/logging/async.go | 10 +++- internal/logging/async_test.go | 58 ++++++++++++++++++- 9 files changed, 125 insertions(+), 13 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 67cd00c..068d13d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -96,11 +96,19 @@ in the same desktop session: "$HOME/.local/bin/cthulhu" --replace ``` -The replacement command finds the running reader through its session D-Bus -service. A terminal using a different session bus can start another reader +The replacement command uses the same session-bus discovery as the reader's +D-Bus service, including when `DBUS_SESSION_BUS_ADDRESS` is unset. It waits +for the identified reader process to exit before starting the replacement. +A terminal using a different session bus can start another reader instead of replacing the intended one. Preserve the original reader's desktop session environment when restarting it from automation. +Check replacement and session-bus discovery without touching the desktop reader: + +```sh +python3 tests/native_test_session.py env CTHULHU_NATIVE_REPLACE=1 CTHULHU_NATIVE_DBUS_TEST=1 go test ./cmd/cthulhu ./internal/sessionbus -run 'TestReplace|TestNativeDesktopSessionBusDiscovery' -count=1 -v +``` + Wine accessibility also needs the separate standalone `wine-access` installation described in the [README](README.md#wine-access). Building the Go reader does not build or refresh that helper. After restoring an overwritten diff --git a/README.md b/README.md index fa57e96..1d5f442 100644 --- a/README.md +++ b/README.md @@ -187,6 +187,11 @@ update immediately. Moving focus, navigating, changing tabs or windows, and locking the session discard pending announcements. Live regions retain their existing announcement behavior. +Debug logging with `--debug-file PATH` replaces any existing log at that path. +Add `--append` to preserve earlier sessions, for example +`cthulhu --debug-file /tmp/cthulhu.log --append`. +`--append` requires `--debug-file`. A log can still grow during a long session. + For browser bug reports, start Cthulhu with `--debug-file /tmp/cthulhu-browser.log` and reproduce the affected navigation. The startup record includes the release version, embedded Git revision and modified-build status, Go and AT-SPI versions, diff --git a/cmd/cthulhu/main.go b/cmd/cthulhu/main.go index 543a0a3..9712092 100644 --- a/cmd/cthulhu/main.go +++ b/cmd/cthulhu/main.go @@ -78,7 +78,8 @@ func run(ctx context.Context, arguments []string, stdout, stderr io.Writer) (res clipboardWatch := options.Bool("clipboard-watch", false, "run the internal clipboard change helper (private pipe protocol)") ocrCapture := options.Bool("ocr-capture", false, "run the internal OCR capture helper (private pipe protocol)") ocrConfirm := options.Bool("ocr-confirm", false, "run the internal OCR click confirmation helper") - debugFile := options.String("debug-file", "", "enable debug logging and append logs only to this file") + appendLog := options.Bool("append", false, "preserve existing debug log contents (requires --debug-file)") + debugFile := options.String("debug-file", "", "enable debug logging only to this file, replacing existing contents") speechSocket := options.String( "speech-socket", "", @@ -123,6 +124,9 @@ func run(ctx context.Context, arguments []string, stdout, stderr io.Writer) (res if options.NArg() != 0 { return fmt.Errorf("unexpected argument %q", options.Arg(0)) } + if *appendLog && *debugFile == "" { + return errors.New("--append requires --debug-file") + } if *showVersion || *shortVersion { fmt.Fprintln(stdout, version.Details( atspi.Version(), @@ -174,7 +178,7 @@ func run(ctx context.Context, arguments []string, stdout, stderr io.Writer) (res } logger := slog.New(slog.DiscardHandler) if *debugFile != "" { - fileWriter, err := logging.OpenFile(*debugFile, nil) + fileWriter, err := logging.OpenFile(*debugFile, nil, *appendLog) if err != nil { return fmt.Errorf("open debug log: %w", err) } diff --git a/cmd/cthulhu/main_test.go b/cmd/cthulhu/main_test.go index e1012cb..215f84b 100644 --- a/cmd/cthulhu/main_test.go +++ b/cmd/cthulhu/main_test.go @@ -146,7 +146,7 @@ func TestDebugFileCapturesStartupFailureAndAppends(t *testing.T) { for range 2 { var stdout, stderr bytes.Buffer err := run(context.Background(), []string{ - "--debug-file", logPath, + "--debug-file", logPath, "--append", "--config", configPath, "--speech-socket", filepath.Join(directory, "absent.sock"), }, &stdout, &stderr) @@ -244,3 +244,31 @@ func TestStandaloneDebugFlagRejected(t *testing.T) { t.Fatalf("standalone --debug error = %v", err) } } + +func TestAppendRequiresDebugFile(t *testing.T) { + var output bytes.Buffer + err := run(context.Background(), []string{"--append"}, &output, &output) + if err == nil || err.Error() != "--append requires --debug-file" { + t.Fatalf("error = %v", err) + } +} + +func TestDebugFileStartsFresh(t *testing.T) { + logPath := filepath.Join(t.TempDir(), "debug.log") + configPath := filepath.Join(t.TempDir(), "absent.toml") + for range 2 { + var output bytes.Buffer + if err := run(context.Background(), []string{ + "--check-config", "--config", configPath, "--debug-file", logPath, + }, &output, &output); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(logPath) + if err != nil { + t.Fatal(err) + } + if strings.Count(string(data), `"message":"starting Cthulhu"`) != 1 { + t.Fatalf("expected only the current session: %s", data) + } + } +} diff --git a/cmd/cthulhu/replace.go b/cmd/cthulhu/replace.go index 8f33ebe..3ce8e9a 100644 --- a/cmd/cthulhu/replace.go +++ b/cmd/cthulhu/replace.go @@ -15,6 +15,7 @@ import ( "time" "git.stormux.org/storm/cthulhu/internal/remote" + "git.stormux.org/storm/cthulhu/internal/sessionbus" "github.com/godbus/dbus/v5" ) @@ -23,7 +24,9 @@ const replaceWait = 8 * time.Second // replaceRunning identifies the active Go reader through its private session // bus name. It never signals a process merely because its name matches. func replaceRunning(ctx context.Context) error { - connection, err := dbus.ConnectSessionBus() + // Match the reader's service discovery without rewriting the environment. + // Other discovery paths can select another bus when the address is unset. + connection, err := sessionbus.Connect(nil, "replacement") if err != nil { return fmt.Errorf("connect to session bus: %w", err) } diff --git a/cmd/cthulhu/replace_test.go b/cmd/cthulhu/replace_test.go index d6bd84a..2824c28 100644 --- a/cmd/cthulhu/replace_test.go +++ b/cmd/cthulhu/replace_test.go @@ -16,6 +16,7 @@ import ( "time" "git.stormux.org/storm/cthulhu/internal/remote" + "git.stormux.org/storm/cthulhu/internal/sessionbus" "github.com/godbus/dbus/v5" ) @@ -23,7 +24,7 @@ func TestReplaceHelper(t *testing.T) { if os.Getenv("CTHULHU_REPLACE_HELPER") != "1" { return } - connection, err := dbus.ConnectSessionBus() + connection, err := sessionbus.Connect(nil, "replacement-test-reader") if err != nil { t.Fatal(err) } @@ -32,10 +33,10 @@ func TestReplaceHelper(t *testing.T) { if err != nil || reply != dbus.RequestNameReplyPrimaryOwner { t.Fatalf("own reader bus name: %v, %v", reply, err) } - fmt.Println("READY") signals := make(chan os.Signal, 1) signal.Notify(signals, syscall.SIGTERM) defer signal.Stop(signals) + fmt.Println("READY") <-signals } @@ -86,6 +87,9 @@ func TestReplaceRunningNative(t *testing.T) { if err := replaceRunning(ctx); err != nil { t.Fatal(err) } + if state, _, err := processIdentity(command.Process.Pid); !os.IsNotExist(err) && (err != nil || state != "Z") { + t.Fatalf("replacement returned before reader exit: state=%q, error=%v", state, err) + } if err := command.Wait(); err != nil { t.Fatalf("running reader did not exit cleanly: %v", err) } diff --git a/distro-packages/Arch-Linux/cthulhu-git/PKGBUILD b/distro-packages/Arch-Linux/cthulhu-git/PKGBUILD index 0c2bc91..ecc67e9 100644 --- a/distro-packages/Arch-Linux/cthulhu-git/PKGBUILD +++ b/distro-packages/Arch-Linux/cthulhu-git/PKGBUILD @@ -4,7 +4,7 @@ pkgbase=cthulhu-git pkgname=cthulhu-git _pkgname=cthulhu -pkgver=2026.10.15.r26.g4be35fc +pkgver=2026.10.20.r61.g4643f8e pkgrel=1 pkgdesc="Desktop-agnostic screen reader with speech, Braille, and Tentacle support" url="https://git.stormux.org/storm/cthulhu" diff --git a/internal/logging/async.go b/internal/logging/async.go index ebc63a9..c982fe7 100644 --- a/internal/logging/async.go +++ b/internal/logging/async.go @@ -359,11 +359,17 @@ type fileWriter struct { closeOnce sync.Once } -func OpenFile(path string, fallback io.Writer) (DiagnosticsWriter, error) { +func OpenFile(path string, fallback io.Writer, appendLog bool) (DiagnosticsWriter, error) { if path == "" || path == "." || path == ".." { return nil, errors.New("invalid diagnostics file path") } - file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o600) + // Keep writes at the end even after truncation: an outgoing reader may + // still append its shutdown records while --replace is in progress. + flags := os.O_WRONLY | os.O_CREATE | os.O_APPEND + if !appendLog { + flags |= os.O_TRUNC + } + file, err := os.OpenFile(path, flags, 0o600) if err != nil { return nil, fmt.Errorf("open diagnostics file: %w", err) } diff --git a/internal/logging/async_test.go b/internal/logging/async_test.go index fcbab77..2a75b61 100644 --- a/internal/logging/async_test.go +++ b/internal/logging/async_test.go @@ -294,7 +294,7 @@ func TestAsyncRepeatedStopAfterSuccess(t *testing.T) { func TestOpenFileIsAppendOnlyOwnerRestrictedAndCancellable(t *testing.T) { path := filepath.Join(t.TempDir(), "diagnostics.log") - writer, err := OpenFile(path, nil) + writer, err := OpenFile(path, nil, true) if err != nil { t.Fatalf("OpenFile: %v", err) } @@ -312,7 +312,7 @@ func TestOpenFileIsAppendOnlyOwnerRestrictedAndCancellable(t *testing.T) { if info.Mode().Perm() != 0o600 { t.Fatalf("file mode = %o, want 0600", info.Mode().Perm()) } - writer, err = OpenFile(path, nil) + writer, err = OpenFile(path, nil, true) if err != nil { t.Fatalf("reopen: %v", err) } @@ -332,3 +332,57 @@ func TestOpenFileIsAppendOnlyOwnerRestrictedAndCancellable(t *testing.T) { } var _ io.Writer = (*controlledWriter)(nil) + +func TestOpenFileReplacesPreviousContents(t *testing.T) { + path := filepath.Join(t.TempDir(), "diagnostics.log") + if err := os.WriteFile(path, []byte("previous session with longer contents\n"), 0o600); err != nil { + t.Fatal(err) + } + writer, err := OpenFile(path, nil, false) + if err != nil { + t.Fatal(err) + } + if _, err := writer.Write([]byte("new session\n")); err != nil { + t.Fatal(err) + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(data) != "new session\n" { + t.Fatalf("file contents = %q", data) + } +} + +func TestFreshLogPreservesWritesFromOutgoingReader(t *testing.T) { + path := filepath.Join(t.TempDir(), "diagnostics.log") + old, err := OpenFile(path, nil, true) + if err != nil { + t.Fatal(err) + } + defer old.Close() + if _, err := old.Write([]byte("old session\n")); err != nil { + t.Fatal(err) + } + next, err := OpenFile(path, nil, false) + if err != nil { + t.Fatal(err) + } + defer next.Close() + if _, err := old.Write([]byte("outgoing shutdown\n")); err != nil { + t.Fatal(err) + } + if _, err := next.Write([]byte("new session\n")); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(data) != "outgoing shutdown\nnew session\n" { + t.Fatalf("overlapping reader writes = %q", data) + } +}