feat: add ppg feedback command and Mac app feedback button#124
Open
2witstudios wants to merge 1 commit intomainfrom
Open
feat: add ppg feedback command and Mac app feedback button#1242witstudios wants to merge 1 commit intomainfrom
2witstudios wants to merge 1 commit intomainfrom
Conversation
Add a `ppg feedback` CLI command that creates GitHub issues via `gh issue create` when available, falling back to opening a pre-filled issue URL in the browser. Includes --title, --body, --label, and --json flags. Add a feedback button (exclamationmark.bubble icon) to the Mac app sidebar footer that opens a native sheet with category picker, title/body fields, and submits via the CLI command.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe changes introduce a new feedback feature across the macOS application and CLI. The macOS app adds a feedback button in the sidebar, a FeedbackViewController for collecting user input with validation and submission, and integrates this flow into the dashboard. The CLI adds a feedback command that submits issues to GitHub either via the gh CLI or by opening a browser URL with pre-filled parameters. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar as SidebarViewController
participant Dashboard as DashboardSplitViewController
participant FeedbackVC as FeedbackViewController
participant Service as PPGService
participant Shell
User->>Sidebar: Click feedback button
Sidebar->>Dashboard: onFeedbackClicked()
Dashboard->>FeedbackVC: Present as sheet
User->>FeedbackVC: Fill form & click Submit
FeedbackVC->>FeedbackVC: Validate title & body
alt Validation fails
FeedbackVC->>FeedbackVC: Shake animation
else Validation succeeds
FeedbackVC->>FeedbackVC: Disable controls, show spinner
FeedbackVC->>FeedbackVC: Determine project root
FeedbackVC->>Service: runPPGCommand(shellCmd)
Service->>Shell: Execute PPG command
Shell-->>Service: Command result
alt Success
Service-->>FeedbackVC: Result on main thread
FeedbackVC->>User: Show success alert
FeedbackVC->>FeedbackVC: Dismiss sheet
else Error
Service-->>FeedbackVC: Error with output
FeedbackVC->>User: Show error alert
end
end
sequenceDiagram
participant User
participant CLI as cli.ts
participant Feedback as feedbackCommand
participant GH as gh CLI
participant Browser
participant GitHub
User->>CLI: feedback --title "..." --body "..."
CLI->>Feedback: feedbackCommand(options)
Feedback->>Feedback: isGhAvailable()
alt gh CLI available
Feedback->>GH: gh issue create ...
GH->>GitHub: Create issue
GitHub-->>GH: Issue URL
GH-->>Feedback: URL in stdout
else gh not available
Feedback->>Feedback: buildIssueUrl(params)
Feedback->>Browser: open(url)
Browser->>GitHub: Navigate to issue template
end
Feedback->>User: Output result (JSON or text)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/commands/feedback.ts (1)
51-59: Add error handling for browser open failure.Similar to the gh path, the browser fallback should handle potential errors when opening the URL.
🛡️ Proposed fix
const issueUrl = buildIssueUrl(title, body, label); info('Opening GitHub issue in browser (gh CLI not found)'); - await execa('open', [issueUrl]); - - if (json) { - output({ success: true, issueUrl, method: 'browser' }, true); - } else { - success(`Opened browser: ${issueUrl}`); + try { + await execa('open', [issueUrl]); + if (json) { + output({ success: true, issueUrl, method: 'browser' }, true); + } else { + success(`Opened browser: ${issueUrl}`); + } + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (json) { + output({ success: false, error: message, method: 'browser' }, true); + } else { + throw new Error(`Failed to open browser: ${message}`); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/feedback.ts` around lines 51 - 59, The browser fallback currently calls execa('open', [issueUrl]) without handling failures; wrap that call in a try/catch around the execa invocation (the block using buildIssueUrl and execa('open', [issueUrl])) and on error log the failure and return the appropriate JSON or human output: if json is true call output({ success: false, issueUrl, method: 'browser', error: error.message }, true) otherwise call info/error or failure similar to success (e.g., error(`Failed to open browser: ${error.message}`)); ensure you still don't throw so downstream flow (output/success) only runs on success and mirror the existing behavior used in the gh path.PPG CLI/PPG CLI/FeedbackViewController.swift (1)
194-200: Shake animation requires layer-backed view.The shake animation uses
view.layer?.add(...), but this will silently do nothing if the view isn't layer-backed. EnsurewantsLayer = trueis set before animating.♻️ Proposed fix
private func shake(_ view: NSView) { + view.wantsLayer = true let animation = CAKeyframeAnimation(keyPath: "position.x") animation.values = [0, -6, 6, -4, 4, -2, 2, 0].map { view.frame.midX + $0 } animation.duration = 0.4 animation.calculationMode = .linear view.layer?.add(animation, forKey: "shake") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PPG` CLI/PPG CLI/FeedbackViewController.swift around lines 194 - 200, The shake(_ view: NSView) function may silently fail when the view isn't layer-backed; before adding the CAKeyframeAnimation to view.layer, ensure the view is layer-backed by setting view.wantsLayer = true (and verify view.layer is non-nil) so the animation is applied — update the shake(_:) implementation to enable layer-backing on the passed NSView before calling view.layer?.add(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@PPG` CLI/PPG CLI/FeedbackViewController.swift:
- Around line 189-191: Replace the force-unwrap of view.window in the
FeedbackViewController where alert.beginSheetModal(for: view.window!) is called:
use optional binding (e.g., guard/if let) to safely unwrap view.window, bail out
or handle the nil case gracefully if the window is nil, and call
alert.beginSheetModal(for: window) only when the unwrapped window is available;
update any early-return behavior so dismissal (self?.dismiss) only runs when
appropriate.
In `@src/commands/feedback.ts`:
- Line 53: The call to execa('open', [issueUrl]) is macOS-specific; update the
feedback command to open URLs cross-platform by either switching to the
well-tested "open" npm package or by selecting the OS-specific opener before
calling execa: detect process.platform and use 'open' on darwin, 'xdg-open' on
linux, and the Windows equivalent ('start' via shell) for win32; modify the code
around the execa invocation in src/commands/feedback.ts (the location with the
execa('open', [issueUrl]) call) to perform this platform check or replace it
with the "open" package so opening the issue URL works on Linux and Windows as
well.
- Around line 42-49: Wrap the execa call that runs the GitHub CLI ("const result
= await execa('gh', ghArgs, execaEnv);") in a try/catch and handle rejections
from the "gh issue create" invocation: on error, parse the thrown Error (or
execa error) and emit a user-friendly message (use output({ success: false,
error: err.message }, json) when json is requested or call error(`Failed to
create issue: ${err.message}`) for human output), and ensure the function
exits/returns after handling the error so downstream code (the success() branch)
does not run.
---
Nitpick comments:
In `@PPG` CLI/PPG CLI/FeedbackViewController.swift:
- Around line 194-200: The shake(_ view: NSView) function may silently fail when
the view isn't layer-backed; before adding the CAKeyframeAnimation to
view.layer, ensure the view is layer-backed by setting view.wantsLayer = true
(and verify view.layer is non-nil) so the animation is applied — update the
shake(_:) implementation to enable layer-backing on the passed NSView before
calling view.layer?.add(...).
In `@src/commands/feedback.ts`:
- Around line 51-59: The browser fallback currently calls execa('open',
[issueUrl]) without handling failures; wrap that call in a try/catch around the
execa invocation (the block using buildIssueUrl and execa('open', [issueUrl]))
and on error log the failure and return the appropriate JSON or human output: if
json is true call output({ success: false, issueUrl, method: 'browser', error:
error.message }, true) otherwise call info/error or failure similar to success
(e.g., error(`Failed to open browser: ${error.message}`)); ensure you still
don't throw so downstream flow (output/success) only runs on success and mirror
the existing behavior used in the gh path.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
PPG CLI/PPG CLI/DashboardSplitViewController.swiftPPG CLI/PPG CLI/FeedbackViewController.swiftPPG CLI/PPG CLI/SidebarViewController.swiftsrc/cli.tssrc/commands/feedback.ts
Comment on lines
+189
to
+191
| alert.beginSheetModal(for: view.window!) { [weak self] _ in | ||
| self?.dismiss(nil) | ||
| } |
There was a problem hiding this comment.
Avoid force unwrap on view.window!.
If view.window is nil (e.g., the view was dismissed before this code runs), this will crash. Use optional binding or a guard statement.
🛡️ Proposed fix
private func showSuccessAndDismiss() {
let alert = NSAlert()
alert.messageText = "Feedback submitted"
alert.informativeText = "Thank you for your feedback!"
alert.alertStyle = .informational
- alert.beginSheetModal(for: view.window!) { [weak self] _ in
- self?.dismiss(nil)
- }
+ guard let window = view.window else {
+ dismiss(nil)
+ return
+ }
+ alert.beginSheetModal(for: window) { [weak self] _ in
+ self?.dismiss(nil)
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@PPG` CLI/PPG CLI/FeedbackViewController.swift around lines 189 - 191, Replace
the force-unwrap of view.window in the FeedbackViewController where
alert.beginSheetModal(for: view.window!) is called: use optional binding (e.g.,
guard/if let) to safely unwrap view.window, bail out or handle the nil case
gracefully if the window is nil, and call alert.beginSheetModal(for: window)
only when the unwrapped window is available; update any early-return behavior so
dismissal (self?.dismiss) only runs when appropriate.
Comment on lines
+42
to
+49
| const result = await execa('gh', ghArgs, execaEnv); | ||
| const issueUrl = result.stdout.trim(); | ||
|
|
||
| if (json) { | ||
| output({ success: true, issueUrl, method: 'gh' }, true); | ||
| } else { | ||
| success(`Issue created: ${issueUrl}`); | ||
| } |
There was a problem hiding this comment.
Add error handling for gh issue create failure.
If gh issue create fails (e.g., authentication issues, network errors, invalid label), the promise will reject with an unhandled error. Consider catching and providing a user-friendly error message.
🛡️ Proposed fix
- const result = await execa('gh', ghArgs, execaEnv);
- const issueUrl = result.stdout.trim();
-
- if (json) {
- output({ success: true, issueUrl, method: 'gh' }, true);
- } else {
- success(`Issue created: ${issueUrl}`);
- }
+ try {
+ const result = await execa('gh', ghArgs, execaEnv);
+ const issueUrl = result.stdout.trim();
+
+ if (json) {
+ output({ success: true, issueUrl, method: 'gh' }, true);
+ } else {
+ success(`Issue created: ${issueUrl}`);
+ }
+ } catch (err) {
+ const message = err instanceof Error ? err.message : String(err);
+ if (json) {
+ output({ success: false, error: message, method: 'gh' }, true);
+ } else {
+ throw new Error(`Failed to create issue: ${message}`);
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/feedback.ts` around lines 42 - 49, Wrap the execa call that runs
the GitHub CLI ("const result = await execa('gh', ghArgs, execaEnv);") in a
try/catch and handle rejections from the "gh issue create" invocation: on error,
parse the thrown Error (or execa error) and emit a user-friendly message (use
output({ success: false, error: err.message }, json) when json is requested or
call error(`Failed to create issue: ${err.message}`) for human output), and
ensure the function exits/returns after handling the error so downstream code
(the success() branch) does not run.
| } else { | ||
| const issueUrl = buildIssueUrl(title, body, label); | ||
| info('Opening GitHub issue in browser (gh CLI not found)'); | ||
| await execa('open', [issueUrl]); |
There was a problem hiding this comment.
Browser fallback uses macOS-specific open command.
The open command is macOS-specific. On Linux, xdg-open is used, and on Windows, start is used. Consider using a cross-platform approach or documenting this limitation.
♻️ Cross-platform alternative
+import { platform } from 'node:os';
+
+function getOpenCommand(): string {
+ switch (platform()) {
+ case 'darwin': return 'open';
+ case 'win32': return 'start';
+ default: return 'xdg-open';
+ }
+}
+
// In the else branch:
- await execa('open', [issueUrl]);
+ const openCmd = getOpenCommand();
+ await execa(openCmd, [issueUrl], { shell: platform() === 'win32' });Alternatively, consider using the open npm package which handles cross-platform URL opening.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/feedback.ts` at line 53, The call to execa('open', [issueUrl])
is macOS-specific; update the feedback command to open URLs cross-platform by
either switching to the well-tested "open" npm package or by selecting the
OS-specific opener before calling execa: detect process.platform and use 'open'
on darwin, 'xdg-open' on linux, and the Windows equivalent ('start' via shell)
for win32; modify the code around the execa invocation in
src/commands/feedback.ts (the location with the execa('open', [issueUrl]) call)
to perform this platform check or replace it with the "open" package so opening
the issue URL works on Linux and Windows as well.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ppg feedback --title "..." --body "..." [--label bug|feature|feedback] [--json]CLI command that creates GitHub issues viagh issue create, falling back to opening a pre-filled browser URL whenghis unavailableTest plan
npm run build && node dist/cli.js feedback --title "Test" --body "Test body" --json— should create a GitHub issue viaghCLIghfrom PATHnpm run typecheckpassesSummary by CodeRabbit
New Features
feedbackcommand to the CLI supporting GitHub integration via gh CLI or browser-based submission