Skip to content

Commit 63ebabf

Browse files
committed
Error reports: send in dev with [DEV] tag forwarded to Discord
- Drop the `cfg!(debug_assertions)` half of the skip in `error_reporter::upload`. CI still skips so test runs don't pollute the live channel, but debug builds now upload normally — manual "Send error report" works in dev. - Add `BuildMode` (`release`/`debug`) and a `buildMode` field on `BundleManifest`. Resolved at compile time from `cfg!(debug_assertions)` via `BuildMode::current()`. Tests cover serialization and the `current()` mapping. - API server reads `meta.buildMode` and forwards it to Discord. The webhook embed title is prefixed `[DEV]` for debug-build reports so triage can tell dev runs apart from production traffic. Field is optional and defaults to `release` for older clients. - Drop the dev-disable behavior on the Send button in `ErrorReportDialog.svelte`: removes the `sendDisabledInDev`/`sendTooltip` constants, the `use:tooltip` wrapper, the unused `tooltip` import, and the `&& !sendDisabledInDev` clause in `handleKeydown`. - Revert the `vi.stubEnv('DEV', false)` plumbing in `ErrorReportDialog.a11y.test.ts` — no longer needed. - Update `error_reporter/CLAUDE.md`: only CI is bypassed, document the new `buildMode` manifest field and what it's for.
1 parent 0debff1 commit 63ebabf

8 files changed

Lines changed: 130 additions & 46 deletions

File tree

apps/api-server/src/discord.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
const baseNotification: ErrorReportNotification = {
1212
id: 'ERR-A2345',
1313
kind: 'user',
14+
buildMode: 'release',
1415
appVersion: '0.13.0',
1516
osVersion: '15.3.1',
1617
arch: 'aarch64',
@@ -97,6 +98,18 @@ describe('buildErrorReportPayload', () => {
9798
}
9899
expect(payload.embeds[0].fields.find((f) => f.name === 'User note')).toBeUndefined()
99100
})
101+
102+
it('prefixes the title with [DEV] when buildMode is debug', () => {
103+
const payload = buildErrorReportPayload({ ...baseNotification, buildMode: 'debug' }) as {
104+
embeds: { title: string }[]
105+
}
106+
expect(payload.embeds[0].title).toBe('[DEV] Error report ERR-A2345')
107+
})
108+
109+
it('does not prefix the title when buildMode is release', () => {
110+
const payload = buildErrorReportPayload(baseNotification) as { embeds: { title: string }[] }
111+
expect(payload.embeds[0].title).toBe('Error report ERR-A2345')
112+
})
100113
})
101114

102115
describe('buildEvictionPayload', () => {

apps/api-server/src/discord.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@
99
export interface ErrorReportNotification {
1010
id: string
1111
kind: 'user' | 'auto'
12+
/**
13+
* Forwarded from the manifest. `'debug'` reports come from a dev build of the
14+
* desktop app (`cfg!(debug_assertions)`), and we prefix the embed title with
15+
* `[DEV]` so triage can tell them apart from production traffic at a glance.
16+
* Defaults to `'release'` upstream when older clients don't set it.
17+
*/
18+
buildMode: 'release' | 'debug'
1219
appVersion: string
1320
osVersion: string
1421
arch: string
@@ -60,10 +67,11 @@ export function buildErrorReportPayload(n: ErrorReportNotification): unknown {
6067
fields.push({ name: 'User note', value: truncatedNote })
6168
}
6269

70+
const titlePrefix = n.buildMode === 'debug' ? '[DEV] ' : ''
6371
return {
6472
embeds: [
6573
{
66-
title: `Error report ${n.id}`,
74+
title: `${titlePrefix}Error report ${n.id}`,
6775
color: ERROR_REPORT_EMBED_COLOR,
6876
fields,
6977
},

apps/api-server/src/error-report.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ const DEFAULT_BUCKET_NAME = 'cmdr-error-reports'
2222

2323
export interface ErrorReportMeta {
2424
kind: 'user' | 'auto'
25+
/**
26+
* Set by the desktop client from `cfg!(debug_assertions)`. `'debug'` reports
27+
* come from a dev build of the app; the Discord notification gets a `[DEV]`
28+
* prefix so triage can keep them apart from production traffic. Optional for
29+
* backwards compatibility with older clients that didn't set it — unset is
30+
* treated as `'release'`.
31+
*/
32+
buildMode?: 'release' | 'debug'
2533
appVersion: string
2634
osVersion: string
2735
arch: string
@@ -38,6 +46,7 @@ function isValidMeta(value: unknown): value is ErrorReportMeta {
3846
if (typeof val !== 'string' || val.length === 0) return false
3947
}
4048
if (v['userNote'] !== undefined && typeof v['userNote'] !== 'string') return false
49+
if (v['buildMode'] !== undefined && v['buildMode'] !== 'release' && v['buildMode'] !== 'debug') return false
4150
return true
4251
}
4352

@@ -128,6 +137,7 @@ async function postUploadWork(
128137
await postErrorReportNotification(env.DISCORD_WEBHOOK_URL, {
129138
id: args.id,
130139
kind: args.meta.kind,
140+
buildMode: args.meta.buildMode ?? 'release',
131141
appVersion: args.meta.appVersion,
132142
osVersion: args.meta.osVersion,
133143
arch: args.meta.arch,

apps/desktop/src-tauri/src/error_reporter/CLAUDE.md

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ Manifest fields (`BundleManifest`):
2727
- `id`: short ID (`ERR-XXXXX`) generated client-side. **Server may regenerate** — UI
2828
always shows the server's response ID, never the local one.
2929
- `kind`: `"user"` or `"auto"` (Phase 5).
30+
- `buildMode`: `"release"` or `"debug"`. Resolved at compile time from
31+
`cfg!(debug_assertions)` via `BuildMode::current()`. Forwarded to the api server so the
32+
Discord notification embed prefixes the title with `[DEV]` for debug-build reports —
33+
triage can keep dev-run reports apart from real production traffic at a glance.
3034
- `appVersion`, `osVersion`, `arch`: build/platform identifiers.
3135
- `activeSettings`: settings snapshot via the `ResolvedSettings` struct — every field
3236
resolved to its effective value (`null` is never shipped). Includes `indexingEnabled`,
@@ -94,11 +98,17 @@ dialog. Re-building is cheap (the heavy work is reading + redacting log lines, w
9498
runs on the blocking pool either way), and the inputs are deterministic enough that the
9599
preview hash matches what'll be uploaded.
96100

97-
## Dev/CI bypass
101+
## CI bypass
98102

99-
[`upload`] checks `cfg!(debug_assertions) || std::env::var("CI").is_ok()`. In either
100-
case it returns the locally-generated ID without calling the network. Mirrors the crash
101-
reporter's bypass — same reasoning (no production data pollution from dev runs).
103+
[`upload`] checks `std::env::var("CI").is_ok()` and short-circuits on a hit, returning
104+
the locally-generated ID without calling the network. CI runs shouldn't pollute the
105+
live error-report channel even if a test triggers a report.
106+
107+
Debug builds **do** upload — that's the point of "Send error report" working in dev. The
108+
manifest carries `buildMode: "debug"`, which the api server reads to prefix the Discord
109+
embed title with `[DEV]` so triage can separate dev-run reports from production traffic
110+
at a glance. (Pre-fix-* `upload()` skipped the network in `cfg!(debug_assertions)` too,
111+
which made dev-mode "Send report" silently no-op — confusing and unhelpful.)
102112

103113
The dialog has an extra "Save bundle to disk (debug)" button in dev that calls
104114
`save_error_report_to_disk` instead, writing the zip to the app data dir for inspection.

apps/desktop/src-tauri/src/error_reporter/mod.rs

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,27 @@ pub enum BundleKind {
116116
Auto,
117117
}
118118

119+
/// Whether this bundle was built by a release or a debug build of the desktop app.
120+
/// Forwarded to the api server in the manifest so triage can tell dev-run reports
121+
/// (which the api server tags `[DEV]` in Discord) apart from production reports.
122+
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
123+
#[serde(rename_all = "lowercase")]
124+
pub enum BuildMode {
125+
Release,
126+
Debug,
127+
}
128+
129+
impl BuildMode {
130+
/// Resolved at compile time from `cfg!(debug_assertions)`.
131+
pub fn current() -> Self {
132+
if cfg!(debug_assertions) {
133+
BuildMode::Debug
134+
} else {
135+
BuildMode::Release
136+
}
137+
}
138+
}
139+
119140
/// Time filter applied when picking which log content to include.
120141
///
121142
/// - `Last24Hours`: include log files whose mtime is within the last 24 hours. Used by
@@ -233,6 +254,9 @@ impl ResolvedSettings {
233254
pub struct BundleManifest {
234255
pub id: String,
235256
pub kind: BundleKind,
257+
/// Release vs debug build of the desktop app. Lets the api server tag dev-run
258+
/// reports so triage can keep them separate from production traffic.
259+
pub build_mode: BuildMode,
236260
pub app_version: String,
237261
pub os_version: String,
238262
pub arch: String,
@@ -357,6 +381,7 @@ pub fn build_bundle<R: tauri::Runtime>(
357381
let manifest = BundleManifest {
358382
id: id.clone(),
359383
kind,
384+
build_mode: BuildMode::current(),
360385
app_version: env!("CARGO_PKG_VERSION").to_string(),
361386
os_version: get_os_version(),
362387
arch: std::env::consts::ARCH.to_string(),
@@ -552,14 +577,17 @@ pub fn generate_short_id() -> String {
552577
out
553578
}
554579

555-
/// POST the bundle to the ingestion server. In dev/CI this skips the network call and
556-
/// synthesizes a response using the locally generated ID.
580+
/// POST the bundle to the ingestion server. In CI this skips the network call and
581+
/// synthesizes a response using the locally generated ID — CI runs shouldn't pollute
582+
/// the live error-report channel even if a test triggers a report. Debug builds DO
583+
/// upload; the manifest's `buildMode: "debug"` field lets the server tag those
584+
/// reports `[DEV]` so triage can separate them from production traffic.
557585
pub async fn upload(zip_bytes: Vec<u8>, manifest: &BundleManifest, server_url: &str) -> Result<UploadResult, String> {
558-
let should_skip = cfg!(debug_assertions) || std::env::var("CI").is_ok();
586+
let should_skip = std::env::var("CI").is_ok();
559587
if should_skip {
560588
log::info!(
561589
target: "cmdr_lib::error_reporter",
562-
"Skipping error report upload (dev mode or CI). Local ID: {}",
590+
"Skipping error report upload (CI). Local ID: {}",
563591
manifest.id,
564592
);
565593
return Ok(UploadResult {

apps/desktop/src-tauri/src/error_reporter/tests.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ fn sample_manifest() -> BundleManifest {
77
BundleManifest {
88
id: "ERR-AB23X".to_string(),
99
kind: BundleKind::User,
10+
build_mode: BuildMode::Release,
1011
app_version: "0.0.0-test".to_string(),
1112
os_version: "macOS test".to_string(),
1213
arch: "aarch64".to_string(),
@@ -134,6 +135,48 @@ fn redaction_is_applied_to_log_lines() {
134135
);
135136
}
136137

138+
#[test]
139+
fn build_mode_serializes_lowercase() {
140+
let release = serde_json::to_string(&BuildMode::Release).unwrap();
141+
assert_eq!(release, "\"release\"");
142+
let debug = serde_json::to_string(&BuildMode::Debug).unwrap();
143+
assert_eq!(debug, "\"debug\"");
144+
}
145+
146+
#[test]
147+
fn manifest_serializes_build_mode_with_camel_case_key() {
148+
let now = SystemTime::now();
149+
let manifest = sample_manifest();
150+
let bytes = build_zip(&manifest, &BTreeMap::new(), now).unwrap();
151+
let entries = read_zip_entries(&bytes);
152+
let manifest_json = entries.get("manifest.json").unwrap();
153+
assert!(
154+
manifest_json.contains("\"buildMode\": \"release\""),
155+
"expected `\"buildMode\": \"release\"` in manifest JSON, got: {manifest_json}",
156+
);
157+
158+
// Debug build serializes to "debug".
159+
let mut debug_manifest = sample_manifest();
160+
debug_manifest.build_mode = BuildMode::Debug;
161+
let bytes = build_zip(&debug_manifest, &BTreeMap::new(), now).unwrap();
162+
let entries = read_zip_entries(&bytes);
163+
let manifest_json = entries.get("manifest.json").unwrap();
164+
assert!(
165+
manifest_json.contains("\"buildMode\": \"debug\""),
166+
"expected `\"buildMode\": \"debug\"` in manifest JSON, got: {manifest_json}",
167+
);
168+
}
169+
170+
#[test]
171+
fn build_mode_current_matches_cfg() {
172+
let expected = if cfg!(debug_assertions) {
173+
BuildMode::Debug
174+
} else {
175+
BuildMode::Release
176+
};
177+
assert_eq!(BuildMode::current(), expected);
178+
}
179+
137180
#[test]
138181
fn short_id_matches_expected_format() {
139182
let re = regex::Regex::new("^ERR-[23456789ABCDEFGHJKMNPQRSTUVWXYZ]{5}$").unwrap();

apps/desktop/src/lib/error-reporter/ErrorReportDialog.a11y.test.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* The `prepareErrorReportPreview` IPC is mocked so the test runs deterministically.
66
*/
77

8-
import { describe, it, vi, expect, beforeEach, afterEach } from 'vitest'
8+
import { describe, it, vi, expect, beforeEach } from 'vitest'
99
import { mount, tick } from 'svelte'
1010
import ErrorReportDialog from './ErrorReportDialog.svelte'
1111
import { expectNoA11yViolations } from '$lib/test-a11y'
@@ -57,15 +57,6 @@ Object.defineProperty(navigator, 'clipboard', {
5757
describe('ErrorReportDialog', () => {
5858
beforeEach(() => {
5959
closeErrorReportDialog()
60-
// Tests run in Vitest's `test` mode where `import.meta.env.DEV` is true
61-
// by default. Pretend we're a release build so the dialog's dev-mode
62-
// Send-disable doesn't shadow the assertions below — there's a separate
63-
// test ("disables Send in dev builds") for the dev-disable behavior.
64-
vi.stubEnv('DEV', false)
65-
})
66-
67-
afterEach(() => {
68-
vi.unstubAllEnvs()
6960
})
7061

7162
it('default render has no a11y violations', async () => {

apps/desktop/src/lib/error-reporter/ErrorReportDialog.svelte

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import BundleSavedToastContent, { setLastSavedBundlePath } from './BundleSavedToastContent.svelte'
2222
import { closeErrorReportDialog, errorReportFlow } from './error-report-flow.svelte'
2323
import { getAppLogger } from '$lib/logging/logger'
24-
import { tooltip } from '$lib/tooltip/tooltip'
2524
2625
const log = getAppLogger('errorReportDialog')
2726
@@ -123,16 +122,6 @@
123122
}
124123
}
125124
126-
// In dev / debug builds the backend's `upload()` function intentionally
127-
// short-circuits before hitting the network (see `error_reporter::upload`),
128-
// so clicking Send used to look like it succeeded but did nothing. Disable
129-
// the button instead and explain via a tooltip — the Save-to-disk button
130-
// is right next to it for inspecting the bundle locally.
131-
const sendDisabledInDev = isDev
132-
const sendTooltip = sendDisabledInDev
133-
? "Disabled in dev builds — uploads are skipped on purpose so dev runs don't pollute prod. Use 'Save bundle to disk' to inspect the report locally."
134-
: undefined
135-
136125
async function handleCopyId() {
137126
if (!preview) return
138127
await navigator.clipboard.writeText(preview.id)
@@ -146,13 +135,7 @@
146135
147136
function handleKeydown(event: KeyboardEvent) {
148137
// Cmd/Ctrl+Enter sends. Plain Enter is consumed by the textarea.
149-
if (
150-
(event.metaKey || event.ctrlKey) &&
151-
event.key === 'Enter' &&
152-
!sending &&
153-
!noteOverLimit &&
154-
!sendDisabledInDev
155-
) {
138+
if ((event.metaKey || event.ctrlKey) && event.key === 'Enter' && !sending && !noteOverLimit) {
156139
event.preventDefault()
157140
void handleSend()
158141
}
@@ -263,15 +246,13 @@
263246
{/if}
264247
<span class="spacer"></span>
265248
<Button variant="secondary" onclick={handleClose} disabled={sending}>Cancel</Button>
266-
<span use:tooltip={sendTooltip}>
267-
<Button
268-
variant="primary"
269-
onclick={() => void handleSend()}
270-
disabled={sending || noteOverLimit || preparing || sendDisabledInDev}
271-
>
272-
{sending ? 'Sending…' : 'Send report'}
273-
</Button>
274-
</span>
249+
<Button
250+
variant="primary"
251+
onclick={() => void handleSend()}
252+
disabled={sending || noteOverLimit || preparing}
253+
>
254+
{sending ? 'Sending…' : 'Send report'}
255+
</Button>
275256
</div>
276257
</div>
277258
</ModalDialog>

0 commit comments

Comments
 (0)