mirror of
https://github.com/stablyai/orca.git
synced 2026-09-26 08:02:38 +00:00
* feat(feedback): attach images to feedback submissions
Users pasting screenshots into the feedback dialog were silently dropped:
the textarea had no paste handler, the IPC payload had no image field, and
the endpoint had nowhere to put one. Reports arrived saying "images
attached" with nothing attached, which is why feedback-sourced tickets
never have a screenshot to work from.
Adds paste, drag-drop, and a file picker with thumbnail previews (up to 4
images, 8 MB each, png/jpeg/webp/gif). Rejected files raise a toast rather
than disappearing — silent loss is the bug being fixed.
Images ride the existing multipart lane, which previously activated only
for crash diagnostic bundles. Crash submissions still drop images; that
lane already carries bundles and the server rejects them there.
When the server reports imagesDelivered: false the dialog says the
feedback sent but the images did not, instead of a blanket success. A 2xx
without the field counts as delivered so this keeps working against a
server that predates the field.
Requires the marketing-site half to deploy first.
* copy(feedback): shorten attachment hint to 'Attach up to 4 screenshots'
* fix(feedback): make dropped screenshots actually attach
Three defects that discarded a user's image without telling them — the exact
failure this feature exists to fix.
Drag-and-drop never worked. `DataTransfer.files` is empty until the drop
lands, so the dragenter guard always saw zero files and the highlight never
armed. Worse, preload consumes native file drops on document capture with
`stopPropagation()` and routes the paths to the editor, so React's `onDrop`
never ran at all: dropping a screenshot on the dialog opened it in an editor
behind the modal. The drop is now claimed one phase earlier on window capture
and scoped to the dialog element, and the highlight keys off the drag types
the OS advertises — matching useComposerFileDragOver and useSidebarProjectDrop.
`crypto.randomUUID()` is undefined in non-secure browser contexts (the LAN web
client over plain HTTP), so building draft ids with it rejected the read and
dropped every image in the batch with no message and an unhandled rejection.
Use createBrowserUuid, the repo's fallback for exactly this.
`readFeedbackImageFiles` had no rejection handler, so any read failure (file
removed after picking, permission error) silently lost the whole batch.
Also: capacity was checked against a ref mirroring committed state, so two
pastes landing during an in-flight read both saw room for four and the main
process then rejected the entire submission; in-flight batches now count
against capacity. And the non-en catalogs still carried the pre-amendment
English copy for the attachment hint.
* fix(feedback): close the prototype-chain hole in the image allow-list
`contentType in FEEDBACK_IMAGE_EXTENSIONS` walks the prototype chain, so
"constructor", "__proto__", "toString", "valueOf" and "hasOwnProperty" all
cleared the allow-list. feedbackImageFilename then indexed the same object and
named the upload after the inherited value — "feedback-image-1.function
Object() { [native code] }" — and the part went out with that content type.
Only reachable by invoking feedback:submit directly (the renderer screens
types with Array.includes), which is exactly the threat model this function's
own doc comment claims to cover. Object.hasOwn matches the 54 other uses in
the repo and is identical for the four real types.
The inherited values carry no quotes or CRLF, so this was a bypassed allow-list
and a malformed upload, not multipart header injection.
Adds unit coverage for the module, which had none, plus an IPC-level case; all
six new assertions fail against `in`.
* fix(feedback): accept the drag on dragover so the drop can fire
The window-capture drop interception only fires if something first
preventDefaults `dragover`. In Electron that comes free from preload's
document-capture handler, but the same renderer is served to browsers as
web-index.html, where `installWebPreloadApi` builds `window.api` in JS and
installs no drag listeners at all. Nothing else in the renderer
preventDefaults dragover for a native file drag.
So on the web client the dialog is not a valid drop target: `drop` never
fires and the browser falls back to its default action for a file dropped
on a page — it navigates the tab to the file, taking the user's typed
feedback with it. The new types-based dragenter guard makes this worse
than before, because the highlight now arms and invites the drop that the
old `files`-based guard could never light up.
Mirrors useSidebarProjectDrop.onDragOver, which the drop rework already
claimed to match. In Electron it is a harmless duplicate of the
preventDefault preload already applied.
* fix(feedback): revoke batch previews when a read rejects partway
readFeedbackImageFiles creates the object URL for each accepted file as it
goes. If a later file in the same batch fails `arrayBuffer()` — the
removed-after-picking case the new rejection handler was added for — the
whole promise rejects and the already-built drafts are never returned, so
nothing ever revokes their previews.
Each leaked URL pins its blob for the life of the renderer, up to three at
8 MB. Release them before rethrowing; the caller's rejection handler is
unaffected.
* fix(feedback): cancel non-image drops the dialog already accepted
dragover advertises copy for every native file drag over the dialog, but
drop only cancelled for images. On the web client an uncancelled drop
navigates the tab to the file, taking the typed feedback with it.
* fix(feedback): stop image validation from aborting crash reports
buildSubmitBody drops images on the crash lane, but validation ran
unconditionally, so a crash submission carrying an invalid image would
have failed outright over attachments that were never going to be sent —
losing a crash report the user needs delivered. Gate validation the same
way body construction is gated.
Not reachable today (the IPC handler forces submissionType 'feedback' and
internal crash callers pass no images), but the two gates disagreeing is a
trap for the next caller. Raised by CodeRabbit.
Also documents why the image lane deliberately skips the 5xx retry the
text lane performs: replaying up to 32 MiB on a flaky link costs more than
it saves, and the dialog preserves the draft and thumbnails on failure.
* fix(feedback): stop mutating the image-count ref during render
React Doctor fails CI on "Ref mutated during render": the count was
assigned in the component body, where React can discard or replay work
that never commits.
Read the committed count from the callback closure instead of a ref.
Syncing the ref in an effect (the suggested fix) would reintroduce the
race a previous commit removed — right after an add, the ref is stale-low
until the effect flushes, so a paste in that window over-accepts and the
main process rejects the whole submission. The closure value is always the
committed count, and pendingImageReadsRef still covers in-flight reads.
Costs a re-registration of the drop listeners per attach, which is the
same teardown the hook already does when the dialog opens or closes.
* fix(feedback): stop an unsupported pasted image from eating co-pasted text
The paste handler consumed the event whenever the clipboard held any
image/* file, but only the four allow-listed types can actually attach.
Pasting text alongside an SVG or BMP therefore lost the text and attached
nothing — a silent loss of the user's own input, in the dialog where they
are mid-sentence.
Consume the paste only when something is attachable. Unsupported types
still route through readFeedbackImageFiles for their rejection toast, so
nothing is dropped silently; the difference is that the default paste is
left alone when we have nothing to offer in exchange.
Extraction deliberately stays broad. Narrowing it there (as suggested by
review) would skip handleAddFiles entirely, and a file paste into a
textarea does nothing visible — the image would vanish with no feedback.
The drop path is untouched: it must keep cancelling every native file drop
or the browser navigates the tab to the file.
* fix(feedback): stop the dialog accepting more than the endpoint will take
The endpoint rejects reports over 5000 characters with a 400, which the
dialog surfaces as a generic "Failed to submit feedback. Please try again."
Nothing said length was the problem, so retrying could not help — the draft
survived but the user had no way to know what to change.
Cap the textarea at the same 5000 and show a counter once 500 characters
remain, so the limit is visible before it bites rather than after. The
counter stays hidden until then; an always-on count reads as a word limit
to hit.
Extracted rather than inlined: the dialog is already past the 300-line mark
React Doctor warns on.
* fix(feedback): prevent silent attachment loss
* fix(feedback): improve attachment failure feedback
* fix(feedback): bound attachment response parsing
* fix(feedback): surface response body timeouts
* fix(feedback): harden image delivery
* fix(feedback): bound image preview resources
* fix(feedback): honor atomic image delivery response
Production’s single-message feedback endpoint uploads text and images atomically, then returns 202 {"ok":true} without an imagesDelivered field. Treating that omission as false warned users that every successful production attachment had failed.
Treat a settled successful JSON response with ok: true and no image field as delivered. Explicit imagesDelivered: false still surfaces partial delivery, while malformed, oversized, aborted, and stalled bodies remain unconfirmed or fail through the existing response bound and timeout path.
362 lines
13 KiB
TypeScript
362 lines
13 KiB
TypeScript
import os from 'node:os'
|
|
import { app, ipcMain, net } from 'electron'
|
|
import {
|
|
appendFeedbackImagesToFormData,
|
|
readFeedbackImagesDelivered,
|
|
validateFeedbackImages,
|
|
type FeedbackImageAttachment
|
|
} from './feedback-image-attachments'
|
|
|
|
export type { FeedbackImageAttachment }
|
|
|
|
// Why: the production Mac build loads the renderer from a file:// origin, so a
|
|
// cross-origin POST from fetch() triggers a CORS preflight that the feedback
|
|
// endpoint rejects. Electron's net module runs in the main process and is not
|
|
// subject to CORS, so we proxy the submission through IPC. This mirrors the
|
|
// same pattern used by updater-changelog.ts and updater-nudge.ts.
|
|
const FEEDBACK_API_URL = 'https://www.onorca.dev/v1/feedback'
|
|
const FEEDBACK_REQUEST_TIMEOUT_MS = 10_000
|
|
const FEEDBACK_ATTACHMENT_REQUEST_TIMEOUT_MS = 60_000
|
|
const DIAGNOSTIC_BUNDLE_CONTENT_TYPE = 'application/x-ndjson'
|
|
// Why: corporate filters can reject multipart with 403 while allowing the
|
|
// small JSON report, so content-shaped failures should shed the attachment.
|
|
const DIAGNOSTIC_BUNDLE_JSON_RETRY_STATUSES = new Set([400, 403, 408, 413, 415, 422])
|
|
|
|
export type FeedbackSubmissionType = 'feedback' | 'crash'
|
|
|
|
export type FeedbackSubmitArgs = {
|
|
feedback: string
|
|
submitAnonymously?: boolean
|
|
githubLogin: string | null
|
|
githubEmail: string | null
|
|
images?: FeedbackImageAttachment[]
|
|
}
|
|
|
|
export type FeedbackDiagnosticBundleAttachment = {
|
|
bundleSubmissionId: string
|
|
content: string
|
|
bytes: number
|
|
spanCount: number
|
|
}
|
|
|
|
type FeedbackSubmitBody = {
|
|
feedback: string
|
|
submissionType: FeedbackSubmissionType
|
|
githubLogin: string | null
|
|
githubEmail: string | null
|
|
appVersion: string
|
|
platform: NodeJS.Platform
|
|
osRelease: string
|
|
arch: string
|
|
diagnosticBundle?: FeedbackDiagnosticBundleAttachment
|
|
images?: FeedbackImageAttachment[]
|
|
}
|
|
|
|
export type FeedbackRequestFailure = {
|
|
status: number | null
|
|
error: string
|
|
}
|
|
|
|
export type FeedbackSubmitResult =
|
|
| {
|
|
ok: true
|
|
diagnosticBundleFailure?: FeedbackRequestFailure
|
|
/** Absent when nothing was attached; false when the text landed but the images did not. */
|
|
imagesDelivered?: boolean
|
|
}
|
|
| ({ ok: false } & FeedbackRequestFailure & {
|
|
diagnosticBundleFailure?: FeedbackRequestFailure
|
|
})
|
|
|
|
type InternalFeedbackSubmitArgs = FeedbackSubmitArgs & {
|
|
submissionType?: FeedbackSubmissionType
|
|
diagnosticBundle?: FeedbackDiagnosticBundleAttachment
|
|
feedbackWithoutDiagnosticBundle?: string
|
|
}
|
|
|
|
// Why: the Slack notification and any follow-up investigation need to know
|
|
// which Orca build and which OS the feedback came from. The main process is
|
|
// the only place with trusted access to these values (app.getVersion and the
|
|
// node os module), so we enrich the payload here rather than trusting the
|
|
// renderer.
|
|
function buildSubmitBody(args: InternalFeedbackSubmitArgs): FeedbackSubmitBody {
|
|
const identity = args.submitAnonymously
|
|
? { githubLogin: null, githubEmail: null }
|
|
: { githubLogin: args.githubLogin, githubEmail: args.githubEmail }
|
|
|
|
// Why: anonymity is an IPC-only privacy decision. Allow-list fields here so
|
|
// stale renderer state or future identity-shaped fields cannot leak upstream.
|
|
return {
|
|
feedback: args.feedback,
|
|
submissionType: args.submissionType ?? 'feedback',
|
|
...identity,
|
|
appVersion: app.getVersion(),
|
|
platform: process.platform,
|
|
osRelease: os.release(),
|
|
arch: process.arch,
|
|
...(args.submissionType === 'crash' && args.diagnosticBundle
|
|
? { diagnosticBundle: args.diagnosticBundle }
|
|
: {}),
|
|
// Why: images are a feedback-only affordance; crash reports already carry
|
|
// diagnostic bundles and the server rejects images on that lane.
|
|
...(args.submissionType !== 'crash' && args.images?.length ? { images: args.images } : {})
|
|
}
|
|
}
|
|
|
|
async function postFeedback(
|
|
url: string,
|
|
body: FeedbackSubmitBody,
|
|
timeoutMs = FEEDBACK_REQUEST_TIMEOUT_MS,
|
|
readResponse?: (response: Response) => Promise<void>
|
|
): Promise<Response> {
|
|
const controller = new AbortController()
|
|
// Why: a silent endpoint must not leave feedback IPC pending forever.
|
|
const timeout = setTimeout(() => controller.abort(), timeoutMs)
|
|
try {
|
|
const init: RequestInit = {
|
|
method: 'POST',
|
|
...feedbackRequestBodyInit(body),
|
|
signal: controller.signal
|
|
}
|
|
const response = await net.fetch(url, init)
|
|
if (readResponse) {
|
|
await readResponse(response)
|
|
}
|
|
// Why: a response parser may tolerate malformed legacy bodies, but it must
|
|
// not turn the deadline's aborted body into a confirmed delivery.
|
|
if (controller.signal.aborted) {
|
|
throw new Error(`request timed out after ${timeoutMs / 1000} seconds`)
|
|
}
|
|
return response
|
|
} catch (error) {
|
|
// Why: Electron and Node report AbortError differently; keep deadline logs stable.
|
|
if (controller.signal.aborted) {
|
|
throw new Error(`request timed out after ${timeoutMs / 1000} seconds`)
|
|
}
|
|
throw error
|
|
} finally {
|
|
clearTimeout(timeout)
|
|
}
|
|
}
|
|
|
|
function feedbackRequestBodyInit(body: FeedbackSubmitBody): Pick<RequestInit, 'body' | 'headers'> {
|
|
if (!body.diagnosticBundle && !body.images?.length) {
|
|
return {
|
|
headers: { 'Content-Type': 'application/json' },
|
|
body: JSON.stringify(body)
|
|
}
|
|
}
|
|
|
|
const formData = new FormData()
|
|
appendFeedbackFormField(formData, 'feedback', body.feedback)
|
|
appendFeedbackFormField(formData, 'submissionType', body.submissionType)
|
|
appendFeedbackFormField(formData, 'githubLogin', body.githubLogin)
|
|
appendFeedbackFormField(formData, 'githubEmail', body.githubEmail)
|
|
appendFeedbackFormField(formData, 'appVersion', body.appVersion)
|
|
appendFeedbackFormField(formData, 'platform', body.platform)
|
|
appendFeedbackFormField(formData, 'osRelease', body.osRelease)
|
|
appendFeedbackFormField(formData, 'arch', body.arch)
|
|
if (body.diagnosticBundle) {
|
|
appendFeedbackFormField(
|
|
formData,
|
|
'diagnosticBundleSubmissionId',
|
|
body.diagnosticBundle.bundleSubmissionId
|
|
)
|
|
appendFeedbackFormField(formData, 'diagnosticBundleBytes', String(body.diagnosticBundle.bytes))
|
|
appendFeedbackFormField(
|
|
formData,
|
|
'diagnosticBundleSpanCount',
|
|
String(body.diagnosticBundle.spanCount)
|
|
)
|
|
formData.append(
|
|
'diagnosticBundleFile',
|
|
new Blob([body.diagnosticBundle.content], { type: DIAGNOSTIC_BUNDLE_CONTENT_TYPE }),
|
|
`orca-diagnostics-${body.diagnosticBundle.bundleSubmissionId}.ndjson`
|
|
)
|
|
}
|
|
appendFeedbackImagesToFormData(formData, body.images ?? [])
|
|
|
|
// Why: multipart avoids JSON-escaping a near-cap NDJSON bundle over the
|
|
// backend request limit while still submitting one feedback request.
|
|
return { body: formData }
|
|
}
|
|
|
|
function appendFeedbackFormField(formData: FormData, key: string, value: string | null): void {
|
|
if (value !== null) {
|
|
formData.append(key, value)
|
|
}
|
|
}
|
|
|
|
function messageFromError(error: unknown): string {
|
|
return error instanceof Error ? error.message : String(error)
|
|
}
|
|
|
|
function responseFailure(response: Response): FeedbackRequestFailure {
|
|
return { status: response.status, error: `status ${response.status}` }
|
|
}
|
|
|
|
function errorFailure(error: unknown): FeedbackRequestFailure {
|
|
return { status: null, error: messageFromError(error) }
|
|
}
|
|
|
|
async function retryFeedbackOnPrimary(
|
|
body: FeedbackSubmitBody,
|
|
primaryError?: unknown
|
|
): Promise<FeedbackSubmitResult> {
|
|
try {
|
|
const retry = await postFeedback(FEEDBACK_API_URL, body)
|
|
if (retry.ok) {
|
|
return { ok: true }
|
|
}
|
|
const retryMessage = `status ${retry.status}`
|
|
if (primaryError === undefined) {
|
|
return { ok: false, status: retry.status, error: retryMessage }
|
|
}
|
|
// Why: keep the first failure visible so support can see 5xx → retry outcome,
|
|
// not only the last error in a same-host retry chain.
|
|
return {
|
|
ok: false,
|
|
status: retry.status,
|
|
error: `${messageFromError(primaryError)}; retry: ${retryMessage}`
|
|
}
|
|
} catch (retryError) {
|
|
const message = messageFromError(retryError)
|
|
if (primaryError === undefined) {
|
|
return { ok: false, status: null, error: message }
|
|
}
|
|
return {
|
|
ok: false,
|
|
status: null,
|
|
error: `${messageFromError(primaryError)}; retry: ${message}`
|
|
}
|
|
}
|
|
}
|
|
|
|
function shouldRetryWithoutDiagnosticBundle(status: number): boolean {
|
|
return DIAGNOSTIC_BUNDLE_JSON_RETRY_STATUSES.has(status) || status === 404 || status >= 500
|
|
}
|
|
|
|
async function submitFeedbackWithoutDiagnosticBundle(
|
|
body: FeedbackSubmitBody,
|
|
diagnosticBundleFailure: FeedbackRequestFailure
|
|
): Promise<FeedbackSubmitResult> {
|
|
try {
|
|
const response = await postFeedback(FEEDBACK_API_URL, body)
|
|
if (response.ok) {
|
|
return { ok: true, diagnosticBundleFailure }
|
|
}
|
|
return { ok: false, ...responseFailure(response), diagnosticBundleFailure }
|
|
} catch (error) {
|
|
return { ok: false, ...errorFailure(error), diagnosticBundleFailure }
|
|
}
|
|
}
|
|
|
|
async function submitFeedbackWithDiagnosticBundle(
|
|
body: FeedbackSubmitBody,
|
|
bodyWithoutDiagnosticBundle: FeedbackSubmitBody | null
|
|
): Promise<FeedbackSubmitResult> {
|
|
try {
|
|
// Why: diagnostic bundles can approach 4 MiB and need more upload time than
|
|
// the small JSON report-only path, especially on constrained connections.
|
|
const response = await postFeedback(
|
|
FEEDBACK_API_URL,
|
|
body,
|
|
FEEDBACK_ATTACHMENT_REQUEST_TIMEOUT_MS
|
|
)
|
|
if (response.ok) {
|
|
return { ok: true }
|
|
}
|
|
const failure = responseFailure(response)
|
|
if (bodyWithoutDiagnosticBundle && shouldRetryWithoutDiagnosticBundle(response.status)) {
|
|
return submitFeedbackWithoutDiagnosticBundle(bodyWithoutDiagnosticBundle, failure)
|
|
}
|
|
return { ok: false, ...failure }
|
|
} catch (error) {
|
|
const failure = errorFailure(error)
|
|
return bodyWithoutDiagnosticBundle
|
|
? submitFeedbackWithoutDiagnosticBundle(bodyWithoutDiagnosticBundle, failure)
|
|
: { ok: false, ...failure }
|
|
}
|
|
}
|
|
|
|
export async function submitFeedback(
|
|
args: InternalFeedbackSubmitArgs
|
|
): Promise<FeedbackSubmitResult> {
|
|
// Why: buildSubmitBody drops images on the crash lane, so validating them
|
|
// there would abort a crash report over attachments it never meant to send.
|
|
if (args.submissionType !== 'crash' && args.images !== undefined) {
|
|
const imageError = validateFeedbackImages(args.images)
|
|
if (imageError) {
|
|
return { ok: false, status: null, error: imageError }
|
|
}
|
|
}
|
|
const body = buildSubmitBody(args)
|
|
if (body.images?.length) {
|
|
try {
|
|
let imagesDelivered = true
|
|
const response = await postFeedback(
|
|
FEEDBACK_API_URL,
|
|
body,
|
|
FEEDBACK_ATTACHMENT_REQUEST_TIMEOUT_MS,
|
|
async (nextResponse) => {
|
|
imagesDelivered = nextResponse.ok ? await readFeedbackImagesDelivered(nextResponse) : true
|
|
}
|
|
)
|
|
if (response.ok) {
|
|
return { ok: true, imagesDelivered }
|
|
}
|
|
// Why: the text lane retries 5xx, this one does not. Replaying up to
|
|
// 32 MiB of attachments on a flaky link costs more than it saves, and the
|
|
// dialog keeps the draft and thumbnails so the user can resend.
|
|
return { ok: false, ...responseFailure(response) }
|
|
} catch (error) {
|
|
return { ok: false, ...errorFailure(error) }
|
|
}
|
|
}
|
|
if (body.diagnosticBundle) {
|
|
const bodyWithoutDiagnosticBundle =
|
|
args.feedbackWithoutDiagnosticBundle !== undefined
|
|
? buildSubmitBody({
|
|
...args,
|
|
feedback: args.feedbackWithoutDiagnosticBundle,
|
|
diagnosticBundle: undefined
|
|
})
|
|
: null
|
|
return submitFeedbackWithDiagnosticBundle(body, bodyWithoutDiagnosticBundle)
|
|
}
|
|
try {
|
|
const res = await postFeedback(FEEDBACK_API_URL, body)
|
|
if (res.ok) {
|
|
return { ok: true }
|
|
}
|
|
// Why: api.onorca.dev serves a different product, so transient failures
|
|
// retry the endpoint that owns feedback and crash delivery.
|
|
if (res.status >= 500) {
|
|
return retryFeedbackOnPrimary(body, new Error(`status ${res.status}`))
|
|
}
|
|
return { ok: false, status: res.status, error: `status ${res.status}` }
|
|
} catch (error) {
|
|
return retryFeedbackOnPrimary(body, error)
|
|
}
|
|
}
|
|
|
|
export function registerFeedbackHandlers(): void {
|
|
ipcMain.removeHandler('feedback:submit')
|
|
ipcMain.handle('feedback:submit', (_event, args: FeedbackSubmitArgs) => {
|
|
// Why: validate the raw clone before normalization so a tiny hostile value
|
|
// cannot become a large main-process typed-array allocation.
|
|
if (args.images !== undefined) {
|
|
const imageError = validateFeedbackImages(args.images)
|
|
if (imageError) {
|
|
return { ok: false, status: null, error: imageError }
|
|
}
|
|
}
|
|
// Why: crash submissions are main-only. A compromised renderer can invoke
|
|
// this channel directly, so force the public feedback lane at the boundary.
|
|
return submitFeedback({
|
|
...args,
|
|
submissionType: 'feedback'
|
|
})
|
|
})
|
|
}
|