Add error-triggered feedback diagnostics#163
Merged
Merged
Conversation
Review Summary by Qodo(Agentic_describe updated until commit 585b98e)Add error-triggered feedback diagnostics with visible error handling
WalkthroughsDescription• Add error-triggered feedback diagnostics collection system capturing runtime, unhandled rejection, fetch/API, and visible-error events • Display chat-visible error banner when Codex CLI is missing with feedback action • Expose feedback mailto action in Settings after failures and directly on visible error states • Integrate feedback diagnostics into multiple error surfaces (Skills Hub, branch dropdown, live overlay, folder picker) Diagramflowchart LR
A["Error Events<br/>window/fetch/rejection"] -->|recordFeedbackDiagnostic| B["Diagnostics Store"]
C["Visible Errors<br/>CLI/API/UI"] -->|recordVisibleFailure| B
B -->|buildFeedbackMailto| D["Prefilled Email<br/>with Context"]
B -->|hasFeedbackDiagnostics| E["Settings Feedback Row"]
C -->|inline feedback| F["Error Banners<br/>with Send Feedback"]
D --> G["User Sends Report"]
E --> G
F --> G
File Changes1. src/composables/useFeedbackDiagnostics.ts
|
Code Review by Qodo
1. Secrets in feedback mailto
|
|
Persistent review updated to latest commit 585b98e |
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
friuns2
force-pushed
the
codex/error-feedback-diagnostics
branch
from
5aa5ad0 to
cdcc143
Compare
Owner
Author
|
/review |
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
Owner
Author
|
/review |
|
Persistent review updated to latest commit b6593ff |
Comment on lines
+53
to
+97
| function readVisiblePageText(): string { | ||
| if (typeof document === 'undefined') return 'unknown' | ||
| const text = document.body?.innerText | ||
| .replace(/\r\n/g, '\n') | ||
| .replace(/[ \t]+/g, ' ') | ||
| .replace(/\n{3,}/g, '\n\n') | ||
| .trim() ?? '' | ||
| if (!text) return 'No visible page text captured.' | ||
| return text | ||
| } | ||
|
|
||
| function normalizeStorageValue(value: string): string { | ||
| return value | ||
| .replace(/\r\n/g, '\n') | ||
| .replace(/[ \t]+/g, ' ') | ||
| .trim() | ||
| } | ||
|
|
||
| function readStorageSnapshot(storage: Storage | undefined, label: string): string { | ||
| if (!storage) return `${label}: unavailable` | ||
| try { | ||
| const rows: string[] = [] | ||
| for (let index = 0; index < storage.length; index += 1) { | ||
| const key = storage.key(index) | ||
| if (!key) continue | ||
| rows.push(`${key}=${normalizeStorageValue(storage.getItem(key) ?? '')}`) | ||
| } | ||
| return `${label}:\n${rows.join('\n') || 'empty'}` | ||
| } catch (error) { | ||
| return `${label}: unavailable (${normalizeSubjectMessage(normalizeMessage(error))})` | ||
| } | ||
| } | ||
|
|
||
| function readBrowserStateSnapshot(): string { | ||
| if (typeof window === 'undefined') return 'unknown' | ||
| return [ | ||
| `Path: ${window.location.pathname || '/'}`, | ||
| `Hash: ${window.location.hash || '(none)'}`, | ||
| `Search: ${window.location.search || '(none)'}`, | ||
| `Online: ${typeof navigator === 'undefined' ? 'unknown' : String(navigator.onLine)}`, | ||
| `Language: ${typeof navigator === 'undefined' ? 'unknown' : navigator.language}`, | ||
| `Platform: ${typeof navigator === 'undefined' ? 'unknown' : navigator.platform}`, | ||
| readStorageSnapshot(window.localStorage, 'localStorage'), | ||
| readStorageSnapshot(window.sessionStorage, 'sessionStorage'), | ||
| ].join('\n') |
There was a problem hiding this comment.
1. Secrets in feedback mailto 🐞 Bug ⛨ Security
buildFeedbackMailto() captures full localStorage/sessionStorage contents and visible page text and
encodes them into the mailto body, which can expose auth tokens/PII in the email draft and directly
in rendered link href attributes. The test suite explicitly verifies a token-like key
("codex-token") is included, confirming the leakage path.
Agent Prompt
### Issue description
`buildFeedbackMailto()` currently includes **all** `localStorage`/`sessionStorage` key-values and `document.body.innerText` in the mailto body. This can leak secrets (tokens) and PII into the DOM (`href`) and into user email drafts.
### Issue Context
- Storage snapshots are iterated without filtering/redaction.
- Visible page text capture can include user content.
- Multiple components bind `:href="feedbackMailto"`, so the full encoded payload exists in the DOM even before a click.
- Tests currently assert token inclusion, so they must be updated after redaction.
### Fix Focus Areas
- src/composables/useFeedbackDiagnostics.ts[53-168]
- src/composables/useFeedbackDiagnostics.test.ts[1-90]
- src/components/content/SkillsHub.vue[28-38]
- src/components/content/ThreadConversation.vue[700-704]
### Suggested changes
1. **Redact storage values**:
- Prefer allowlisting safe keys/prefixes (e.g., `codex-web-local.`), and/or
- Forbidlist common secret patterns in keys (`token`, `auth`, `secret`, `key`, `session`) and replace values with `[REDACTED]`.
- Consider emitting only key names + value length instead of full values.
2. **Reduce DOM exposure**:
- Avoid binding full diagnostics mailto to `href` reactively; use a minimal `href="mailto:..."` and set the full href only in the click handler right before navigation.
3. Update tests to assert redaction behavior (e.g., `codex-token=[REDACTED]` or that the key is omitted).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
2 participants
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
Verification