fix: unify diagnostics package export - #1240
Conversation
Astro-Han
added
enhancement
|
Warning Review limit reached
More reviews will be available in 41 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR consolidates the fragmented diagnostics and feedback export flows into a single unified "prepare diagnostics package" entry point, updated localization copy throughout the app, added backend log path tracking, and rewired menus to always offer the option regardless of feedback URL availability. ChangesDiagnostics Package Unification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/desktop-api-contract.ts, packages/app/src/i18n/en.ts, packages/app/src/i18n/zh.ts, packages/app/src/pages/error-report.test.ts, packages/app/src/pages/error-report.ts, packages/app/src/pages/layout/pawwork-session-commands.test.ts, packages/app/src/pages/layout/pawwork-session-commands.ts, packages/desktop-electron/src/main/feedback.test.ts, packages/desktop-electron/src/main/feedback.ts, packages/desktop-electron/src/main/index.ts, packages/desktop-electron/src/main/logging.test.ts, packages/desktop-electron/src/main/logging.ts, packages/desktop-electron/src/main/menu-labels.test.ts, packages/desktop-electron/src/main/menu-labels.ts, packages/desktop-electron/src/main/menu-template.test.ts, packages/desktop-electron/src/main/menu-template.ts, packages/desktop-electron/src/main/menu.test.ts, packages/desktop-electron/src/main/server.test.ts, packages/desktop-electron/src/main/server.ts)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the raw session JSON export with a comprehensive "Diagnostics Package" that bundles session content, renderer diagnostics, app logs, local paths, and environment details. It also introduces a "package-only" status for builds without a configured feedback form URL, allowing users to still generate and save local diagnostics packages. The feedback dialog, menu items, and tests have been updated and localized accordingly. Feedback on the changes suggests making the confirmation dialog labels dynamically aware of whether a feedback form is configured to avoid confusing users in the package-only scenario.
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.
![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 (1)
packages/desktop-electron/src/main/feedback.test.ts (1)
394-411: 💤 Low valueTest coverage for package-only path looks good.
The test verifies the core package-only behavior: summary copied, markdown saved, folder revealed, but feedback form not opened. The result structure correctly expects
status: "package-only"withfullReport.status: "ready".Minor observation: The edge case where
saveReportfails ANDfeedbackUrlis missing (producingpackage-only+fullReport.status: "failed") is not explicitly tested, though the ternary logic at lines 271-273 offeedback.tshandles it correctly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/desktop-electron/src/main/feedback.test.ts` around lines 394 - 411, Add a test covering the edge case where saveReport fails while feedbackUrl is empty: use the existing setup helper to create subject with feedbackUrl: "" but stub or mock saveReport to return a failed report (or throw) so subject.handler() yields status "package-only" and fullReport.status "failed"; assert copied summary, saved markdown, revealed folder, opened is "", and result.fullReport.fileName/locationHint reflect failure case (use same expect.stringContaining checks), referencing setup, subject.handler, and saveReport in feedback.ts to locate where to inject the failing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/app/e2e/sidebar/sidebar-session-organization.spec.ts`:
- Around line 94-98: Add a positive assertion that the session row menu actually
opened before performing the negative checks: after calling
row.locator('[data-action="session-row-menu"]').click(), assert that a known
visible menu item (e.g., page.getByRole("menuitem", { name: /rename/i }) or
another stable menu option) is present using expect(...).toHaveCount(1) or
toBeVisible(), then proceed with the existing negative assertions for
export/diagnostics; update the block around row.locator(...) and the subsequent
page.getByRole("menuitem", { name: /export session/i }) and
page.getByRole("menuitem", { name: /diagnostics package/i }) checks to ensure
the positive check runs first.
In `@packages/app/src/i18n/zh.ts`:
- Around line 491-509: Update the translation for the key
"error.page.report.confirm.privacy" to remove the ambiguous "默认" so it matches
the English phrasing; replace the current value
"诊断包可能包含会话内容、应用日志、本地路径和环境信息。默认不会自动上传你的原始项目文件。" with a definitive privacy
statement (e.g. "诊断包可能包含会话内容、应用日志、本地路径和环境信息。不会自动上传你的原始项目文件。") to ensure
consistency.
In `@packages/desktop-electron/src/main/server.ts`:
- Around line 69-72: Reset backendLogPath to undefined (or an empty value)
before attempting to call Log.file() so stale path isn't retained when Log.file
is absent; specifically, set backendLogPath = undefined right before the block
that reads (Log as typeof Log & { file?: () => string }).file and then only
assign backendLogPath = logFile() if typeof logFile === "function" inside that
block (symbols: backendLogPath, Log.file).
---
Nitpick comments:
In `@packages/desktop-electron/src/main/feedback.test.ts`:
- Around line 394-411: Add a test covering the edge case where saveReport fails
while feedbackUrl is empty: use the existing setup helper to create subject with
feedbackUrl: "" but stub or mock saveReport to return a failed report (or throw)
so subject.handler() yields status "package-only" and fullReport.status
"failed"; assert copied summary, saved markdown, revealed folder, opened is "",
and result.fullReport.fileName/locationHint reflect failure case (use same
expect.stringContaining checks), referencing setup, subject.handler, and
saveReport in feedback.ts to locate where to inject the failing behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 191567ca-62e7-48c6-a917-2d00adad9034
📒 Files selected for processing (21)
packages/app/e2e/sidebar/sidebar-session-organization.spec.tspackages/app/src/desktop-api-contract.tspackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/pages/error-report.test.tspackages/app/src/pages/error-report.tspackages/app/src/pages/layout/pawwork-session-commands.test.tspackages/app/src/pages/layout/pawwork-session-commands.tspackages/desktop-electron/scripts/report-problem-smoke.mjspackages/desktop-electron/src/main/feedback.test.tspackages/desktop-electron/src/main/feedback.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/logging.test.tspackages/desktop-electron/src/main/logging.tspackages/desktop-electron/src/main/menu-labels.test.tspackages/desktop-electron/src/main/menu-labels.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/menu-template.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/server.test.tspackages/desktop-electron/src/main/server.ts
1 participant
Summary
Unifies the user-facing diagnostics path into one support package entry from Help and error reporting, while hiding the old raw session JSON export from the session row menu.
The package now keeps session content, renderer diagnostics, environment context, and both main-process and backend log tails together. If a build has no feedback form URL configured, the same action still prepares the local diagnostics package instead of disappearing.
Why
Issue #1205 asks for one clear support package path without auto-upload, checklist building, or a broad feedback form redesign. The previous UX split troubleshooting across Report a Problem, Export Diagnostics Log, and raw session export, which made it easy to send an incomplete report.
Related Issue
Closes #1205
Human Review Status
Pending
Review Focus
Please check the fallback behavior when
PAWWORK_FEEDBACK_FORM_URLis empty, the combined main/backend log tail collection, and the copy that explains what the package may contain before export.Risk Notes
This touches desktop menu behavior, local diagnostics file generation, and user-facing copy on macOS and Windows. The old standalone diagnostics-log menu item and sidebar raw session JSON entry are no longer user-facing; the IPC helpers remain in place for internal callers and tests.
How To Verify
Screenshots or Recordings
Generated and reviewed
docs/design/preview/screenshots/sidebar.pngfrombun run snap sidebar.Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit
New Features
User-Facing Changes
Tests