feat(desktop): remote relay push, commit log, and tag delete options#15
feat(desktop): remote relay push, commit log, and tag delete options#15kooksee wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces macOS Keychain integration for secure GitHub token storage and authentication, adds a new Commit Log module for viewing commit history and details, supports local and remote tag deletion, and adds a remote relay push feature to sync repositories with external platforms. The reviewer feedback highlights a security vulnerability where sensitive tokens are exposed in the process list via the security CLI, an unused parameter in parseLogItems, a potential metadata parsing bug in parseLogDetail, a risk of silently overwriting existing remotes in remoteRelayPush, and a reliability issue where local tags are deleted before remote tags.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
I am having trouble creating individual review comments. Click here to see my feedback.
desktop/service.go (2953-2965)
Passing sensitive credentials (like the GitHub token) as a command-line argument to the security CLI tool exposes them in the system's process list (e.g., visible via ps aux or ps -ef to any local process or user).\n\nTo prevent this exposure, consider using a secure Go library that interacts with the macOS Keychain API directly via CGO or system frameworks (such as github.com/keybase/go-keychain or github.com/zalando/go-keyring), rather than spawning the security CLI with the -w flag.
desktop/frontend/src/app/providers/app-provider.tsx (185)
The index parameter in the .flatMap callback is unused. This is inconsistent with the cleanup done in parseRemoteItems and can trigger ESLint no-unused-vars errors.
.flatMap((line) => {
desktop/frontend/src/app/providers/app-provider.tsx (467-469)
Skipping empty lines with continue inside the metadata parsing loop can lead to incorrect parsing or metadata injection. Since the metadata block is contiguous and separated from the commit body by an empty line, the loop should break immediately upon encountering an empty line. This prevents any lines in the commit body that happen to match the metadata format (e.g., starting with commit\t or author\t) from being incorrectly parsed as metadata.
if (!line) {\n break;\n }
desktop/service.go (1477-1485)
Silently updating/overwriting an existing remote (such as origin) without warning or confirmation can lead to accidental configuration loss or confusion if the user inputs an existing remote name.\n\nConsider either:\n1. Pushing directly to the target URL (e.g., git push <url> --all) without adding or modifying a local remote, which avoids polluting the local git config.\n2. Or returning an error/warning if the remote already exists to prevent accidental overwrites.
desktop/service.go (2178-2186)
The local tag is deleted before attempting to delete the remote tag. If the remote deletion fails (e.g., due to network issues, authentication failures, or permission restrictions), the local tag is already gone, leaving the repository in an inconsistent state and making it harder for the user to retry the deletion.\n\nIt is safer to delete the remote tag first, and only delete the local tag if the remote deletion succeeds.
Summary
Verification
Notes