fix: connect renderer error reports to feedback form#187
Conversation
Astro-Han
added
enhancement
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a renderer-to-desktop problem-reporting flow: new types and Platform.reportProblem API, localized report UI and helpers for formatting/summarizing errors, IPC + preload wiring for "report-problem", desktop feedback handler returning structured results, payload support for rendererError with sanitization/truncation, and tests/workflow updates. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant ErrorPage as Error Page (renderer)
participant Platform as Platform (renderer)
participant Preload as Preload API
participant IPC as ipcRenderer / ipcMain
participant Main as Main Process
participant FS as File System
participant Dialog as Feedback Dialog
User->>ErrorPage: Click "Report Problem"
ErrorPage->>ErrorPage: Build summary & details
ErrorPage->>User: Show confirmation (privacy + summary)
User->>ErrorPage: Confirm
ErrorPage->>Platform: reportProblem(rendererError)
Platform->>Preload: window.api.reportProblem(input)
Preload->>IPC: invoke("report-problem", input)
IPC->>Main: deps.reportProblem(input)
Main->>Main: Build full report (include rendererError)
Main->>FS: Write report file (may succeed/fail)
Main->>Dialog: Open feedback form (may succeed/fail)
alt success (form opened + file written)
Main->>Main: Copy summary to clipboard
Main-->>IPC: {status:"ready", summaryCopied:true, feedbackOpened:true, fullReport:{status:"ready", ...}}
else form open failed but file written
Main->>Main: Copy summary to clipboard
Main-->>IPC: {status:"form-fallback", feedbackUrl, summaryCopied:true, fullReport:{status:"ready", ...}}
else file write failed
Main-->>IPC: {status:"summary-only", summaryCopied:true, fullReport:{status:"failed"}}
else unexpected error
Main-->>IPC: {status:"failed", summaryCopied:false, feedbackOpened:false, fullReport:{status:"failed"}}
end
IPC-->>Preload: FeedbackResult
Preload-->>Platform: FeedbackResult
Platform-->>ErrorPage: ReportProblemResult
ErrorPage->>User: Display status message / fallback instructions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 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)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/desktop-electron/src/main/problem-report.ts (2)
402-414:⚠️ Potential issue | 🟡 MinorValidate optional
rendererErrorshape when parsing payloads.
isProblemReportPayloaddoesn’t verify the optionalrendererErrorfield, butparseProblemReportPayload()returnsPayload. This can admit malformed data under a typed return.Proposed fix
+function isRendererErrorDetails(value: unknown): value is RendererErrorDetails { + return isRecord(value) && typeof value.summary === "string" && typeof value.details === "string" +} + function isProblemReportPayload(value: unknown): value is Payload { if (!isRecord(value)) return false return ( @@ typeof value.logTail === "string" && + (value.rendererError === undefined || isRendererErrorDetails(value.rendererError)) && isSessionExport(value.sessionExport) && isTruncation(value.truncation) ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop-electron/src/main/problem-report.ts` around lines 402 - 414, isProblemReportPayload currently doesn't validate the optional rendererError property which allows malformed payloads to pass as a Payload; update isProblemReportPayload to include a check that value.rendererError is either undefined or satisfies the correct shape (e.g. add or reuse a type guard like isRendererError) so parseProblemReportPayload's returned Payload is actually validated; reference the isProblemReportPayload function and the rendererError field and ensure the predicate includes (value.rendererError === undefined || isRendererError(value.rendererError)) or an equivalent shape check.
192-255:⚠️ Potential issue | 🟠 MajorCap or drop oversized
rendererErrorduring payload shrinking.
rendererError.detailsis currently unbounded. If it alone pushes the payload overmaxBytes, the function throws after all other truncation passes, causing report generation to fail even though a degraded report could be produced.Proposed fix
export function buildProblemReport(input: Input, options: Options = {}) { @@ - let omittedDiagnosticsBytes = 0 + let omittedDiagnosticsBytes = 0 + let rendererError = input.rendererError @@ const makePayload = (): Payload => ({ @@ - ...(input.rendererError ? { rendererError: input.rendererError } : {}), + ...(rendererError ? { rendererError } : {}), @@ while (bytes(output) > maxBytes && diagnosticStringLimit >= 0) { @@ } + + if (bytes(output) > maxBytes && rendererError) { + const originalDetails = rendererError.details + let detailsLimit = Math.max(0, Math.floor(originalDetails.length / 2)) + while (bytes(output) > maxBytes && detailsLimit >= 0) { + rendererError = { ...rendererError, details: truncateString(originalDetails, detailsLimit) } + output = markdown(makePayload()) + if (detailsLimit === 0) break + detailsLimit = Math.floor(detailsLimit / 2) + } + } + + if (bytes(output) > maxBytes && rendererError) { + rendererError = undefined + output = markdown(makePayload()) + } if (bytes(output) > maxBytes) { throw new Error("Problem report exceeds maxBytes after truncation") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop-electron/src/main/problem-report.ts` around lines 192 - 255, The payload builder makePayload currently inlines input.rendererError without any truncation, so an oversized rendererError.details can make the report exceed maxBytes even after other truncation passes; modify the shrinking routine to treat rendererError like other fields: introduce a mutable rendererError variable derived from input.rendererError and, when bytes(output) > maxBytes, iteratively truncate rendererError.details (or drop rendererError entirely as a last resort) updating omitted counts and regenerating output via makePayload until the payload fits or rendererError is removed; ensure makePayload references the mutable rendererError (not input.rendererError) so the truncated/dropped value is used in all subsequent size checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/pages/error-report.ts`:
- Around line 228-233: The function errorReportStatusMessage does not explicitly
handle ReportProblemResult statuses "cancelled" and "unavailable", causing them
to fall back to the generic failure message; add explicit branches in
errorReportStatusMessage for result.status === "cancelled" and result.status ===
"unavailable" that return appropriate translator keys (e.g.,
t("error.page.report.cancelled") for a user-cancelled flow and
t("error.page.report.unavailable") for a disabled/unavailable feature) or map to
an existing non-error key, ensuring the logic distinguishes user cancellation
from actual failures.
In `@packages/app/src/pages/error.tsx`:
- Around line 49-56: The copyCurrentErrorDetails function can return true even
when navigator.clipboard is undefined because optional chaining yields undefined
rather than throwing; change copyCurrentErrorDetails to explicitly check for
navigator.clipboard (and navigator.clipboard.writeText) before calling it,
return false immediately if missing, and only attempt writeText when present;
optionally implement a safe fallback (e.g., use a textarea +
document.execCommand or return false) so the function accurately reports
success/failure.
---
Outside diff comments:
In `@packages/desktop-electron/src/main/problem-report.ts`:
- Around line 402-414: isProblemReportPayload currently doesn't validate the
optional rendererError property which allows malformed payloads to pass as a
Payload; update isProblemReportPayload to include a check that
value.rendererError is either undefined or satisfies the correct shape (e.g. add
or reuse a type guard like isRendererError) so parseProblemReportPayload's
returned Payload is actually validated; reference the isProblemReportPayload
function and the rendererError field and ensure the predicate includes
(value.rendererError === undefined || isRendererError(value.rendererError)) or
an equivalent shape check.
- Around line 192-255: The payload builder makePayload currently inlines
input.rendererError without any truncation, so an oversized
rendererError.details can make the report exceed maxBytes even after other
truncation passes; modify the shrinking routine to treat rendererError like
other fields: introduce a mutable rendererError variable derived from
input.rendererError and, when bytes(output) > maxBytes, iteratively truncate
rendererError.details (or drop rendererError entirely as a last resort) updating
omitted counts and regenerating output via makePayload until the payload fits or
rendererError is removed; ensure makePayload references the mutable
rendererError (not input.rendererError) so the truncated/dropped value is used
in all subsequent size checks.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e574d279-c742-4936-9299-a3bbb89fd601
📒 Files selected for processing (18)
packages/app/src/context/platform.tsxpackages/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/error.tsxpackages/app/src/pages/layout.tsxpackages/app/src/utils/support-links.tspackages/desktop-electron/src/main/feedback.test.tspackages/desktop-electron/src/main/feedback.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/main/problem-report.test.tspackages/desktop-electron/src/main/problem-report.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/renderer/index.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: smoke-macos-arm64
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-desktop
- GitHub Check: unit-opencode
- GitHub Check: e2e-artifacts
- GitHub Check: analyze-js-ts
🧰 Additional context used
📓 Path-based instructions (3)
packages/desktop-electron/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Renderer process should only call
window.apifromsrc/preload
Files:
packages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/renderer/index.tsxpackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/main/problem-report.tspackages/desktop-electron/src/main/problem-report.test.tspackages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/main/feedback.tspackages/desktop-electron/src/main/feedback.test.ts
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/utils/support-links.tspackages/app/src/i18n/en.tspackages/app/src/pages/error-report.test.tspackages/app/src/pages/layout.tsxpackages/app/src/i18n/zh.tspackages/app/src/context/platform.tsxpackages/app/src/pages/error-report.tspackages/app/src/pages/error.tsx
packages/desktop-electron/src/main/ipc.ts
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Main process should register IPC handlers in
src/main/ipc.ts
Files:
packages/desktop-electron/src/main/ipc.ts
🧠 Learnings (13)
📚 Learning: 2026-04-20T14:36:08.745Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/desktop-electron/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:08.745Z
Learning: Applies to packages/desktop-electron/src/main/ipc.ts : Main process should register IPC handlers in `src/main/ipc.ts`
Applied to files:
packages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-20T14:36:08.745Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/desktop-electron/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:08.745Z
Learning: Applies to packages/desktop-electron/src/**/*.{ts,tsx,js,jsx} : Renderer process should only call `window.api` from `src/preload`
Applied to files:
packages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/renderer/index.tsxpackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/preload/types.ts
📚 Learning: 2026-04-23T07:23:23.849Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 180
File: packages/app/src/components/session/session-new-view.tsx:13-18
Timestamp: 2026-04-23T07:23:23.849Z
Learning: In pawwork (Astro-Han/pawwork), prefer using `createStore` instead of multiple `createSignal` calls only when the signals represent **coupled** object state that is updated together (i.e., there is at least one shared batch-update site where the state is changed in the same transaction). If the state fields are **independent** and are mutated by separate handlers (e.g., one handler updates only `selectedSkill` while another updates only `mode`), keep them as individual `createSignal` calls—using `createStore` for truly independent fields adds boilerplate without behavioral benefit.
Applied to files:
packages/app/src/utils/support-links.tspackages/app/src/i18n/en.tspackages/app/src/pages/error-report.test.tspackages/app/src/pages/layout.tsxpackages/app/src/i18n/zh.tspackages/app/src/context/platform.tsxpackages/app/src/pages/error-report.tspackages/app/src/pages/error.tsx
📚 Learning: 2026-04-22T08:49:44.563Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:44.563Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/main/problem-report.test.tspackages/desktop-electron/src/main/feedback.test.ts
📚 Learning: 2026-04-22T05:24:42.404Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 98
File: packages/desktop-electron/src/main/menu-labels.ts:1-2
Timestamp: 2026-04-22T05:24:42.404Z
Learning: In Astro-Han/pawwork, the app i18n layer (`packages/app/src/i18n/`) only contains `en.ts` and `zh.ts`, and `normalizeLocale` (in `packages/app/src/context/language.tsx`) only returns `"en"` or `"zh"`. The desktop `MenuLocale = "en" | "zh"` union in `packages/desktop-electron/src/main/menu-labels.ts` is intentionally limited to these two locales and is not a broader restriction — do not flag it as overly restrictive or suggest adding other locales.
Applied to files:
packages/app/src/i18n/en.tspackages/app/src/i18n/zh.ts
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Define `const it = testEffect(...)` near the top of the test file and keep the test body inside `Effect.gen(function* () { ... })`. Yield services directly with `yield* MyService.Service` or `yield* MyTool`.
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test one feature per test file
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use `testEffect(...)` from `test/lib/effect.ts` for tests that exercise Effect services or Effect-based workflows.
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use lowercase, descriptive test names (e.g., 'sidebar can be toggled')
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Prefer fluent helpers and drivers when they make intent obvious and reduce locator-heavy noise in tests
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-22T09:32:52.535Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/ui/src/theme/context.tsx:11-16
Timestamp: 2026-04-22T09:32:52.535Z
Learning: In Astro-Han/pawwork (`packages/ui/src/theme/context.tsx` and related files), the renaming of localStorage theme keys from `opencode-*` to `pawwork-*` (THEME_ID, COLOR_SCHEME, THEME_CSS_LIGHT, THEME_CSS_DARK) is intentional and should NOT include a migration path from the old keys. Migrating would re-couple PawWork and OpenCode browser storage namespaces, which the PR is explicitly designed to avoid. A reset to the PawWork default theme on upgrade is acceptable by design.
Applied to files:
packages/app/src/pages/layout.tsx
📚 Learning: 2026-04-20T14:36:21.274Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.274Z
Learning: Applies to packages/opencode/**/*.ts : In `Effect.gen` / `Effect.fn`, prefer `yield* new MyError(...)` over `yield* Effect.fail(new MyError(...))` for direct early-failure branches
Applied to files:
packages/app/src/pages/error.tsx
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.
Applied to files:
packages/desktop-electron/src/main/feedback.test.ts
🔇 Additional comments (21)
packages/app/src/utils/support-links.ts (1)
1-1: Good extraction of support URL constant.Line 1 centralizes the GitHub issue URL and removes hardcoded-link drift across UI entry points.
packages/app/src/pages/layout.tsx (1)
21-21: Help-link wiring is clean and consistent.Line 21 and Line 2428 correctly switch the sidebar help action to the shared
PAWWORK_GITHUB_ISSUE_URL, which keeps link behavior consistent with other flows.Also applies to: 2428-2428
packages/desktop-electron/src/main/index.ts (1)
498-498: IPC dependency wiring looks correct.Line 498 cleanly forwards optional input to the existing
reportProblemimplementation through the IPC dependency map.packages/desktop-electron/src/preload/index.ts (1)
85-85: Preload bridge addition is correct.Line 85 exposes
reportProblemthrough the existing invoke pattern and keeps the renderer/main boundary consistent.packages/desktop-electron/src/renderer/index.tsx (1)
232-233: Platform adapter mapping is consistent.Line 232 correctly adds the
reportProblemplatform method and forwards the optional payload unchanged.packages/desktop-electron/src/main/ipc-window-config.test.ts (1)
15-18: Useful IPC contract guard test.Lines 15–18 add a focused assertion that the report-problem channel is present in IPC wiring, matching the intended renderer error-report path coverage.
packages/desktop-electron/src/main/problem-report.test.ts (1)
82-109: Strong coverage for renderer-error report content.Lines 82–109 correctly verify that
rendererErroris preserved in full report payloads and sanitized in the short summary.packages/desktop-electron/src/main/ipc.ts (1)
10-11: Typed IPC integration for problem reporting is solid.Lines 10–11, Line 58, and Lines 139–141 are coherently wired: imported types, dependency contract, and
"report-problem"handler all align.Also applies to: 58-58, 139-141
packages/app/src/i18n/en.ts (1)
491-514: Looks good — complete coverage for the new report flow copy.The new keys cleanly cover confirmation, privacy, success/fallback outcomes, and local-state error messaging.
packages/app/src/pages/error-report.test.ts (1)
27-100: Strong helper-level coverage for the new reporting behavior.These tests verify known-error summarization, detailed cause retention, and user-facing status mapping in a focused way.
packages/app/src/i18n/zh.ts (1)
478-496: LGTM for the Simplified Chinese report-flow localization block.The newly added keys are complete and consistent with the reporting UX states.
packages/desktop-electron/src/preload/types.ts (1)
1-5: Preload contract extension is consistent and correctly typed.
reportProblem(input?) => Promise<ReportProblemResult>matches the IPC/preload wiring pattern and keeps the API surface coherent.Also applies to: 92-92
packages/app/src/context/platform.tsx (1)
16-51: Type modeling for report flow is clean and explicit.The discriminated
ReportProblemResultunion andreportProblemplatform hook provide a solid contract for renderer-side handling.Also applies to: 104-105
packages/desktop-electron/src/main/feedback.test.ts (1)
107-148: Great upgrade to result-contract coverage.These assertions materially improve confidence in success/degraded reporting paths, especially renderer-triggered reporting and fallback statuses.
Also applies to: 241-249, 272-281, 346-352
packages/desktop-electron/src/main/feedback.ts (4)
46-75: Well-designed discriminated union for feedback results.The
FeedbackInputandFeedbackResulttypes are well-structured. The discriminated union withstatusfield provides good type narrowing for consumers, and each variant correctly reflects the state (e.g.,summaryCopied: truepaired withfeedbackOpened: trueforreadystatus).
178-189: LGTM: Input handling and confirmation logic.The optional
confirmfield (defaulting totrue) allows callers to skip the confirmation dialog when needed, while the early return for unavailable feedback URL ensures graceful degradation.
218-240: Renderer error correctly integrated into report pipeline.The
rendererErrorfrom input is properly passed to bothbuildProblemReport(line 221) andbuildProblemReportSummary(line 239), ensuring renderer crash context appears in both the full report file and the copied summary.
298-315: Solid deduplication and error handling.The
inFlightguard prevents concurrent report generations, and the.catchhandler correctly returns a typedFeedbackResultwithstatus: "failed"while still invokingonErrorfor logging/telemetry.packages/app/src/pages/error-report.ts (3)
32-47: Robust JSON serialization with circular reference handling.Good implementation using
WeakSetfor cycle detection (GC-friendly),bigintconversion to string, and fallback toString(value)for undefined results. This safely handles edge cases like functions and symbols.
159-184: Correct but verbose stack handling logic.The four explicit branches for
(isDuplicate, startsWithHeader)combinations are correct and handle all cases properly. The implicit case where there's no stack and it IS a duplicate correctly produces no output (avoiding repetition).
219-226: Well-structured summary fallback chain.The priority order (known error description →
Error.message→ first line of formatted output) provides sensible defaults for all error types while preferring user-friendly descriptions for known errors likeChildStoreError.
![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 implements a comprehensive error reporting system, allowing users to generate and submit diagnostic reports directly from the application's error interface. Key changes include the introduction of a dedicated error formatting utility, localized reporting strings for English and Chinese, and the integration of a new IPC channel to pass renderer-side error details to the Electron main process for inclusion in problem reports. Feedback was provided to improve the robustness of error type checking within the formatting logic to better handle multiple execution contexts in Electron.
Astro-Han
force-pushed
the
codex/fix-i158-error-report
branch
from
ebb8fc6 to
46444cf
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/pages/error-report.ts`:
- Around line 29-36: The type guard is accepting { name, data: null } because
typeof null === "object"; update isInitError to explicitly reject null by
checking (error as InitError).data !== null (and optionally verify data has the
expected fields like 'name' and 'message') so the function only returns true for
genuine InitError objects; update the guard in isInitError and ensure any
downstream usage in formatInitError relies on this stricter check.
- Around line 154-219: formatErrorChain can recurse forever on cyclic
error.cause chains; add cycle detection by threading a seen Set through calls:
change formatErrorChain signature to accept an optional seen: Set<unknown> (init
as new Set() on first call), before processing check if seen.has(error) and
return a short marker or empty string if already seen, and add each non-null
error to seen before recursing; update the recursive call to pass the same seen
Set so self-referential/cyclic cause chains are guarded against.
In `@packages/app/src/pages/error.tsx`:
- Around line 89-93: When platform.reportProblem is falsy in the reportProblem
function, don't open PAWWORK_GITHUB_ISSUE_URL directly; instead trigger the new
"unavailable" UX route so the confirmation copy and fallback options are shown.
Replace the platform.openLink(PAWWORK_GITHUB_ISSUE_URL) call in reportProblem's
fallback with a call that navigates to the error.page.report.unavailable UX
(e.g., platform.openLink('error.page.report.unavailable') or the project's
equivalent navigation helper), leaving the platform.reportProblem path
unchanged.
In `@packages/desktop-electron/src/main/feedback.ts`:
- Around line 301-308: The catch block in reportProblem
(packages/desktop-electron/src/main/feedback.ts) can rethrow if
deps.onError?.(error) rejects, so wrap the onError call in its own try/catch (or
await Promise.resolve(deps.onError?.(error)).catch(() => undefined)) to swallow
any errors from deps.onError and ensure the function always returns the fallback
FeedbackResult ({ status: "failed", summaryCopied: false, feedbackOpened: false,
fullReport: { status: "failed" } }) instead of letting the promise reject; do
not rethrow the error and preserve the existing "satisfies FeedbackResult"
shape.
In `@packages/desktop-electron/src/main/problem-report.ts`:
- Around line 310-315: safeRendererErrorSummary currently only calls
redactLocalPathFragments and may still leak storage/key identifiers; update the
function to also remove storage/key tokens before truncation by composing an
additional redaction step (e.g., call
redactStorageKeyFragments(redactLocalPathFragments(...)) or extend
redactLocalPathFragments to include storage/key regexes) so the final string
passed to oneLine and truncateSummaryLine has storage/key identifiers removed
while preserving SUMMARY_RENDERER_ERROR_MAX_CHARS behavior.
- Around line 62-63: The type guard isProblemReportPayload must validate the
optional rendererError so parseProblemReportPayload cannot return a payload with
malformed rendererError; update isProblemReportPayload (and any helpers it uses)
to: ensure payload.rendererError is either undefined or a non-null object and
that it contains the expected properties of RendererErrorDetails (e.g., check
typeof rendererError.message === 'string', typeof rendererError.name ===
'string', and if a stack is expected, typeof rendererError.stack === 'string' or
undefined) before returning true; adjust any tests accordingly so
parseProblemReportPayload only accepts payloads where rendererError conforms to
RendererErrorDetails.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d6b9581e-b00a-4705-adeb-e6f7963acd47
📒 Files selected for processing (18)
packages/app/src/context/platform.tsxpackages/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/error.tsxpackages/app/src/pages/layout.tsxpackages/app/src/utils/support-links.tspackages/desktop-electron/src/main/feedback.test.tspackages/desktop-electron/src/main/feedback.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/main/problem-report.test.tspackages/desktop-electron/src/main/problem-report.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/renderer/index.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: smoke-macos-arm64
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-desktop
- GitHub Check: unit-opencode
- GitHub Check: analyze-js-ts
- GitHub Check: e2e-artifacts
🧰 Additional context used
📓 Path-based instructions (3)
packages/desktop-electron/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Renderer process should only call
window.apifromsrc/preload
Files:
packages/desktop-electron/src/renderer/index.tsxpackages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/problem-report.test.tspackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/main/problem-report.tspackages/desktop-electron/src/main/feedback.test.tspackages/desktop-electron/src/main/feedback.ts
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/utils/support-links.tspackages/app/src/pages/layout.tsxpackages/app/src/pages/error-report.test.tspackages/app/src/context/platform.tsxpackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/pages/error.tsxpackages/app/src/pages/error-report.ts
packages/desktop-electron/src/main/ipc.ts
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Main process should register IPC handlers in
src/main/ipc.ts
Files:
packages/desktop-electron/src/main/ipc.ts
🧠 Learnings (16)
📚 Learning: 2026-04-20T14:36:08.745Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/desktop-electron/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:08.745Z
Learning: Applies to packages/desktop-electron/src/**/*.{ts,tsx,js,jsx} : Renderer process should only call `window.api` from `src/preload`
Applied to files:
packages/desktop-electron/src/renderer/index.tsxpackages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/main/problem-report.test.ts
📚 Learning: 2026-04-23T07:23:23.849Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 180
File: packages/app/src/components/session/session-new-view.tsx:13-18
Timestamp: 2026-04-23T07:23:23.849Z
Learning: In pawwork (Astro-Han/pawwork), prefer using `createStore` instead of multiple `createSignal` calls only when the signals represent **coupled** object state that is updated together (i.e., there is at least one shared batch-update site where the state is changed in the same transaction). If the state fields are **independent** and are mutated by separate handlers (e.g., one handler updates only `selectedSkill` while another updates only `mode`), keep them as individual `createSignal` calls—using `createStore` for truly independent fields adds boilerplate without behavioral benefit.
Applied to files:
packages/app/src/utils/support-links.tspackages/app/src/pages/layout.tsxpackages/app/src/pages/error-report.test.tspackages/app/src/context/platform.tsxpackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/pages/error.tsxpackages/app/src/pages/error-report.ts
📚 Learning: 2026-04-20T14:36:08.745Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/desktop-electron/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:08.745Z
Learning: Applies to packages/desktop-electron/src/main/ipc.ts : Main process should register IPC handlers in `src/main/ipc.ts`
Applied to files:
packages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-22T08:49:44.563Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:44.563Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/main/problem-report.test.tspackages/app/src/pages/error-report.test.tspackages/desktop-electron/src/main/feedback.test.ts
📚 Learning: 2026-04-22T09:32:52.535Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/ui/src/theme/context.tsx:11-16
Timestamp: 2026-04-22T09:32:52.535Z
Learning: In Astro-Han/pawwork (`packages/ui/src/theme/context.tsx` and related files), the renaming of localStorage theme keys from `opencode-*` to `pawwork-*` (THEME_ID, COLOR_SCHEME, THEME_CSS_LIGHT, THEME_CSS_DARK) is intentional and should NOT include a migration path from the old keys. Migrating would re-couple PawWork and OpenCode browser storage namespaces, which the PR is explicitly designed to avoid. A reset to the PawWork default theme on upgrade is acceptable by design.
Applied to files:
packages/app/src/pages/layout.tsx
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Define `const it = testEffect(...)` near the top of the test file and keep the test body inside `Effect.gen(function* () { ... })`. Yield services directly with `yield* MyService.Service` or `yield* MyTool`.
Applied to files:
packages/app/src/pages/error-report.test.tspackages/desktop-electron/src/main/feedback.test.ts
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use `testEffect(...)` from `test/lib/effect.ts` for tests that exercise Effect services or Effect-based workflows.
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.
Applied to files:
packages/app/src/pages/error-report.test.tspackages/desktop-electron/src/main/feedback.test.ts
📚 Learning: 2026-04-22T09:32:52.806Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/opencode/test/provider/provider.test.ts:64-85
Timestamp: 2026-04-22T09:32:52.806Z
Learning: In `packages/opencode/test/provider/provider.test.ts`, the file intentionally uses AppRuntime-based async helpers (`run`, `list`, `getProvider`, etc.) rather than `testEffect(...)` for all tests. Converting individual tests to `testEffect` while leaving the rest on the async pattern would create internal inconsistency. A full harness migration of this file is the right approach if the pattern needs to change, but that should be a separate PR.
Applied to files:
packages/app/src/pages/error-report.test.tspackages/desktop-electron/src/main/feedback.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test one feature per test file
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Import test utilities from `../fixtures` instead of `playwright/test`
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use lowercase, descriptive test names (e.g., 'sidebar can be toggled')
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Prefer fluent helpers and drivers when they make intent obvious and reduce locator-heavy noise in tests
Applied to files:
packages/app/src/pages/error-report.test.tspackages/desktop-electron/src/main/feedback.test.ts
📚 Learning: 2026-04-22T05:24:42.404Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 98
File: packages/desktop-electron/src/main/menu-labels.ts:1-2
Timestamp: 2026-04-22T05:24:42.404Z
Learning: In Astro-Han/pawwork, the app i18n layer (`packages/app/src/i18n/`) only contains `en.ts` and `zh.ts`, and `normalizeLocale` (in `packages/app/src/context/language.tsx`) only returns `"en"` or `"zh"`. The desktop `MenuLocale = "en" | "zh"` union in `packages/desktop-electron/src/main/menu-labels.ts` is intentionally limited to these two locales and is not a broader restriction — do not flag it as overly restrictive or suggest adding other locales.
Applied to files:
packages/app/src/i18n/en.tspackages/app/src/i18n/zh.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use `expect.poll(...)` for probing mock or backend state rather than transient DOM visibility
Applied to files:
packages/desktop-electron/src/main/feedback.test.ts
📚 Learning: 2026-04-20T14:36:21.274Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.274Z
Learning: Applies to packages/opencode/**/*.ts : In `Effect.gen` / `Effect.fn`, prefer `yield* new MyError(...)` over `yield* Effect.fail(new MyError(...))` for direct early-failure branches
Applied to files:
packages/app/src/pages/error.tsx
🔇 Additional comments (8)
packages/app/src/utils/support-links.ts (1)
1-1: Good centralization of the GitHub issue URL.This avoids hardcoded duplicates and keeps support-link changes maintainable.
packages/app/src/i18n/zh.ts (1)
478-497: Localization coverage for report flow looks complete.The new
error.page.report.*and known-local-state strings align well with the new renderer error reporting UX.packages/desktop-electron/src/main/problem-report.ts (1)
8-8: Renderer-error propagation through report payload and summary is well integrated.Nice job threading the new type from full report generation to summary rendering in a minimal, coherent way.
Also applies to: 28-31, 47-48, 198-199, 269-270, 319-320, 343-344
packages/desktop-electron/src/main/ipc-window-config.test.ts (1)
15-18: Good IPC contract coverage for the new channel.The added assertion closes the gap by explicitly guarding
report-problemwiring at startup.packages/desktop-electron/src/main/index.ts (1)
498-498: IPC handler registration is correctly wired to the existing report flow.This keeps report handling centralized while exposing the new renderer-trigger path.
packages/desktop-electron/src/renderer/index.tsx (1)
232-233: Platform bridge extension is clean and consistent.
reportProblemis exposed at the same abstraction layer as other desktop platform capabilities.packages/app/src/pages/layout.tsx (1)
21-21: Nice cleanup by reusing the shared support-link constant.This eliminates a hardcoded URL in the layout layer and keeps link ownership centralized.
Also applies to: 2428-2428
packages/desktop-electron/src/preload/index.ts (1)
85-85: Preload bridge wiring is correct for the new report channel.The new method cleanly forwards to IPC without bypassing existing boundary patterns.
Astro-Han
force-pushed
the
codex/fix-i158-error-report
branch
from
46444cf to
6342243
Compare
|
Handled the non-inline CodeRabbit items in 6342243:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/pages/error-report.ts`:
- Around line 183-226: The formatter (formatErrorChain) currently only emits
name/message/stack/cause and skips ChildStoreError.context, so update
formatErrorChain to detect when the error is a ChildStoreError (or has a context
property) and append its context fields (kind, directory, storage, key and any
other context entries) to the parts array—preferably as a compact labeled block
or JSON string—so buildErrorReportDetails / rendererError.details will include
those diagnostics; ensure this logic mirrors the same addition for the other
occurrence noted (the similar block around the later lines) and preserve
existing duplicate/header/stack behavior.
In `@packages/app/src/pages/error.tsx`:
- Around line 18-24: The result of platform.reportProblem() can return status:
"form-fallback" with a concrete feedbackUrl, but the code currently flattens
that into actionMessage/actionError and only shows the static GitHub fallback
button; update the state shape and handling so you keep and surface the
feedbackUrl instead of losing it: add a feedbackUrl?: string to ErrorPageStore,
update the reportProblem response handling (the code paths around
platform.reportProblem() that currently set actionMessage/actionError and
reportConfirmOpen) to set feedbackUrl when result.status === "form-fallback"
(and set actionMessage to a short user-facing message), and update the UI
rendering logic that shows the fallback button to render an actionable
link/button using feedbackUrl when present (fall back to the existing static
GitHub button only if feedbackUrl is undefined).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d4ebbba2-e49b-462e-9eb4-688a74406f60
📒 Files selected for processing (18)
packages/app/src/context/platform.tsxpackages/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/error.tsxpackages/app/src/pages/layout.tsxpackages/app/src/utils/support-links.tspackages/desktop-electron/src/main/feedback.test.tspackages/desktop-electron/src/main/feedback.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/main/problem-report.test.tspackages/desktop-electron/src/main/problem-report.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/renderer/index.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-desktop
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-session
- GitHub Check: typecheck
- GitHub Check: unit-opencode
- GitHub Check: smoke-macos-arm64
- GitHub Check: analyze-js-ts
- GitHub Check: e2e-artifacts
🧰 Additional context used
📓 Path-based instructions (3)
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/utils/support-links.tspackages/app/src/pages/layout.tsxpackages/app/src/i18n/en.tspackages/app/src/context/platform.tsxpackages/app/src/pages/error-report.test.tspackages/app/src/i18n/zh.tspackages/app/src/pages/error-report.tspackages/app/src/pages/error.tsx
packages/desktop-electron/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Renderer process should only call
window.apifromsrc/preload
Files:
packages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/renderer/index.tsxpackages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/main/problem-report.test.tspackages/desktop-electron/src/main/feedback.test.tspackages/desktop-electron/src/main/problem-report.tspackages/desktop-electron/src/main/feedback.ts
packages/desktop-electron/src/main/ipc.ts
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Main process should register IPC handlers in
src/main/ipc.ts
Files:
packages/desktop-electron/src/main/ipc.ts
🧠 Learnings (18)
📚 Learning: 2026-04-23T07:23:23.849Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 180
File: packages/app/src/components/session/session-new-view.tsx:13-18
Timestamp: 2026-04-23T07:23:23.849Z
Learning: In pawwork (Astro-Han/pawwork), prefer using `createStore` instead of multiple `createSignal` calls only when the signals represent **coupled** object state that is updated together (i.e., there is at least one shared batch-update site where the state is changed in the same transaction). If the state fields are **independent** and are mutated by separate handlers (e.g., one handler updates only `selectedSkill` while another updates only `mode`), keep them as individual `createSignal` calls—using `createStore` for truly independent fields adds boilerplate without behavioral benefit.
Applied to files:
packages/app/src/utils/support-links.tspackages/app/src/pages/layout.tsxpackages/app/src/i18n/en.tspackages/app/src/context/platform.tsxpackages/app/src/pages/error-report.test.tspackages/app/src/i18n/zh.tspackages/app/src/pages/error-report.tspackages/app/src/pages/error.tsx
📚 Learning: 2026-04-20T14:36:08.745Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/desktop-electron/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:08.745Z
Learning: Applies to packages/desktop-electron/src/main/ipc.ts : Main process should register IPC handlers in `src/main/ipc.ts`
Applied to files:
packages/desktop-electron/src/main/ipc-window-config.test.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-22T08:49:44.563Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:44.563Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/desktop-electron/src/main/ipc-window-config.test.tspackages/app/src/pages/error-report.test.tspackages/desktop-electron/src/main/problem-report.test.tspackages/desktop-electron/src/main/feedback.test.tspackages/desktop-electron/src/main/problem-report.ts
📚 Learning: 2026-04-20T14:36:08.745Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/desktop-electron/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:08.745Z
Learning: Applies to packages/desktop-electron/src/**/*.{ts,tsx,js,jsx} : Renderer process should only call `window.api` from `src/preload`
Applied to files:
packages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/renderer/index.tsxpackages/desktop-electron/src/preload/types.ts
📚 Learning: 2026-04-22T09:32:52.535Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/ui/src/theme/context.tsx:11-16
Timestamp: 2026-04-22T09:32:52.535Z
Learning: In Astro-Han/pawwork (`packages/ui/src/theme/context.tsx` and related files), the renaming of localStorage theme keys from `opencode-*` to `pawwork-*` (THEME_ID, COLOR_SCHEME, THEME_CSS_LIGHT, THEME_CSS_DARK) is intentional and should NOT include a migration path from the old keys. Migrating would re-couple PawWork and OpenCode browser storage namespaces, which the PR is explicitly designed to avoid. A reset to the PawWork default theme on upgrade is acceptable by design.
Applied to files:
packages/app/src/pages/layout.tsxpackages/app/src/pages/error-report.tspackages/desktop-electron/src/main/problem-report.tspackages/app/src/pages/error.tsx
📚 Learning: 2026-04-22T05:24:42.404Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 98
File: packages/desktop-electron/src/main/menu-labels.ts:1-2
Timestamp: 2026-04-22T05:24:42.404Z
Learning: In Astro-Han/pawwork, the app i18n layer (`packages/app/src/i18n/`) only contains `en.ts` and `zh.ts`, and `normalizeLocale` (in `packages/app/src/context/language.tsx`) only returns `"en"` or `"zh"`. The desktop `MenuLocale = "en" | "zh"` union in `packages/desktop-electron/src/main/menu-labels.ts` is intentionally limited to these two locales and is not a broader restriction — do not flag it as overly restrictive or suggest adding other locales.
Applied to files:
packages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/pages/error-report.ts
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Define `const it = testEffect(...)` near the top of the test file and keep the test body inside `Effect.gen(function* () { ... })`. Yield services directly with `yield* MyService.Service` or `yield* MyTool`.
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-22T09:32:52.806Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/opencode/test/provider/provider.test.ts:64-85
Timestamp: 2026-04-22T09:32:52.806Z
Learning: In `packages/opencode/test/provider/provider.test.ts`, the file intentionally uses AppRuntime-based async helpers (`run`, `list`, `getProvider`, etc.) rather than `testEffect(...)` for all tests. Converting individual tests to `testEffect` while leaving the rest on the async pattern would create internal inconsistency. A full harness migration of this file is the right approach if the pattern needs to change, but that should be a separate PR.
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test one feature per test file
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use `testEffect(...)` from `test/lib/effect.ts` for tests that exercise Effect services or Effect-based workflows.
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Prefer fluent helpers and drivers when they make intent obvious and reduce locator-heavy noise in tests
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/packages/app/src/testing/**/*.ts : Test-only hooks must be inert unless explicitly enabled and should not add normal-runtime listeners, reactive subscriptions, or per-update allocations
Applied to files:
packages/app/src/pages/error-report.test.ts
📚 Learning: 2026-04-20T14:21:51.047Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 71
File: packages/app/src/components/session/session-status-connections.tsx:146-147
Timestamp: 2026-04-20T14:21:51.047Z
Learning: In the Astro-Han/pawwork repository (SolidJS app), `sync.data.config` is always initialized to `{}` at `packages/app/src/context/global-sync.tsx` line 71 and is never `undefined` at runtime. Non-optional property access like `sync.data.config.plugin` is intentional and consistent with the pattern used in `packages/app/src/components/status-popover-body.tsx` line 243. Do not flag `sync.data.config.plugin` as needing optional chaining.
Applied to files:
packages/app/src/pages/error-report.tspackages/app/src/pages/error.tsx
📚 Learning: 2026-04-20T14:36:21.274Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.274Z
Learning: Applies to packages/opencode/**/*.ts : In `Effect.gen` / `Effect.fn`, prefer `yield* new MyError(...)` over `yield* Effect.fail(new MyError(...))` for direct early-failure branches
Applied to files:
packages/app/src/pages/error-report.tspackages/desktop-electron/src/main/feedback.ts
📚 Learning: 2026-04-20T17:03:37.710Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 73
File: packages/opencode/src/cli/cmd/tui/context/sync.tsx:486-489
Timestamp: 2026-04-20T17:03:37.710Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/cli/cmd/tui/context/sync.tsx`), `sync.ready` returning `true` when `process.env.OPENCODE_FAST_BOOT` is set is intentional. The plugin-facing data properties `state.config` (initialized to `{}`) and `state.provider` (initialized to `[]`) expose safe-empty defaults, so they are safe to access before bootstrap completes. Do not flag these as needing null-guards or conditional patterns to match `vcs` — the difference is intentional because `vcs` starts as `undefined` while the others have initialized defaults. Changing this would alter the plugin API contract without a concrete failing case.
Applied to files:
packages/app/src/pages/error-report.tspackages/app/src/pages/error.tsx
📚 Learning: 2026-04-21T13:45:41.773Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 99
File: packages/desktop-electron/src/renderer/i18n/index.ts:30-35
Timestamp: 2026-04-21T13:45:41.773Z
Learning: In Astro-Han/pawwork, the locale normalization helpers in `packages/app/src/context/language.tsx` (`normalizeLocale`) and `packages/desktop-electron/src/renderer/i18n/index.ts` (`parseLocale`) are intentionally kept separate and have different fallback shapes: `normalizeLocale` always returns a concrete `Locale` (falling back to `"en"`), while `parseLocale` returns `Locale | null` so the desktop shell can decide whether to fall back to browser detection. Do not suggest extracting a shared normalization helper across these two runtimes.
Applied to files:
packages/app/src/pages/error-report.ts
📚 Learning: 2026-04-21T16:57:22.813Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 102
File: packages/opencode/src/config/agent.ts:108-119
Timestamp: 2026-04-21T16:57:22.813Z
Learning: In `packages/opencode/src/config/agent.ts` (Astro-Han/pawwork), `ConfigPermission.Info` only accepts permission objects or the three action strings `"ask"`, `"allow"`, `"deny"`, and transforms those action strings into `{ "*": action }` before `normalize()` runs. By the time `normalize()` is reached, `configuredPermission` is always either `undefined` or a `Record<string, Rule>` — never a raw arbitrary string. The `Object.assign(permission, configuredPermission)` pattern is therefore safe. Do not flag it as corrupting string permission references.
Applied to files:
packages/app/src/pages/error-report.ts
🔇 Additional comments (25)
packages/app/src/utils/support-links.ts (1)
1-1: Good centralization of the GitHub fallback URL.Using a shared constant here is clean and prevents URL drift across renderer fallback paths.
packages/app/src/pages/layout.tsx (1)
21-21: Nice hardcoded-link removal in help action.Importing and reusing
PAWWORK_GITHUB_ISSUE_URLkeeps the sidebar help flow aligned with the shared fallback link source.Also applies to: 2428-2428
packages/desktop-electron/src/main/ipc-window-config.test.ts (1)
15-18: Good IPC contract guard for problem reporting.This test addition correctly locks in registration of the new
report-problemchannel and handler symbol.packages/desktop-electron/src/main/index.ts (1)
498-498: IPC dependency wiring looks correct.Directly delegating
reportProblemthroughregisterIpcHandlerskeeps the main-process flow consistent with existing handler registration patterns.packages/desktop-electron/src/renderer/index.tsx (1)
232-232: Renderer platform bridge extension is clean.Exposing
reportProblemthroughPlatformby delegating towindow.apikeeps the new flow aligned with the existing desktop abstraction boundary.As per coding guidelines:
packages/desktop-electron/src/**/*.{ts,tsx,js,jsx}: Renderer process should only callwindow.apifromsrc/preload.packages/desktop-electron/src/preload/index.ts (1)
85-85: Preload IPC bridge addition is correct.The new
report-probleminvoke mapping is added in the right boundary layer and matches the existing preload API pattern.packages/app/src/i18n/en.ts (1)
491-516: Error-report i18n coverage is strong.This block adds complete user-facing copy for local-state context plus all reporting result states, which improves clarity in the new renderer error flow.
packages/desktop-electron/src/preload/types.ts (1)
1-5: Typed API contract update is consistent.Adding
ReportProblemInput/ReportProblemResultand thereportProblemsignature keeps preload typings aligned with the new IPC/reporting flow end-to-end.Also applies to: 92-92
packages/desktop-electron/src/main/ipc.ts (1)
10-11: LGTM!The new
"report-problem"IPC handler is correctly registered inipc.tsfollowing the established pattern. The type imports align with the preload contract (ElectronAPI.reportProblem), and the handler properly delegates todeps.reportProblem(input). As per coding guidelines, main process IPC handlers should be registered insrc/main/ipc.ts.Also applies to: 58-58, 139-141
packages/desktop-electron/src/main/problem-report.test.ts (3)
82-112: Thorough test coverage for renderer error handling.The test correctly validates:
- Round-trip parsing preserves
rendererErrorin the payload.- Summary includes the
"Renderer error:"line with redacted storage/key identifiers.- Sensitive values (
/Users/test/project,pawwork.workspace.project.abc123.dat,workspace:vcs) are properly redacted.
297-316: Good coverage for size enforcement.The test validates that
buildProblemReporttruncatesrendererError.detailsto honormaxByteswhile preserving the summary. This ensures large renderer errors don't break the report size constraints.
482-505: Validates parser robustness against malformed payloads.The test ensures
parseProblemReportPayloadrejects JSON whererendererErroris present but lacks the requireddetailsproperty, confirming theisRendererErrorDetailsguard works correctly.packages/app/src/context/platform.tsx (2)
16-50: Well-designed discriminated union for report outcomes.The
ReportProblemResulttype clearly distinguishes all possible outcomes with appropriate fields for each status. This enables the UI to provide precise user feedback (e.g., showingfeedbackUrlwhenform-fallback, indicating copy status, etc.).
104-105: Clean Platform API extension.The optional
reportProblemmethod correctly reflects that this feature is desktop-only. The signature aligns with the preload/IPC types.packages/app/src/i18n/zh.ts (1)
478-497: Comprehensive Chinese translations for the report flow.The translations cover all user-facing strings for the new error reporting feature, including the confirmation dialog, outcome messages, and fallback guidance. The strings provide clear, actionable guidance in Chinese.
packages/app/src/pages/error-report.test.ts (1)
1-159: Solid test coverage for error-report helpers.The test file thoroughly validates:
- Known error summarization for
ChildStoreError.- Error detail extraction with cause chain preservation.
- Defensive handling of error-like objects from different execution contexts.
- Cyclic cause detection with
[Circular]marker.- Status message mapping for all
ReportProblemResultvariants.The mock translator pattern with
{{url}}substitution correctly mirrors real i18n behavior.packages/desktop-electron/src/main/problem-report.ts (3)
28-31: Past review feedback addressed:rendererErrorvalidation added.The
isRendererErrorDetailsguard (lines 416-418) properly validates bothsummaryanddetailsare strings, andisProblemReportPayload(line 441) now checksrendererErroragainst this guard. This ensuresparseProblemReportPayloadcannot return malformed payloads.Also applies to: 416-418, 441-441
304-305: Past review feedback addressed: storage/key redaction hardened.The
redactLocalPathFragmentsfunction now includes:
pawwork.workspace.*.dat→[storage]storage=.../key=...→$1=[redacted]This prevents leaking local-state storage identifiers in the copied summary.
248-265: Truncation logic follows established patterns.The iterative halving approach for
rendererError.detailsmatches the existing truncation strategy for other fields. The fallback to completely removerendererError(lines 262-265) ensures the report can always fit withinmaxBytes.packages/desktop-electron/src/main/feedback.ts (3)
302-306: Past review feedback addressed:onErrorfailure no longer rejects.The
onErrorcall is now wrapped in its own try/catch (lines 302-306), ensuring the catch block always returns the structured{ status: "failed", ... }result even ifdeps.onErrorthrows.
46-75: Clean structured result design.The
FeedbackResultdiscriminated union covers all outcome paths, and each return statement correctly constructs the appropriate variant. Thereadyvssummary-onlydistinction (lines 283-295) based onsavedReportpresence is logical.Also applies to: 283-295
178-189: Renderer error integration complete.The handler correctly:
- Accepts optional
rendererErrorinFeedbackInput.- Passes it through to both
buildProblemReportandbuildProblemReportSummary.- Supports skipping confirmation via
confirm: false.Also applies to: 221-221, 239-239
packages/desktop-electron/src/main/feedback.test.ts (3)
128-148: Good coverage for renderer-initiated reports.The test validates that:
confirm: falseskips the confirmation dialog.rendererErrorcontent appears in both the copied summary and saved markdown.- The handler returns
status: "ready"on success.
356-374: Validates defensive error handling.The test confirms that when
onErroritself throws, the handler still resolves to the structured{ status: "failed", ... }result and logs viaonHandledError. This prevents unhandled rejections from propagating to the renderer.
116-125: Tests updated for structured result assertions.All outcome tests now validate the full
FeedbackResultstructure includingstatus,summaryCopied,feedbackOpened, andfullReportfields. This ensures the contract between the handler and callers is well-tested.Also applies to: 241-248, 272-280, 346-354
Astro-Han
force-pushed
the
codex/fix-i158-error-report
branch
from
6342243 to
ba90629
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/desktop-smoke.yml:
- Around line 136-137: The Report problem smoke step is missing the
PAWWORK_FEEDBACK_FORM_URL environment variable which is currently set only on
the build step; add PAWWORK_FEEDBACK_FORM_URL:
https://example.com/pawwork-feedback to the env block of the "Report problem
smoke" step (and similarly add the same env entry where the other related
smoke/report step appears around the 146-149 area) so both steps explicitly set
the feedback URL rather than relying on script defaults.
In `@packages/desktop-electron/scripts/report-problem-smoke.mjs`:
- Around line 58-64: The temp directory created by mkdtempSync assigned to
homeDir is never removed; update the smoke-run logic that launches electron (the
block using mkdtempSync, homeDir, buildSmokeEnv and electron.launch) to delete
the homeDir in a finally block so it always cleans up even on errors; apply the
same finally-based cleanup to the other instance noted around the code that uses
homeDir at the later occurrence (the similar block around lines 103–105) to
remove the temp directory with a safe rimraf/unlink operation.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7c846b8a-941e-4647-b74a-a63af8f8f109
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/desktop-smoke.ymlpackages/desktop-electron/package.jsonpackages/desktop-electron/scripts/report-problem-smoke.mjspackages/desktop-electron/src/main/support-links.test.tspackages/desktop-electron/src/main/support-links.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
packages/desktop-electron/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Renderer process should only call
window.apifromsrc/preload
Files:
packages/desktop-electron/src/main/support-links.test.tspackages/desktop-electron/src/main/support-links.ts
🧠 Learnings (6)
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Import test utilities from `../fixtures` instead of `playwright/test`
Applied to files:
packages/desktop-electron/package.jsonpackages/desktop-electron/src/main/support-links.test.ts
📚 Learning: 2026-04-22T08:49:44.563Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:44.563Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/desktop-electron/src/main/support-links.test.tspackages/desktop-electron/scripts/report-problem-smoke.mjs
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test one feature per test file
Applied to files:
packages/desktop-electron/src/main/support-links.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : When validating routing, assert against canonical or resolved workspace slugs using shared helpers from `../actions` to account for Windows canonicalization
Applied to files:
packages/desktop-electron/src/main/support-links.test.ts
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.
Applied to files:
packages/desktop-electron/src/main/support-links.test.ts
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use `testEffect(...)` from `test/lib/effect.ts` for tests that exercise Effect services or Effect-based workflows.
Applied to files:
packages/desktop-electron/src/main/support-links.test.ts
🔇 Additional comments (5)
packages/desktop-electron/src/main/support-links.ts (1)
12-20: Good fallback + normalization flow in main-process support link resolution.This keeps precedence clear (build-time first, runtime fallback when empty) while still enforcing HTTPS normalization before export.
packages/desktop-electron/src/main/support-links.test.ts (2)
2-2: Import update is correct for the new behavior under test.Pulling in
feedbackFormUrlhere cleanly scopes tests to the new selection logic.
12-17: Nice targeted coverage for build-time/runtime URL selection behavior.The new assertions validate both the empty-build fallback path and build-time precedence path.
packages/desktop-electron/scripts/report-problem-smoke.mjs (1)
66-102: Good smoke-contract coverage for renderer error reporting path.The wait gates,
reportProbleminvocation, and clipboard/markdown assertions give a strong end-to-end signal for this flow.packages/desktop-electron/package.json (1)
17-17: Script/dependency wiring is consistent.Adding
smoke:reportalongside a pinned@playwright/testversion cleanly supports the new Electron smoke path.Also applies to: 46-46
Astro-Han
force-pushed
the
codex/fix-i158-error-report
branch
from
4e33e61 to
a0aeca7
Compare
1 participant
Summary
ChildStoreErrorwhile keeping full technical details copyable.platform.reportProblemthrough preload, renderer, IPC, and the desktop feedback handler.Why
Renderer crashes currently leave non-technical users with a stack trace and little recovery guidance. This keeps restart/update actions, adds a safer report flow, and gives maintainers the current renderer error context in the existing diagnostic report path.
Related Issue
Closes #158
How To Verify
Screenshots or Recordings
Not captured. This changes a crash boundary page and the diagnostic reporting path; the behavior is covered by focused helper, IPC, feedback, and report payload tests.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
New Features
Localization
Tests
Chores / CI