|
@@ -0,0 +1,320 @@
|
|
|
|
|
+# Compliance Review -- discordo-plus
|
|
|
|
|
+
|
|
|
|
|
+Reviewed: 2026-03-20 | Reviewer: Claude (automated)
|
|
|
|
|
+Scope: all source files | Standard: General (Code Quality, Performance, Go Best Practices)
|
|
|
|
|
+Reference: `research/TECHFILE.md` for dependency and version context.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+## CRITICAL
|
|
|
|
|
+
|
|
|
|
|
+### 1. ✅ FIXED — Cache.Get Panics on Missing Key
|
|
|
|
|
+`internal/cache/cache.go:27` -- The `Get` method performs an unchecked type assertion `i.(uint)` on the result of `sync.Map.Load`. If the key does not exist, `i` is `nil` and the assertion panics. While `Exists` is called before `Get` in the current codebase (`message_input.go:499-500`), there is no guarantee callers will always check first, and the race window between `Exists` and `Get` on a `sync.Map` means concurrent `Invalidate` could delete the key between the two calls, causing a runtime panic.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Change `Get` to return `(uint, bool)` or use a comma-ok assertion: `if i, ok := c.items.Load(query); ok { return i.(uint) }; return 0`. Update callers to handle the zero/missing case.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. Only `message_input.go` calls `Get`, and the change is a straightforward API update. The fix also closes a potential TOCTOU race.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 2. ✅ FIXED — HTTP Response Body Leak in Brotli Transport
|
|
|
|
|
+`internal/http/transport.go:28-32` -- When the response has `Content-Encoding: br`, the original `resp.Body` is wrapped with `brotli.NewReader` via `io.NopCloser`. This means calling `Close()` on the new body is a no-op and the original body's connection is never returned to the pool. For long-running applications like a Discord client, this can exhaust available connections over time.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Replace `io.NopCloser(brotli.NewReader(resp.Body))` with a custom `ReadCloser` that wraps the brotli reader but delegates `Close()` to the original `resp.Body`. Example: `type brotliReadCloser struct { io.Reader; closer io.Closer }` with `func (b *brotliReadCloser) Close() error { return b.closer.Close() }`.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. The fix is a small wrapper struct. Verify with a test that the underlying body is closed when the brotli-wrapped response body is closed.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 3. ✅ FIXED — Notification Avatar Download Uses Bare http.Get Without Timeout
|
|
|
|
|
+`internal/notifications/notifications.go:93` -- `http.Get(url)` uses the default HTTP client which has no timeout. A slow or unresponsive Discord CDN server will block the goroutine indefinitely. Since notifications are triggered from gateway events, this could accumulate blocked goroutines during network issues.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Use `http.Client{Timeout: 10 * time.Second}` or create a request with `context.WithTimeout` and pass it to `http.DefaultClient.Do(req)`. Apply the same fix to `downloadToCache` in `messages_list.go:1191` which also uses bare `http.Get`.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. Adding a timeout may cause some avatar downloads to fail under poor network conditions, but this is preferable to indefinite blocking. Existing error handling already covers the failure path.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 4. ✅ FIXED — Log File Opened With os.ModePerm (0777)
|
|
|
|
|
+`internal/logger/logger.go:24` -- The log file is opened with `os.ModePerm` (0777), meaning it is world-readable and world-writable. Log files may contain sensitive debugging information (user IDs, channel names, error messages). Similarly, `internal/consts/consts.go:25` creates the cache directory with `os.ModePerm`.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Use `0600` for the log file (owner read/write only) and `0700` for the cache directory (owner only). Apply the same change to `guildstate.go:45` which writes state with `0644` (already better but could be `0600` for consistency).
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. Users running in shared environments benefit from restrictive permissions. Existing users may need to manually fix permissions on existing files if they switch.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+## CODE QUALITY
|
|
|
|
|
+
|
|
|
|
|
+### 5. messages_list.go Is Over 1600 Lines -- God File
|
|
|
|
|
+`internal/ui/chat/messages_list.go` -- This file contains message rendering, embed processing, URL extraction, attachment downloading, file I/O, external process management (image viewer, xdotool), keybind handling, selection logic, and help text generation. At 1600+ lines, it violates single-responsibility and is the hardest file for a new developer to navigate.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Extract into focused files: (a) `embed_renderer.go` for `embedLines`, `embedLineStyles`, `wrapStyledLine`, `lineWithURL`, `linkDisplayText`, `unescapeMarkdownEscapes`; (b) `attachment_handler.go` for `downloadToCache`, `openAttachment`, `saveAttachmentImage`, `openURL`, `viewerArgs`, `terminalGeometry`, `supportedImageTypes`; (c) `url_extractor.go` for `extractURLs`, `extractEmbedURLs`, `messageURLs`. Each extraction reduces this file by ~200-300 lines.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Medium. All extracted functions are file-private (`lowercase`), so no external API changes. However, the functions access `messagesList` fields through receiver methods, so some will need to become methods on `messagesList` or take explicit parameters. Test with `go build ./...` after each extraction.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 6. ✅ FIXED — Duplicated ShortHelp/FullHelp Logic in guilds_tree.go
|
|
|
|
|
+`internal/ui/chat/guilds_tree.go:67-132` -- The `ShortHelp()` and `FullHelp()` methods contain nearly identical logic for determining the `selectDesc` string based on node state (lines 69-86 duplicated at 100-117). This 18-line block is copy-pasted.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Extract a helper method `func (gt *guildsTree) selectActionDesc() string` that returns "collapse", "expand", or the default description based on node state. Call it from both `ShortHelp` and `FullHelp`.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. Pure refactoring of display logic with no behavioral change.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 7. ✅ FIXED — Duplicated attachmentsPicker Layer Setup
|
|
|
|
|
+`internal/ui/chat/messages_list.go:1178-1187` and `internal/ui/chat/messages_list.go:1318-1327` -- The code to show the attachments picker overlay is identical in `showAttachmentsList` and `saveImage`: create centered layer, add with overlay options, send to front, set focus. This 10-line pattern is duplicated verbatim.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Extract a method `func (ml *messagesList) showAttachmentsOverlay()` that handles the layer setup and focus. Call it from both `showAttachmentsList` and `saveImage` after setting items.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. Simple extraction of identical code.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 8. ✅ FIXED — Inconsistent Error Handling: Some Paths Silently Ignore Errors
|
|
|
|
|
+`internal/ui/chat/messages_list.go:1409` -- `me, _ := ml.chatView.state.Cabinet.Me()` discards the error. If the state is not ready, `me` is nil and `me.ID` will panic on the next line. This pattern appears in multiple places: `state.go:195`, `messages_list.go:1446`, `messages_list.go:1539`, `messages_list.go:1563`.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Handle the error case: `me, err := ml.chatView.state.Cabinet.Me(); if err != nil { slog.Error(...); return }`. Apply to all instances where `Me()` is called with a discarded error.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. Adding error checks prevents nil-pointer panics. The `Me()` call should always succeed after login, but defensive coding prevents crashes during edge-case race conditions (e.g., state reconnection).
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 9. ✅ FIXED — Magic Number: embedLineStyles Uses Hardcoded Array Size [8]
|
|
|
|
|
+`internal/ui/chat/messages_list.go:670` -- `[8]tcell.Style{}` is a magic number that must manually stay in sync with the number of `embedLineKind` constants (lines 660-668). Adding a new embed line kind without updating the array size causes a silent bug (zero-value style).
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Define a sentinel `const embedLineKindCount = 8` after the last `iota` constant, or use a slice. Example: add `embedLineKindCount` as the last iota entry and use `[embedLineKindCount]tcell.Style{}`.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. Compile-time enforcement prevents future mismatches.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 10. ✅ FIXED — guildState Uses Mutex Where RWMutex Would Be Appropriate
|
|
|
|
|
+`internal/ui/chat/guildstate.go:14-17` -- `guildState` uses `sync.Mutex` for all access, but `isExpanded` (line 61) is a read-only operation that could use `RLock`. Guild state is read frequently (every node creation during `onReady`) and written infrequently (user expand/collapse action).
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Change `mu sync.Mutex` to `mu sync.RWMutex` and use `gs.mu.RLock()`/`gs.mu.RUnlock()` in `isExpanded`.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. Standard Go concurrency pattern upgrade.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 11. Inconsistent Log Level Usage
|
|
|
|
|
+Multiple files -- Errors that represent user-facing failures are logged at different levels without consistency: `slog.Info` for failed keyring retrieval (`root/events.go:29`), `slog.Info` for failed avatar cache (`notifications.go:65`), `slog.Error` for failed presence lookup (`message_input.go:571-572`) which is actually a normal condition (presence not always available). The inconsistency makes log filtering unreliable.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Establish a convention: `slog.Error` for failures that affect functionality (failed API calls, state corruption), `slog.Warn` for degraded-but-functional conditions (missing avatar, unknown presence), `slog.Info` for expected alternative paths (keyring not found, config file missing). Apply consistently across all files.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. Log level changes have no functional impact but significantly improve operational debugging.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 12. ✅ FIXED — Invalid Log Level Silently Defaults to Zero Value
|
|
|
|
|
+`cmd/root.go:29-39` -- The `switch` on `logLevel` has no `default` case. An invalid value like `--log-level=trace` silently results in `slog.Level(0)` which is `slog.LevelInfo`. The user gets no feedback that their log level was invalid.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Add a `default` case that either returns an error (`return fmt.Errorf("unknown log level: %q", logLevel)`) or logs a warning and falls back to info.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. Adds user feedback for misconfiguration.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+## DOCUMENTATION GAPS
|
|
|
|
|
+
|
|
|
|
|
+### 13. No File-Level Comments on Most Source Files
|
|
|
|
|
+All `.go` files except `internal/cache/cache.go` lack a package-level doc comment explaining the file's purpose. For a new developer, understanding what `state.go`, `events.go`, `keybinds.go`, and `util.go` contain requires reading them entirely. The `internal/ui/chat/` package is particularly challenging with 10+ files and no file-level guidance.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 14. No Documentation on the Event/Command Architecture
|
|
|
|
|
+`internal/ui/chat/model.go`, `internal/ui/chat/events.go`, `internal/ui/root/events.go` -- The application uses a custom event-driven architecture (tview.Event/tview.Command pattern) that is central to understanding the codebase. There is no documentation explaining: (a) how events flow from gateway to UI, (b) the Command pattern (returning closures from handlers), (c) the `listen()` loop pattern, or (d) how `tview.Batch` composes commands. A new developer cannot understand the control flow without reverse-engineering it.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 15. No Documentation on the Rendering Pipeline
|
|
|
|
|
+`internal/markdown/renderer.go`, `internal/ui/chat/messages_list.go` -- The message rendering pipeline (Discord message -> goldmark AST -> tview.LineBuilder -> tview.Line with styled segments) is complex and undocumented. Key concepts like `styleStack`, `linkDepth`, `MergeStyle`, embed rendering with wrapping, and OSC 8 URL metadata have no explanatory comments.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 16. Config Validation Rules Undocumented
|
|
|
|
|
+`internal/config/config.go:102-103` -- `AutocompleteLimit` and `MessagesLimit` are `uint8` types. The config.toml comment says "minimum and maximum value is 1 and 100" for `messages_limit`, but this constraint is not enforced in code. A user setting `messages_limit = 255` would silently exceed the Discord API limit. The `uint8` type provides implicit 0-255 range but the TOML comment claims 1-100.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+## PERFORMANCE
|
|
|
|
|
+
|
|
|
|
|
+### 17. Unbounded itemByID Cache in messagesList
|
|
|
|
|
+`internal/ui/chat/messages_list.go:58` -- `itemByID map[discord.MessageID]*tview.TextView` caches rendered message TextViews and is only cleared on channel switch (`reset()`) or when messages are modified. For channels with heavy traffic, this map grows without bound as new messages arrive. Each `tview.TextView` holds rendered line data, styles, and a text buffer. Over a long session in a busy channel, this can accumulate thousands of entries.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Add a size cap. When `len(itemByID) > maxCacheSize` (e.g., 500), evict entries for message IDs no longer in the current `messages` slice. Alternatively, clear the cache when it exceeds a threshold during `addMessage`.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low-medium. Cache eviction during message additions may cause brief re-rendering of older messages if the user scrolls up. The visual impact is negligible since re-rendering is the fallback path.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 18. ✅ FIXED — collapseParentNode Uses O(n) Tree Walk
|
|
|
|
|
+`internal/ui/chat/guilds_tree.go:380-395` -- `collapseParentNode` walks the entire tree from root to find the parent of the given node, even though `GetPath(node)` (used in `canCollapseParent`) returns the full path including the parent. The O(n) walk runs on every `-` keypress.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Use `gt.GetPath(node)` to get the parent directly: `path := gt.GetPath(node); if len(path) >= 3 { parent := path[len(path)-2]; ... }`. This is O(depth) instead of O(total nodes).
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. `GetPath` is already used in `canCollapseParent` and `expandPathToNode`, so it is a known-reliable method. Verify the path includes the root node as the first element.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 19. Repeated Full-Tree GetPath Calls in canCollapseParent
|
|
|
|
|
+`internal/ui/chat/guilds_tree.go:134-145` -- `canCollapseParent` is called from both `ShortHelp()` and `FullHelp()`, which run on every draw cycle to update the help bar. Each call invokes `gt.GetPath(node)` which walks the tree. Combined with `ShortHelp` and `FullHelp` duplicating the node-state checks, the help bar triggers 2+ tree walks per render frame.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Cache the path result for the current node, or compute `canCollapseParent` once and store it. Since the current node only changes on explicit user navigation, the cached value remains valid between navigation events.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. Cache invalidation happens naturally when `SetCurrentNode` is called.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 20. extractURLs Parses Message Content Twice
|
|
|
|
|
+`internal/ui/chat/messages_list.go:1094-1115` and `internal/ui/chat/messages_list.go:511-519` -- When rendering a message with embeds, `drawEmbeds` calls `extractURLs(message.Content)` to build a dedup set, and then `drawContent` also parses the same content through `discordmd.ParseWithMessage` for markdown rendering. The goldmark parser is invoked twice on the same content. For messages with complex markdown, this is measurable.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Parse content once and pass both the rendered lines and the extracted URLs to the embed renderer. This requires restructuring `drawDefaultMessage` to call a combined parse+render function that returns both outputs.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Medium. The rendering pipeline currently separates content rendering from embed rendering. Merging them requires careful handling of the `forceMarkdown` flag used in embed descriptions.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+## INFRASTRUCTURE
|
|
|
|
|
+
|
|
|
|
|
+### 21. ✅ FIXED — CI Uses go-version: stable -- Not Pinned
|
|
|
|
|
+`.github/workflows/ci.yml:25` -- As documented in `research/TECHFILE.md` (finding #5), the CI uses `go-version: stable` which auto-advances when Go releases a new version. This can introduce unexpected breakage from new vet rules or behavior changes.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Pin to `go-version: "1.26"` or `"1.26.x"`. Update intentionally when upgrading.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** None from pinning. The risk is the current unpinned behavior.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 22. No Linter in CI
|
|
|
|
|
+`.github/workflows/ci.yml` -- The CI pipeline only runs `go test` and `go build`. There is no linter (`golangci-lint`, `staticcheck`, or `go vet` beyond what `go build` implicitly checks). Issues like unused variables, shadowed errors, and inefficient patterns are not caught in CI.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Add a lint step using `golangci-lint` with a `.golangci.yml` config. Start with default linters (`vet`, `errcheck`, `staticcheck`, `unused`) and gradually enable more.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low for adding the step. Initial lint run may produce many findings that need to be triaged. Use `--new-from-rev=HEAD~1` for incremental enforcement during transition.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### 23. ✅ FIXED — CI Artifact Name Does Not Match Binary Name
|
|
|
|
|
+`.github/workflows/ci.yml:41-43` -- CI uploads artifact named `discordo_$OS_$ARCH` but the binary is `discordo` (or `discordo.exe`). The project README and CLAUDE.md refer to the binary as `discordo-plus`. There is a naming inconsistency between the upstream module path (`github.com/ayn2op/discordo`), the CI artifact name, and the intended installation name.
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Update the build step to `go build -trimpath -ldflags=-s -o discordo-plus .` (or `discordo-plus.exe` on Windows) to match the documented binary name. Update the artifact upload to reference the correct file.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** Low. Downstream users of CI artifacts will see the renamed binary. Scoop/AUR packages may need path updates.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+## TEST COVERAGE
|
|
|
|
|
+
|
|
|
|
|
+### 24. Only 2 Packages Have Tests -- Minimal Coverage
|
|
|
|
|
+`internal/config/config_test.go`, `internal/keyring/keyring_test.go` -- Out of 12 internal packages, only 2 have any tests. Zero test coverage for: markdown rendering, clipboard operations, HTTP transport, notifications, cache, guild state persistence, UI components, embed processing, URL extraction, and the entire event handling pipeline.
|
|
|
|
|
+
|
|
|
|
|
+Critical untested code paths:
|
|
|
|
|
+- `cache.go:Get` -- can panic (finding #1)
|
|
|
|
|
+- `transport.go` -- body leak (finding #2)
|
|
|
|
|
+- `guildstate.go` -- JSON marshal/unmarshal roundtrip
|
|
|
|
|
+- `messages_list.go` -- `extractURLs`, `messageURLs`, `wrapStyledLine`, `unescapeMarkdownEscapes`, `linkDisplayText`, `sameLocalDate` are all pure functions ideal for unit testing
|
|
|
|
|
+- `config/theme.go` -- TOML unmarshaling for all wrapper types
|
|
|
|
|
+- `util.go` -- `MergeStyle`, `SortGuildChannels`, `ChannelToString`
|
|
|
|
|
+
|
|
|
|
|
+**Fix:** Prioritize tests for: (a) pure functions in messages_list.go (URL extraction, wrapping, escaping), (b) cache.go roundtrip and race conditions, (c) guildstate.go persistence roundtrip, (d) theme.go unmarshaling edge cases. These require no mocking and have clear input/output contracts.
|
|
|
|
|
+
|
|
|
|
|
+**Implementation Risk:** None. Tests are additive.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+## POSITIVES
|
|
|
|
|
+
|
|
|
|
|
+- Go 1.26 module minimum is the latest stable release; dependency versions are current (see TECHFILE.md).
|
|
|
|
|
+- Error wrapping with `%w` is used consistently in config loading, HTTP transport, and attachment handling.
|
|
|
|
|
+- The event/command architecture cleanly separates I/O from UI updates; all gateway operations run in commands (closures), keeping the UI thread responsive.
|
|
|
|
|
+- Config defaults are embedded via `//go:embed`, eliminating runtime file-not-found failures.
|
|
|
|
|
+- Token storage uses the OS keyring (zalando/go-keyring) rather than plaintext files.
|
|
|
|
|
+- Guild state persistence uses proper mutex synchronization around both reads and writes.
|
|
|
|
|
+- The typing indicator implementation correctly uses `time.AfterFunc` with cleanup, preventing timer leaks.
|
|
|
|
|
+- `selectedChannelMu` RWMutex correctly protects the selected channel across goroutines.
|
|
|
|
|
+- The markdown renderer's style stack pattern is clean and handles nested formatting correctly.
|
|
|
|
|
+- The embed deduplication logic (`embedLineDedupKey`) prevents duplicate content across embed fields.
|
|
|
|
|
+- Platform-specific code uses Go's filename-based build tag convention cleanly (clipboard, editor, suspend).
|
|
|
|
|
+- The `ConfigurePicker` helper in `util.go` avoids duplicating picker setup across three picker types.
|
|
|
|
|
+- `wrapStyledLine` correctly uses grapheme cluster width via `uniseg` for accurate wrapping with wide glyphs and emoji.
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+## Summary
|
|
|
|
|
+
|
|
|
|
|
+| Severity | Total |
|
|
|
|
|
+|----------|-------|
|
|
|
|
|
+| Critical | 4 |
|
|
|
|
|
+| Code Quality | 8 |
|
|
|
|
|
+| Documentation Gaps | 4 |
|
|
|
|
|
+| Performance | 4 |
|
|
|
|
|
+| Infrastructure | 3 |
|
|
|
|
|
+| Test Coverage | 1 |
|
|
|
|
|
+| Compliance | N/A |
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+## Files Reviewed
|
|
|
|
|
+
|
|
|
|
|
+| # | File | Type |
|
|
|
|
|
+|---|---|---|
|
|
|
|
|
+| 1 | `main.go` | Entry point |
|
|
|
|
|
+| 2 | `cmd/root.go` | CLI / app init |
|
|
|
|
|
+| 3 | `internal/config/config.go` | Config struct + loader |
|
|
|
|
|
+| 4 | `internal/config/config.toml` | Embedded default config |
|
|
|
|
|
+| 5 | `internal/config/config_test.go` | Config tests |
|
|
|
|
|
+| 6 | `internal/config/keybinds.go` | Keybind definitions |
|
|
|
|
|
+| 7 | `internal/config/theme.go` | Theme TOML unmarshaling |
|
|
|
|
|
+| 8 | `internal/config/editor_unix.go` | Unix editor command |
|
|
|
|
|
+| 9 | `internal/config/editor_default.go` | Non-unix editor command |
|
|
|
|
|
+| 10 | `internal/consts/consts.go` | App name + cache dir |
|
|
|
|
|
+| 11 | `internal/logger/logger.go` | slog file logger |
|
|
|
|
|
+| 12 | `internal/keyring/keyring.go` | Token keyring wrapper |
|
|
|
|
|
+| 13 | `internal/keyring/keyring_test.go` | Keyring tests |
|
|
|
|
|
+| 14 | `internal/cache/cache.go` | Autocomplete cache |
|
|
|
|
|
+| 15 | `internal/http/transport.go` | HTTP transport (gzip + brotli) |
|
|
|
|
|
+| 16 | `internal/http/client.go` | API client constructor |
|
|
|
|
|
+| 17 | `internal/http/headers.go` | Request headers |
|
|
|
|
|
+| 18 | `internal/http/props.go` | Discord identify properties |
|
|
|
|
|
+| 19 | `internal/notifications/notifications.go` | Notification dispatch |
|
|
|
|
|
+| 20 | `internal/notifications/desktop_toast.go` | Non-darwin notification |
|
|
|
|
|
+| 21 | `internal/notifications/desktop_toast_darwin.go` | macOS notification |
|
|
|
|
|
+| 22 | `internal/clipboard/clipboard.go` | Clipboard format types |
|
|
|
|
|
+| 23 | `internal/clipboard/clipboard_default.go` | Default clipboard impl |
|
|
|
|
|
+| 24 | `internal/clipboard/clipboard_wayland.go` | Wayland clipboard impl |
|
|
|
|
|
+| 25 | `internal/markdown/renderer.go` | Discord markdown renderer |
|
|
|
|
|
+| 26 | `internal/ui/util.go` | Shared UI helpers |
|
|
|
|
|
+| 27 | `internal/ui/root/model.go` | Root model + help overlay |
|
|
|
|
|
+| 28 | `internal/ui/root/events.go` | Root events (token, clipboard) |
|
|
|
|
|
+| 29 | `internal/ui/root/keybinds.go` | Root keymap |
|
|
|
|
|
+| 30 | `internal/ui/root/suspend_unix.go` | Unix SIGTSTP suspend |
|
|
|
|
|
+| 31 | `internal/ui/root/suspend_default.go` | No-op suspend |
|
|
|
|
|
+| 32 | `internal/ui/chat/model.go` | Chat model + layout |
|
|
|
|
|
+| 33 | `internal/ui/chat/state.go` | Gateway state commands |
|
|
|
|
|
+| 34 | `internal/ui/chat/events.go` | Chat event handlers |
|
|
|
|
|
+| 35 | `internal/ui/chat/keybinds.go` | Chat keymap |
|
|
|
|
|
+| 36 | `internal/ui/chat/guilds_tree.go` | Guild/channel tree |
|
|
|
|
|
+| 37 | `internal/ui/chat/guildstate.go` | Guild expand persistence |
|
|
|
|
|
+| 38 | `internal/ui/chat/messages_list.go` | Message rendering + actions |
|
|
|
|
|
+| 39 | `internal/ui/chat/message_input.go` | Message input + mentions |
|
|
|
|
|
+| 40 | `internal/ui/chat/attachments_picker.go` | Attachment picker |
|
|
|
|
|
+| 41 | `internal/ui/chat/channels_picker.go` | Channel picker |
|
|
|
|
|
+| 42 | `internal/ui/chat/mentions_list.go` | Mentions autocomplete list |
|
|
|
|
|
+| 43 | `internal/ui/chat/util.go` | Picker config + humanJoin |
|
|
|
|
|
+| 44 | `internal/ui/login/model.go` | Login model |
|
|
|
|
|
+| 45 | `internal/ui/login/events.go` | Login events |
|
|
|
|
|
+| 46 | `internal/ui/login/keybinds.go` | Login keymap |
|
|
|
|
|
+| 47 | `internal/ui/login/token/model.go` | Token login form |
|
|
|
|
|
+| 48 | `internal/ui/login/token/events.go` | Token events |
|
|
|
|
|
+| 49 | `internal/ui/login/qr/model.go` | QR login model |
|
|
|
|
|
+| 50 | `internal/ui/login/qr/events.go` | QR login events + crypto |
|
|
|
|
|
+| 51 | `go.mod` | Module manifest |
|
|
|
|
|
+| 52 | `.github/workflows/ci.yml` | CI configuration |
|
|
|
|
|
+| 53 | `.gitignore` | Git ignore rules |
|
|
|
|
|
+| 54 | `README.md` | Project readme |
|
|
|
|
|
+| 55 | `CLAUDE.md` | Project context |
|
|
|
|
|
+| 56 | `internal/config/config.toml` | Default config (embedded) |
|