diff --git a/apps/account-directory/README.md b/apps/account-directory/README.md index a964c6793..f83a60437 100644 --- a/apps/account-directory/README.md +++ b/apps/account-directory/README.md @@ -198,14 +198,17 @@ commands and paste output is where most of them stalled. | | | |---|---| | Method | `POST` (plus `OPTIONS` preflight; anything else is `405`) | -| Body | `text/plain` — the report itself; or `application/json` — `{ report, installId?, appVersion? }` | -| Metadata on `text/plain` | `?installId=` / `?appVersion=` query parameters | +| Body | `text/plain` — the report itself; or `application/json` — `{ report, installId?, appVersion?, auto?, failureCode? }` | +| Metadata on `text/plain` | `?installId=` / `?appVersion=` / `?auto=` / `?failureCode=` query parameters | +| Automatic sends | `auto` (boolean, or `1`/`true`/`yes` as a string) marks a report the client sent on its own rather than one a human pressed send on. `failureCode` names what broke and is bound to `^[a-z][a-z0-9_-]{0,47}$`. Both are optional; absent means a manual send. A `failureCode` that does not match the shape is **dropped, not refused** — the label is cosmetic and the report is not | | Auth | **Optional** `Authorization: Bearer `, verified exactly as the account routes verify it. Absent, the upload is anonymous. A header that is sent and does not verify — or does not even parse as `Bearer ` — is `401`, never silently downgraded. A Worker with no Clerk configuration answers `503`, exactly as the account routes do | | Origin | `403` when the browser reports `sec-fetch-site: cross-site` from a real remote origin. ADE's own senders are unaffected: the CLI sends no fetch-metadata header, and the Electron renderer's `null` (packaged `file://`) and loopback (development) origins are exempt | | Size | `413` above 512 KB. `content-length` is checked first, then the stream is counted as it arrives, so a missing or dishonest length changes nothing | -| Rate limit | 5 per UTC day per user (signed in) or per `cf-connecting-ip` (anonymous) → `429` with `retry-after: 86400`. Off Cloudflare there is no trustworthy address, so anonymous callers share one bucket; `x-forwarded-for` is caller-controlled and is never read | +| Per-caller limit | 5 **stored** per UTC day per user (signed in) or per `cf-connecting-ip` (anonymous) → `429 {"error":"rate limited"}` with `retry-after: 86400`. Off Cloudflare there is no trustworthy address, so anonymous callers share one bucket; `x-forwarded-for` is caller-controlled and is never read | +| Fleet limit | `DIAGNOSTICS_DAILY_GLOBAL_LIMIT` uploads **stored per UTC day across every caller** (default 400) → `429 {"error":"daily diagnostics budget exhausted"}` with `retry-after` counting the seconds to the next UTC midnight. A **distinct body** from the per-caller `429` on purpose: only one of the two is about the caller, and an auto-sender that reads a fleet-wide stop as its own quota retries forever | +| Budget unavailable | `503 {"error":"diagnostics upload unavailable"}`. The claim **fails closed** — a ceiling that is skipped whenever D1 hiccups is not a ceiling | | Success | `200 {"ok": true, "id": ""}`. The report is **never** echoed back | -| Storage | `reports///.md` in the `DIAGNOSTICS` R2 bucket, with `userId` / `installId` / `appVersion` as custom metadata | +| Storage | `reports///.md` in the `DIAGNOSTICS` R2 bucket, with `userId` / `installId` / `appVersion` / `auto` / `failureCode` as custom metadata (`auto` is written only when true, so a manual upload stores exactly what it always did) | | No binding | `503`, and the in-app button says sending is unavailable | The key's identity segment is `u-` when signed in and @@ -219,15 +222,70 @@ allow-list can name it; `*` is safe here because the route reads no account state, returns only an opaque id, and cannot be used with `credentials: "include"`. Every `/account/*` route keeps its exact-origin rule. -**Rate limiting without a migration.** The device flow counts attempts in the -`device_approval_rate_limits` D1 table. This route deliberately does not: it -ships without touching `migrations/`, so the quota is enforced by a per-isolate -counter (fast, but lost when Cloudflare recycles the isolate) backed by an R2 -prefix listing (durable and global, one class-A operation per upload). The -listing is not transactional, so genuinely simultaneous requests can land a -couple of objects over five. For a bound whose only job is "one person cannot -fill the bucket", that is an acceptable trade; if volume ever justifies exact -counting, move it to the D1 pattern the device flow already uses. +**Two limits, because they bound different things.** + +The *per-caller* quota answers "one person cannot fill the bucket". It is +enforced by a per-isolate counter (fast, but lost when Cloudflare recycles the +isolate) backed by an R2 prefix listing on the caller's day (durable and global, +one class-A operation per upload). The listing is not transactional, so +genuinely simultaneous requests can land a couple of objects over five — an +acceptable slop for an abuse bound. + +Both halves count **stored objects**, never attempts, and the counter is +advanced only after the `put` returns. The durable half cannot do otherwise — +it is a listing of what is in the bucket — and the fast path has to agree with +it or it is not a cache of it. Counting attempts let refusals the caller did +not cause (a fleet budget that was out for the day, a bucket having a bad +minute) lock an install out until UTC midnight having stored nothing, which is +the same reasoning as the fleet budget's refund below. + +The *fleet* budget answers a different question, and it is not allowed any slop +at all, because it is the storage bill. ADE clients now send reports +**automatically on failure**, so a single bug that fires for every install at +once multiplies "five each" by the install base, and no per-caller limit can see +that coming. `diagnostics_upload_days` (migration `0009`) holds one row per UTC +day, and every upload claims a slot from it in a single statement: + +```sql +insert into diagnostics_upload_days (day, count) +values (?, 1) +on conflict(day) do update set count = count + 1 +where count < ? +``` + +`changes === 1` is the whole proof — the same upsert idiom +`device_approval_rate_limits` uses, and for the same reason: a read followed by +a write lets two concurrent uploads both observe the last free slot. No +`RETURNING`, so nothing depends on a D1 version. + +**The cost ceiling is arithmetic, not an estimate.** This Worker is the *only* +writer the bucket has, so the numbers below are the whole spend: + +``` +400 uploads/day DIAGNOSTICS_DAILY_GLOBAL_LIMIT +× 512 KB/upload MAX_DIAGNOSTIC_REPORT_BYTES (413 above it) +× 30 days the bucket's expiry lifecycle rule +≈ 6 GB steady-state maximum, against R2's 10 GB free tier +``` + +Every term is enforced somewhere a client cannot reach: the first by the claim +above, the second by the streaming size cap, the third by the bucket lifecycle +(see the deployment steps — **nothing in the Worker ever deletes a report**). +Change any one of them and redo the multiplication. + +Ordering matters and is deliberate: the fleet slot is claimed **after** the +per-caller quota and **before** the R2 `put`. After, because one caller +hammering their own limit must not spend the fleet's budget on requests that +were never going to be stored. Before, because that ordering is what makes the +cap unraceable — the day's stored count cannot exceed the day's claimed count. A +`put` that then fails **refunds** the slot, so an R2 outage does not quietly eat +the day's ceiling for reports that do not exist. + +`0` is a kill switch: it refuses every upload without a code deploy. An unset or +unparseable value falls back to 400, so a typo can neither uncap the bill nor +close the route. The cron sweep prunes budget rows older than seven days; +today's row is never in range, so a sweep can never hand back budget the running +day has already spent. ## Local checks @@ -275,20 +333,24 @@ deployment: The binding is optional in code, so an already-deployed Worker whose bucket was removed answers `503` on `/diagnostics/upload` and keeps every other route working. -4. Give both diagnostics buckets an expiry lifecycle rule. **Nothing in the - Worker ever deletes a report**, so without this the bucket grows forever and - every report a user ever sent stays readable indefinitely. Ninety days is the - default because it is far longer than any support thread and far shorter than - "forever" — shorten it if your retention policy says so: +4. Give both diagnostics buckets a **30-day** expiry lifecycle rule. **Nothing + in the Worker ever deletes a report**, so without this the bucket grows + forever and every report a user ever sent stays readable indefinitely. Thirty + days is not a taste preference: it is the third term of the cost ceiling + above (400/day × 512 KB × 30 days ≈ 6 GB, inside R2's 10 GB free tier), and + it is the one term this repository cannot enforce in code. Lengthen it and + the ceiling moves with it — 90 days is ~18 GB and off the free tier: ```sh npx wrangler r2 bucket lifecycle add ade-diagnostics \ - expire-reports reports/ --expire-days 90 + expire-reports reports/ --expire-days 30 npx wrangler r2 bucket lifecycle add ade-diagnostics-production \ - expire-reports reports/ --expire-days 90 + expire-reports reports/ --expire-days 30 ``` - Confirm with `npx wrangler r2 bucket lifecycle list `. + Confirm with `npx wrangler r2 bucket lifecycle list `. Thirty days is + still far longer than any support thread; a report nobody has read in a month + is not going to be read. 5. Apply the remote migrations and deploy the Worker. Use `npm run d1:migrate:production` and `npm run deploy:production` for the production environment. Each deploy script validates only the environment it diff --git a/apps/account-directory/migrations/0009_diagnostics_upload_budget.sql b/apps/account-directory/migrations/0009_diagnostics_upload_budget.sql new file mode 100644 index 000000000..b2e025f49 --- /dev/null +++ b/apps/account-directory/migrations/0009_diagnostics_upload_budget.sql @@ -0,0 +1,30 @@ +-- A fleet-wide daily ceiling on stored diagnostic reports. +-- +-- The per-identity quota (5 a day, enforced against an R2 prefix listing) bounds +-- what ONE caller can store. It does not bound what the fleet can store, and +-- that stopped being a theoretical distinction the moment clients began sending +-- reports AUTOMATICALLY on failure: a bug that fires for every install at once +-- turns "five each" into "five times the install base", and the only thing +-- standing between that and an unbounded R2 bill is a number nobody chose. +-- +-- This table is that number. The Worker is the sole writer of the bucket, so a +-- counter it must claim from before every `put` is not a heuristic — it is the +-- spend cap. `DIAGNOSTICS_DAILY_GLOBAL_LIMIT` (default 400) times the 512 KB +-- per-report cap times the bucket's 30-day expiry lifecycle is the entire +-- steady-state ceiling: ~6 GB, inside R2's free tier, with no path to exceed it +-- that does not go through this row. +-- +-- One row per UTC day, claimed by a single upsert whose `where count < ?` makes +-- the increment and the check the same statement — the idiom +-- `device_approval_rate_limits` already uses, for the same reason: a read +-- followed by a write lets two concurrent uploads both observe the last free +-- slot. `changes === 1` is the whole proof, so no RETURNING is needed and the +-- statement stays portable across D1 versions. +-- +-- The day key is the UTC date string (`2026-08-19`), not an epoch: it is what +-- the R2 key prefix already uses, it sorts lexicographically, and it makes both +-- the cron sweep's `day < ?` and a human reading the table trivial. +create table if not exists diagnostics_upload_days ( + day text primary key, + count integer not null +); diff --git a/apps/account-directory/src/diagnostics.ts b/apps/account-directory/src/diagnostics.ts index 5744a642e..b2adf28b8 100644 --- a/apps/account-directory/src/diagnostics.ts +++ b/apps/account-directory/src/diagnostics.ts @@ -16,7 +16,11 @@ import { isLoopbackHostname } from "./trustedOrigin"; */ /** R2 is optional in the binding type so a Worker deployed before the bucket exists degrades instead of crashing. */ -export type DiagnosticsEnv = Env & { DIAGNOSTICS?: R2Bucket }; +export type DiagnosticsEnv = Env & { + DIAGNOSTICS?: R2Bucket; + /** Fleet-wide uploads allowed per UTC day; see `DEFAULT_DIAGNOSTICS_DAILY_GLOBAL_LIMIT`. */ + DIAGNOSTICS_DAILY_GLOBAL_LIMIT?: string; +}; export const DIAGNOSTICS_UPLOAD_PATH = "/diagnostics/upload"; @@ -36,7 +40,47 @@ export const MAX_DIAGNOSTIC_REPORT_BYTES = 512 * 1024; /** Uploads one identity may store per UTC day. */ export const MAX_DIAGNOSTIC_UPLOADS_PER_DAY = 5; +/** + * Uploads the WHOLE FLEET may store per UTC day, when + * `DIAGNOSTICS_DAILY_GLOBAL_LIMIT` is unset or unreadable. + * + * The per-identity quota above bounds one caller. This one bounds the bill. + * Clients now send reports automatically on failure, so a bug that fires for + * every install at once multiplies "five each" by the install base, and this + * Worker is the only writer the bucket has — which means this number IS the + * spend cap rather than an estimate of one: + * + * 400 uploads/day × 512 KB/upload × 30-day bucket lifecycle ≈ 6 GB steady + * state, against R2's 10 GB free tier. + * + * Raising it is a deliberate act with arithmetic attached; see the README. + */ +export const DEFAULT_DIAGNOSTICS_DAILY_GLOBAL_LIMIT = 400; + +/** + * How many days of budget rows the cron sweep keeps. + * + * Only today's row is ever read, so the rest is history kept for one reason: a + * support question about a fleet-wide refusal is asked days after it happened. + * A week covers that and keeps the table permanently tiny. + */ +export const DIAGNOSTICS_BUDGET_RETENTION_DAYS = 7; + +const DAY_MS = 86_400_000; + const MAX_METADATA_CHARS = 200; + +/** + * Shape of a `failureCode`, and the whole validation it gets. + * + * It is a client-supplied label for an automatic send ("brain_start_timeout"), + * so it is bound to a shape that is safe as an R2 custom-metadata header value + * and as a log field, and anything else is DROPPED rather than refused: a + * report that arrives with a malformed label is still the report support needs, + * and failing the upload over a cosmetic field would be the auto-send path + * losing exactly the diagnostics it exists to collect. + */ +const FAILURE_CODE_PATTERN = /^[a-z][a-z0-9_-]{0,47}$/; /** * Bound on the per-isolate counter map. An isolate that has seen more distinct * uploaders than this is being probed, not used; dropping the whole map costs @@ -162,6 +206,113 @@ function utcDayKey(nowMs: number): string { return new Date(nowMs).toISOString().slice(0, 10); } +/** + * Seconds until the fleet budget resets. + * + * Unix time has no leap seconds, so `nowMs % DAY_MS` is exactly the time since + * UTC midnight and this is the honest number rather than the flat 86400 the + * per-identity limit answers. A client refused at 23:59 should retry in a + * minute, not tomorrow night. + */ +function secondsUntilNextUtcDay(nowMs: number): number { + return Math.max(1, Math.ceil((DAY_MS - (nowMs % DAY_MS)) / 1000)); +} + +/** + * The configured fleet ceiling. + * + * An unset, empty, or unparseable value falls back to the default — a typo in a + * var must not silently uncap the bill or silently close the route. `0` is + * honored, on purpose: it is the kill switch that stops every upload without a + * redeploy of code. + */ +function dailyGlobalLimit(env: DiagnosticsEnv): number { + const raw = env.DIAGNOSTICS_DAILY_GLOBAL_LIMIT?.trim(); + if (!raw) return DEFAULT_DIAGNOSTICS_DAILY_GLOBAL_LIMIT; + const configured = Number(raw); + return Number.isFinite(configured) && configured >= 0 + ? Math.trunc(configured) + : DEFAULT_DIAGNOSTICS_DAILY_GLOBAL_LIMIT; +} + +type BudgetClaim = { ok: true } | { ok: false; reason: "exhausted" | "unavailable" }; + +/** + * Claim one slot out of today's fleet budget, atomically. + * + * The increment and the check are ONE statement — the same upsert idiom + * `checkDeviceRateLimit` uses — because a read followed by a write lets two + * concurrent uploads both observe the last free slot, and a spend cap that can + * be raced is not a cap. `changes === 1` is the whole proof: SQLite applies the + * `where count < ?` to the `do update`, so a full day changes no row. + * + * FAILS CLOSED. A budget that cannot be counted is a budget that is not + * enforced, and the whole point of this table is that no path to the bucket + * bypasses it — so a D1 error refuses the upload rather than storing it + * uncounted. The deploy scripts apply migrations before publishing, so the + * window where the table does not exist is a deploy that has already failed. + */ +async function claimGlobalBudget( + env: DiagnosticsEnv, + dayKey: string, + limit: number, +): Promise { + // A configured zero means "store nothing today"; there is no row to write for + // an upload that will never happen. + if (limit <= 0) return { ok: false, reason: "exhausted" }; + try { + const claimed = await env.DB.prepare(` + insert into diagnostics_upload_days (day, count) + values (?, 1) + on conflict(day) do update set count = count + 1 + where count < ? + `).bind(dayKey, limit).run(); + return (claimed.meta?.changes ?? 0) === 1 ? { ok: true } : { ok: false, reason: "exhausted" }; + } catch { + return { ok: false, reason: "unavailable" }; + } +} + +/** + * Give the slot back when the write it was claimed for did not happen. + * + * The claim has to precede the `put` — that ordering is what makes the cap + * unraceable — so an R2 failure would otherwise burn budget for bytes nobody + * can ever read. `count > 0` keeps the row from going negative if a refund ever + * arrives without a matching claim, and a failed refund is swallowed: the + * caller is already being told the upload failed, and turning that into a 500 + * would trade an accurate error for a confusing one. The cost of losing a + * refund is one slot out of the day, which is the safe direction. + */ +async function refundGlobalBudget(env: DiagnosticsEnv, dayKey: string): Promise { + try { + await env.DB.prepare( + "update diagnostics_upload_days set count = count - 1 where day = ? and count > 0", + ).bind(dayKey).run(); + } catch { + // Best effort by design; see above. + } +} + +/** + * Cron sweep: budget rows are write-once-a-day and read only for today. + * + * Lexicographic comparison is correct here because the key is a fixed-width + * ISO date. Today's row is never in range, so a sweep can never free budget the + * running day has already spent. + */ +export async function cleanupDiagnosticsUploadDays( + env: { DB: D1Database }, + nowMs = Date.now(), +): Promise { + const cutoff = utcDayKey(nowMs - DIAGNOSTICS_BUDGET_RETENTION_DAYS * DAY_MS); + const result = await env.DB + .prepare("delete from diagnostics_upload_days where day < ?") + .bind(cutoff) + .run(); + return result.meta.changes ?? 0; +} + function boundedMetadata(value: unknown): string | undefined { if (typeof value !== "string") return undefined; const trimmed = value.trim(); @@ -172,6 +323,29 @@ function boundedMetadata(value: unknown): string | undefined { return trimmed.replace(/[\u0000-\u001f\u007f]/g, " ").slice(0, MAX_METADATA_CHARS); } +/** + * Was this report sent by a human pressing a button, or by the client deciding + * on its own that something had broken? + * + * Both shapes have to be read because both senders exist: a JSON body carries a + * real boolean, and the `text/plain` path can only say `?auto=1`. Absent — or + * anything unrecognized — means manual, which is what every sender that shipped + * before this flag existed is. + */ +function parseAutoFlag(value: unknown): boolean { + if (typeof value === "boolean") return value; + if (typeof value !== "string") return false; + const normalized = value.trim().toLowerCase(); + return normalized === "true" || normalized === "1" || normalized === "yes"; +} + +/** A `failureCode` that matches `FAILURE_CODE_PATTERN`, or nothing at all. */ +function boundedFailureCode(value: unknown): string | undefined { + if (typeof value !== "string") return undefined; + const trimmed = value.trim(); + return FAILURE_CODE_PATTERN.test(trimmed) ? trimmed : undefined; +} + /** * Reads at most `MAX_DIAGNOSTIC_REPORT_BYTES + 1` bytes. * @@ -221,6 +395,9 @@ type ParsedUpload = { report: string; installId?: string; appVersion?: string; + /** True when the client sent this on its own; false is a human pressing send. */ + auto: boolean; + failureCode?: string; }; /** @@ -249,30 +426,68 @@ function parseUpload(contentType: string, raw: string, url: URL): ParsedUpload | report, installId: boundedMetadata(fields.installId ?? url.searchParams.get("installId")), appVersion: boundedMetadata(fields.appVersion ?? url.searchParams.get("appVersion")), + // `??` rather than `||` so an explicit `auto: false` in the body is honored + // instead of falling through to a query parameter that says otherwise. + auto: parseAutoFlag(fields.auto ?? url.searchParams.get("auto")), + failureCode: boundedFailureCode(fields.failureCode ?? url.searchParams.get("failureCode")), }; } -async function withinDailyLimit( +/** + * How much of this identity's day is already spent. READ ONLY. + * + * Both halves of the quota count STORED OBJECTS, never attempts — the durable + * half cannot do otherwise (it is a listing of what is in the bucket) and the + * fast path must agree with it or it is not a cache of it. Counting an attempt + * would let refusals the caller did not cause — a fleet budget that is out for + * the day, a bucket having a bad minute — burn a quota that exists to bound how + * much one caller can STORE, and lock an install out over nothing. + * + * `null` is "the quota could not be counted". FAILS CLOSED, exactly as the + * fleet budget does: a listing that throws would otherwise escape this function + * as a bare 500 with no `diagnostics_upload` line at all, and support could not + * tell that from a report that never left the machine. + */ +async function spentToday( bucket: R2Bucket, identity: string, prefix: string, dayKey: string, -): Promise { +): Promise { const remembered = isolateUploadCounts.get(identity); const rememberedCount = remembered?.dayKey === dayKey ? remembered.count : 0; - if (rememberedCount >= MAX_DIAGNOSTIC_UPLOADS_PER_DAY) return false; + if (rememberedCount >= MAX_DIAGNOSTIC_UPLOADS_PER_DAY) return rememberedCount; // The durable half of the limit. `limit` stops one greedy prefix from // listing an unbounded page just to answer a yes/no question. - const listed = await bucket.list({ prefix, limit: MAX_DIAGNOSTIC_UPLOADS_PER_DAY + 1 }); - const stored = listed.objects.length; - const count = Math.max(stored, rememberedCount); + let listed: R2Objects; + try { + listed = await bucket.list({ prefix, limit: MAX_DIAGNOSTIC_UPLOADS_PER_DAY + 1 }); + } catch { + return null; + } + const count = Math.max(listed.objects.length, rememberedCount); + // A caller the listing shows is already full stays full for the rest of the + // day, so remembering that saves a listing on each further attempt. Still a + // count of STORED objects — it is the optimistic `count + 1` that was the bug. + if (count >= MAX_DIAGNOSTIC_UPLOADS_PER_DAY) remember(identity, dayKey, count); + return count; +} + +function remember(identity: string, dayKey: string, count: number): void { if (isolateUploadCounts.size >= MAX_TRACKED_IDENTITIES) isolateUploadCounts.clear(); - isolateUploadCounts.set(identity, { - dayKey, - count: count >= MAX_DIAGNOSTIC_UPLOADS_PER_DAY ? count : count + 1, - }); - return count < MAX_DIAGNOSTIC_UPLOADS_PER_DAY; + isolateUploadCounts.set(identity, { dayKey, count }); +} + +/** + * Advance the fast path, and only for an object that is actually in the bucket. + * + * Called after the `put` returns, so the counter tracks the listing rather than + * running ahead of it. The listing stays the authority either way: this only + * saves a class-A operation on the next request from the same caller. + */ +function rememberStored(identity: string, dayKey: string, spentBefore: number): void { + remember(identity, dayKey, spentBefore + 1); } export type DiagnosticsRequestOptions = { @@ -361,7 +576,23 @@ export async function handleDiagnosticsRequest( const now = options.now?.() ?? Date.now(); const dayKey = utcDayKey(now); const prefix = `reports/${dayKey}/${identity}/`; - if (!(await withinDailyLimit(bucket, identity, prefix, dayKey))) { + const spent = await spentToday(bucket, identity, prefix, dayKey); + if (spent == null) { + // Nothing was claimed and nothing was stored — the fleet budget is not even + // touched, because a quota this route cannot count must not spend one. + logDiagnosticsUpload({ + outcome: "rejected", + status: 503, + reason: "quota_unavailable", + identity, + authenticated: Boolean(userId), + bytes: parsed.report.length, + auto: parsed.auto, + failureCode: parsed.failureCode, + }); + return json({ error: "diagnostics upload unavailable" }, { status: 503 }); + } + if (spent >= MAX_DIAGNOSTIC_UPLOADS_PER_DAY) { logDiagnosticsUpload({ outcome: "rejected", status: 429, @@ -369,6 +600,8 @@ export async function handleDiagnosticsRequest( identity, authenticated: Boolean(userId), bytes: parsed.report.length, + auto: parsed.auto, + failureCode: parsed.failureCode, }); return json({ error: "rate limited" }, { status: 429, @@ -376,6 +609,40 @@ export async function handleDiagnosticsRequest( }); } + // The fleet ceiling is claimed AFTER the per-caller quota and BEFORE the put. + // + // After, because one caller hammering their own limit must not be able to + // spend the fleet's budget on requests that were never going to be stored — + // that would turn a per-caller abuse bound into a fleet-wide denial of + // service. Before, because the claim is what makes the cap real: every byte + // that reaches the bucket passes through this statement first, so the day's + // stored count cannot exceed the day's claimed count. + const budget = await claimGlobalBudget(env, dayKey, dailyGlobalLimit(env)); + if (!budget.ok) { + const exhausted = budget.reason === "exhausted"; + logDiagnosticsUpload({ + outcome: "rejected", + status: exhausted ? 429 : 503, + reason: exhausted ? "global_budget_exhausted" : "budget_unavailable", + identity, + authenticated: Boolean(userId), + bytes: parsed.report.length, + auto: parsed.auto, + failureCode: parsed.failureCode, + }); + // A DISTINCT 429 body from the per-caller one above. Both mean "not now", + // but only one of them is about the caller: a client that cannot tell them + // apart cannot decide whether backing off its own sends would help, and an + // auto-sender that treats a fleet-wide stop as its own quota would keep + // retrying forever. + return exhausted + ? json({ error: "daily diagnostics budget exhausted" }, { + status: 429, + headers: { "retry-after": String(secondsUntilNextUtcDay(now)) }, + }) + : json({ error: "diagnostics upload unavailable" }, { status: 503 }); + } + const id = options.randomId?.() ?? crypto.randomUUID(); try { await bucket.put(`${prefix}${id}.md`, parsed.report, { @@ -384,6 +651,10 @@ export async function handleDiagnosticsRequest( ...(userId ? { userId } : {}), ...(parsed.installId ? { installId: parsed.installId } : {}), ...(parsed.appVersion ? { appVersion: parsed.appVersion } : {}), + // Only written when true, so a manual upload's metadata is exactly what + // it was before this flag existed. + ...(parsed.auto ? { auto: "true" } : {}), + ...(parsed.failureCode ? { failureCode: parsed.failureCode } : {}), }, }); } catch { @@ -395,6 +666,11 @@ export async function handleDiagnosticsRequest( // not the caller's. 502 rather than the 503 the missing-binding path uses, // so a configured bucket having a bad minute stays distinguishable from a // bucket that was never created. + // + // The budget slot claimed a moment ago is given back: nothing was stored, + // so nothing should have been spent, and a bucket having a bad hour must + // not quietly consume the day's ceiling for reports that do not exist. + await refundGlobalBudget(env, dayKey); logDiagnosticsUpload({ outcome: "rejected", status: 502, @@ -402,16 +678,24 @@ export async function handleDiagnosticsRequest( identity, authenticated: Boolean(userId), bytes: parsed.report.length, + auto: parsed.auto, + failureCode: parsed.failureCode, }); return json({ error: "diagnostics upload failed" }, { status: 502 }); } + // Stored, so now it counts. Deliberately after the `put`, for the same reason + // the fleet budget refunds itself above: nothing that failed to reach the + // bucket may cost a caller part of the day they are allowed to store. + rememberStored(identity, dayKey, spent); logDiagnosticsUpload({ outcome: "stored", status: 200, identity, authenticated: Boolean(userId), bytes: parsed.report.length, + auto: parsed.auto, + failureCode: parsed.failureCode, }); // Only the id goes back. The report is never echoed: a route that returned // what it stored would be a way to read other people's uploads the moment an diff --git a/apps/account-directory/src/index.ts b/apps/account-directory/src/index.ts index 7a0d57372..0e7536c5e 100644 --- a/apps/account-directory/src/index.ts +++ b/apps/account-directory/src/index.ts @@ -1,7 +1,12 @@ import { handleRequest, type Env } from "./directory"; import { cleanupExpiredPairingGrants } from "./pairingGrants"; import { cleanupExpiredDeviceAuthorizations } from "./deviceAuthorization"; -import { handleDiagnosticsRequest, isDiagnosticsRequest, type DiagnosticsEnv } from "./diagnostics"; +import { + cleanupDiagnosticsUploadDays, + handleDiagnosticsRequest, + isDiagnosticsRequest, + type DiagnosticsEnv, +} from "./diagnostics"; export default { fetch(request: Request, env: DiagnosticsEnv): Promise { @@ -19,6 +24,9 @@ export default { ctx.waitUntil(Promise.all([ cleanupExpiredDeviceAuthorizations(env), cleanupExpiredPairingGrants(env), + // Only today's budget row is ever read; the rest is kept for a week so a + // support question about a fleet-wide refusal still has a row to point at. + cleanupDiagnosticsUploadDays(env), ])); }, }; diff --git a/apps/account-directory/src/logging.ts b/apps/account-directory/src/logging.ts index acc6f67f2..841de2bd1 100644 --- a/apps/account-directory/src/logging.ts +++ b/apps/account-directory/src/logging.ts @@ -73,6 +73,12 @@ export function logDirectoryRefusal(args: { * The route stores bytes it never parses, so this line is the only record that * an upload happened at all — and the only way to tell "the user's report never * arrived" from "it arrived and was refused for being too large". + * + * `auto` and `failureCode` are what make the automatic senders legible: without + * them a spike in this line is indistinguishable from a spike in users pressing + * a button, and "which failure is generating all this traffic" — the question + * asked the day the fleet budget is first exhausted — has no answer at all. + * They are absent on the paths that reject before the body is parsed. */ export function logDiagnosticsUpload(args: { outcome: "stored" | "rejected"; @@ -81,6 +87,9 @@ export function logDiagnosticsUpload(args: { identity: string; authenticated: boolean; bytes: number; + /** Omitted where the route refused before it could know. */ + auto?: boolean; + failureCode?: string; }): void { console.log(JSON.stringify({ ts: new Date().toISOString(), @@ -94,6 +103,11 @@ export function logDiagnosticsUpload(args: { identity: args.identity.slice(0, 24), authenticated: args.authenticated, bytes: args.bytes, + // Emitted whenever it is known, false included: "how many of today's + // uploads were automatic" is a ratio, and a field that only appears on one + // side of it cannot be counted. + ...(args.auto === undefined ? {} : { auto: args.auto }), + ...(args.failureCode ? { failureCode: args.failureCode } : {}), })); } diff --git a/apps/account-directory/test/diagnostics.test.ts b/apps/account-directory/test/diagnostics.test.ts index 486fc1179..c43c84bf9 100644 --- a/apps/account-directory/test/diagnostics.test.ts +++ b/apps/account-directory/test/diagnostics.test.ts @@ -1,6 +1,9 @@ import { createHash } from "node:crypto"; import { describe, expect, it, vi } from "vitest"; import { + cleanupDiagnosticsUploadDays, + DEFAULT_DIAGNOSTICS_DAILY_GLOBAL_LIMIT, + DIAGNOSTICS_BUDGET_RETENTION_DAYS, handleDiagnosticsRequest, isDiagnosticsRequest, MAX_DIAGNOSTIC_REPORT_BYTES, @@ -8,6 +11,7 @@ import { type DiagnosticsEnv, } from "../src/diagnostics"; import worker from "../src/index"; +import { FakeD1Database } from "./fakeD1"; import { ISSUER, jwksEndpoint, mintToken, OAUTH_CLIENT_ID } from "./jwks"; const UPLOAD_URL = "https://directory.test/diagnostics/upload"; @@ -29,6 +33,9 @@ class FakeR2Bucket { /** Set to make every `put` reject, the way a bucket having a bad minute does. */ putFailure: Error | null = null; + /** Same, for the listing the per-caller quota is counted from. */ + listFailure: Error | null = null; + async put( key: string, value: string | ArrayBuffer | ArrayBufferView, @@ -52,6 +59,7 @@ class FakeR2Bucket { options?: { prefix?: string; limit?: number }, ): Promise<{ objects: Array<{ key: string }>; truncated: boolean }> { this.listCalls.push({ prefix: options?.prefix, limit: options?.limit }); + if (this.listFailure) throw this.listFailure; const prefix = options?.prefix ?? ""; const keys = [...this.objects.keys()].filter((key) => key.startsWith(prefix)).sort(); const limited = options?.limit === undefined ? keys : keys.slice(0, options.limit); @@ -66,17 +74,49 @@ class FakeR2Bucket { } } +/** + * The route now claims a slot out of a D1-held fleet budget before every write, + * so a real fake database is part of the harness rather than a stub: an env + * whose `DB` cannot answer is exactly the fail-closed case, and it has its own + * test below. + */ function makeEnv( overrides: Partial = {}, -): DiagnosticsEnv & { DIAGNOSTICS: FakeR2Bucket } { +): DiagnosticsEnv & { DIAGNOSTICS: FakeR2Bucket; DB: FakeD1Database } { return { - DB: {} as unknown as D1Database, + DB: new FakeD1Database(), CLERK_JWKS_URL: jwksEndpoint(), CLERK_ISSUER: ISSUER, CLERK_OAUTH_CLIENT_ID: OAUTH_CLIENT_ID, DIAGNOSTICS: new FakeR2Bucket(), ...overrides, - } as unknown as DiagnosticsEnv & { DIAGNOSTICS: FakeR2Bucket }; + } as unknown as DiagnosticsEnv & { DIAGNOSTICS: FakeR2Bucket; DB: FakeD1Database }; +} + +/** How much of today's fleet budget the route has claimed. */ +function spentBudget(env: { DB: FakeD1Database }, dayKey = FIXED_DAY_KEY): number { + return env.DB.diagnosticsUploadDays.get(dayKey) ?? 0; +} + +/** Captures the structured upload lines a request emits. */ +async function captureUploadLines( + run: () => Promise, +): Promise<{ result: T; lines: Array> }> { + const raw: string[] = []; + const logged = vi.spyOn(console, "log").mockImplementation((line: unknown) => { + raw.push(String(line)); + }); + try { + const result = await run(); + return { + result, + lines: raw + .map((line) => JSON.parse(line) as Record) + .filter((entry) => entry.kind === "diagnostics_upload"), + }; + } finally { + logged.mockRestore(); + } } /** A distinct address per test: the anonymous quota is keyed on the caller IP. */ @@ -455,6 +495,34 @@ describe("diagnostics upload route", () => { }); }); + it("fails closed, and audibly, when the per-caller quota cannot be counted", async () => { + // A listing that throws used to escape as a bare 500 with no line at all, + // which is the one outcome support cannot tell from "the report never + // left the machine". It must also not spend a fleet slot on a report that + // was never going to be stored. + const env = makeEnv(); + env.DIAGNOSTICS.listFailure = new Error("R2 unavailable"); + const { result: response, lines } = await captureUploadLines(() => + handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }) }), + env, + FIXED_CLOCK, + ), + ); + + expect(response.status).toBe(503); + expect(await response.json()).toEqual({ error: "diagnostics upload unavailable" }); + expect(env.DIAGNOSTICS.keys()).toHaveLength(0); + expect(spentBudget(env)).toBe(0); + + expect(lines).toHaveLength(1); + expect(lines[0]).toMatchObject({ + outcome: "rejected", + status: 503, + reason: "quota_unavailable", + }); + }); + it("answers 503 when the bucket binding is missing", async () => { const env = makeEnv({ DIAGNOSTICS: undefined }); const response = await handleDiagnosticsRequest( @@ -484,6 +552,366 @@ describe("diagnostics upload route", () => { expect(wrongMethod.status).toBe(405); }); + it("stops the whole fleet at the daily budget, whoever is uploading", async () => { + // The per-caller quota bounds one person. This bounds the bill: clients now + // send reports automatically on failure, so the number of DISTINCT callers + // is the thing that runs away, and no per-caller limit can see that. + const env = makeEnv({ DIAGNOSTICS_DAILY_GLOBAL_LIMIT: "3" }); + for (let caller = 0; caller < 3; caller += 1) { + const accepted = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip: `192.0.2.${caller}` }), + env, + FIXED_CLOCK, + ); + expect(accepted.status).toBe(200); + } + expect(spentBudget(env)).toBe(3); + + // A caller who has never uploaded before, well inside their own five. + const refused = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip: "192.0.2.99" }), + env, + FIXED_CLOCK, + ); + expect(refused.status).toBe(429); + expect(await refused.json()).toEqual({ error: "daily diagnostics budget exhausted" }); + // Honest seconds to the reset rather than a flat day: FIXED_NOW is UTC noon. + expect(refused.headers.get("retry-after")).toBe("43200"); + expect(env.DIAGNOSTICS.keys()).toHaveLength(3); + // The refusal did not bump the counter past the ceiling it enforces. + expect(spentBudget(env)).toBe(3); + }); + + it("keeps the fleet 429 distinguishable from the per-caller 429", async () => { + // A client that cannot tell them apart cannot decide whether backing off + // its own sends would help, and an auto-sender that reads a fleet-wide stop + // as its own quota retries forever. + const env = makeEnv({ DIAGNOSTICS_DAILY_GLOBAL_LIMIT: "1000" }); + const ip = "198.51.100.61"; + for (let attempt = 0; attempt < MAX_DIAGNOSTIC_UPLOADS_PER_DAY; attempt += 1) { + await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip }), + env, + FIXED_CLOCK, + ); + } + const perCaller = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip }), + env, + FIXED_CLOCK, + ); + expect(perCaller.status).toBe(429); + expect(await perCaller.json()).toEqual({ error: "rate limited" }); + expect(perCaller.headers.get("retry-after")).toBe("86400"); + + // And a caller refused by their OWN quota must not have spent fleet budget: + // otherwise one abusive sender denies the route to everybody else. + expect(spentBudget(env)).toBe(MAX_DIAGNOSTIC_UPLOADS_PER_DAY); + }); + + it("does not spend a caller's day on uploads the fleet budget refused", async () => { + // Regression: the per-identity counter advanced when the limit was CHECKED, + // so five refusals the caller did not cause — a fleet budget that was out + // for the day — locked that install out of the route until UTC midnight, + // having stored nothing. Both halves of the quota count stored objects. + const env = makeEnv({ DIAGNOSTICS_DAILY_GLOBAL_LIMIT: "0" }); + const ip = "198.51.100.71"; + for (let attempt = 0; attempt < MAX_DIAGNOSTIC_UPLOADS_PER_DAY; attempt += 1) { + const refused = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip }), + env, + FIXED_CLOCK, + ); + expect(refused.status).toBe(429); + expect(await refused.json()).toEqual({ error: "daily diagnostics budget exhausted" }); + } + expect(env.DIAGNOSTICS.keys()).toHaveLength(0); + + // The kill switch comes off and this caller still has their whole day. + env.DIAGNOSTICS_DAILY_GLOBAL_LIMIT = "1000"; + for (let attempt = 0; attempt < MAX_DIAGNOSTIC_UPLOADS_PER_DAY; attempt += 1) { + const accepted = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip }), + env, + FIXED_CLOCK, + ); + expect(accepted.status).toBe(200); + } + expect(env.DIAGNOSTICS.keys()).toHaveLength(MAX_DIAGNOSTIC_UPLOADS_PER_DAY); + }); + + it("does not spend a caller's day on uploads the bucket dropped", async () => { + // Same rule from the other side: the refund the fleet budget already gets + // for a failed `put` has to apply to the per-identity quota too, or a + // bucket having a bad minute costs the user their reports for the day. + const env = makeEnv({ DIAGNOSTICS_DAILY_GLOBAL_LIMIT: "1000" }); + const ip = "198.51.100.72"; + env.DIAGNOSTICS.putFailure = new Error("R2 is having a moment"); + for (let attempt = 0; attempt < MAX_DIAGNOSTIC_UPLOADS_PER_DAY; attempt += 1) { + const failed = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip }), + env, + FIXED_CLOCK, + ); + expect(failed.status).toBe(502); + } + expect(spentBudget(env)).toBe(0); + + env.DIAGNOSTICS.putFailure = null; + const accepted = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip }), + env, + FIXED_CLOCK, + ); + expect(accepted.status).toBe(200); + }); + + it("still enforces the per-caller quota under a generous fleet budget", async () => { + const env = makeEnv({ DIAGNOSTICS_DAILY_GLOBAL_LIMIT: "1000" }); + const ip = "198.51.100.62"; + for (let attempt = 0; attempt < MAX_DIAGNOSTIC_UPLOADS_PER_DAY; attempt += 1) { + const accepted = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip }), + env, + FIXED_CLOCK, + ); + expect(accepted.status).toBe(200); + } + const refused = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip }), + env, + FIXED_CLOCK, + ); + expect(refused.status).toBe(429); + expect(env.DIAGNOSTICS.keys()).toHaveLength(MAX_DIAGNOSTIC_UPLOADS_PER_DAY); + }); + + it("counts the fleet budget per UTC day, not per rolling window", async () => { + const env = makeEnv({ DIAGNOSTICS_DAILY_GLOBAL_LIMIT: "1" }); + const first = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip: "192.0.2.10" }), + env, + FIXED_CLOCK, + ); + expect(first.status).toBe(200); + const sameDay = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip: "192.0.2.11" }), + env, + FIXED_CLOCK, + ); + expect(sameDay.status).toBe(429); + + const nextDayMs = FIXED_NOW + 86_400_000; + const nextDay = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip: "192.0.2.12" }), + env, + { now: () => nextDayMs }, + ); + expect(nextDay.status).toBe(200); + expect(spentBudget(env, new Date(nextDayMs).toISOString().slice(0, 10))).toBe(1); + }); + + it("treats a configured zero as a kill switch and an unreadable value as the default", async () => { + const off = makeEnv({ DIAGNOSTICS_DAILY_GLOBAL_LIMIT: "0" }); + const refused = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }) }), + off, + FIXED_CLOCK, + ); + expect(refused.status).toBe(429); + expect(await refused.json()).toEqual({ error: "daily diagnostics budget exhausted" }); + // Nothing is written for an upload that was never going to happen. + expect(off.DB.diagnosticsUploadDays.size).toBe(0); + + // A typo must not uncap the bill OR close the route; it falls back. + const typo = makeEnv({ DIAGNOSTICS_DAILY_GLOBAL_LIMIT: "four hundred" }); + const accepted = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }) }), + typo, + FIXED_CLOCK, + ); + expect(accepted.status).toBe(200); + expect(DEFAULT_DIAGNOSTICS_DAILY_GLOBAL_LIMIT).toBe(400); + }); + + it("refuses rather than storing uncounted when the budget cannot be claimed", async () => { + // Fail closed on purpose: a ceiling that is skipped whenever D1 hiccups is + // not a ceiling, and this route is the least critical thing the Worker does. + const env = makeEnv(); + env.DB.prepare = () => { + throw new Error("no such table: diagnostics_upload_days"); + }; + const { result: response, lines } = await captureUploadLines(() => + handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }) }), + env, + FIXED_CLOCK, + ) + ); + expect(response.status).toBe(503); + expect(await response.json()).toEqual({ error: "diagnostics upload unavailable" }); + expect(env.DIAGNOSTICS.keys()).toHaveLength(0); + expect(lines).toHaveLength(1); + expect(lines[0]).toMatchObject({ status: 503, reason: "budget_unavailable" }); + }); + + it("gives the budget slot back when the store refuses the write", async () => { + // The claim has to precede the put, so an R2 outage would otherwise burn the + // day's ceiling on reports nobody can ever read. + const env = makeEnv({ DIAGNOSTICS_DAILY_GLOBAL_LIMIT: "1" }); + env.DIAGNOSTICS.putFailure = new Error("R2 unavailable"); + const failed = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip: "192.0.2.20" }), + env, + FIXED_CLOCK, + ); + expect(failed.status).toBe(502); + expect(spentBudget(env)).toBe(0); + + // Proof the refund is real and not just a decremented number: the slot it + // returned is spendable by the next upload. + env.DIAGNOSTICS.putFailure = null; + const accepted = await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT }), ip: "192.0.2.21" }), + env, + FIXED_CLOCK, + ); + expect(accepted.status).toBe(200); + expect(spentBudget(env)).toBe(1); + }); + + it("stores and logs the auto flag and failure code an automatic send carries", async () => { + const env = makeEnv(); + const { result: response, lines } = await captureUploadLines(() => + handleDiagnosticsRequest( + uploadRequest({ + body: JSON.stringify({ + report: REPORT, + installId: "install-auto", + auto: true, + failureCode: "brain_start_timeout", + }), + }), + env, + FIXED_CLOCK, + ) + ); + expect(response.status).toBe(200); + const stored = env.DIAGNOSTICS.objects.get(env.DIAGNOSTICS.keys()[0]!)!; + expect(stored.customMetadata).toEqual({ + installId: "install-auto", + auto: "true", + failureCode: "brain_start_timeout", + }); + expect(lines).toHaveLength(1); + expect(lines[0]).toMatchObject({ + outcome: "stored", + auto: true, + failureCode: "brain_start_timeout", + }); + }); + + it("reads the auto flag and failure code from query parameters for a text/plain send", async () => { + const env = makeEnv(); + const response = await handleDiagnosticsRequest( + uploadRequest({ + body: REPORT, + contentType: "text/plain; charset=utf-8", + url: `${UPLOAD_URL}?auto=1&failureCode=sync-handshake_9`, + }), + env, + FIXED_CLOCK, + ); + expect(response.status).toBe(200); + expect(env.DIAGNOSTICS.objects.get(env.DIAGNOSTICS.keys()[0]!)!.customMetadata).toEqual({ + auto: "true", + failureCode: "sync-handshake_9", + }); + }); + + it("records a manual send as manual and leaves its metadata untouched", async () => { + const env = makeEnv(); + const { result: response, lines } = await captureUploadLines(() => + handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT, auto: false }) }), + env, + FIXED_CLOCK, + ) + ); + expect(response.status).toBe(200); + // Absent means manual: an upload from a sender that predates the flag stores + // exactly the metadata it always did. + expect(env.DIAGNOSTICS.objects.get(env.DIAGNOSTICS.keys()[0]!)!.customMetadata).toEqual({}); + // Logged as false rather than omitted, so "how many sends were automatic" + // is a ratio with both sides present. + expect(lines[0]).toMatchObject({ auto: false }); + expect(lines[0]).not.toHaveProperty("failureCode"); + }); + + it("drops a failure code that does not match the shape without refusing the report", async () => { + // The label is cosmetic; the report is not. Failing the upload over a bad + // one would lose exactly the diagnostics the auto-send path exists to + // collect. + const env = makeEnv(); + const malformed = [ + "Brain_Start", + "9lives", + "has space", + "trailing!", + "x".repeat(49), + "", + ]; + for (const failureCode of malformed) { + const { result: response, lines } = await captureUploadLines(() => + handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT, auto: true, failureCode }) }), + env, + FIXED_CLOCK, + ) + ); + expect(response.status).toBe(200); + expect(lines[0]).not.toHaveProperty("failureCode"); + } + for (const key of env.DIAGNOSTICS.keys()) { + expect(env.DIAGNOSTICS.objects.get(key)!.customMetadata).toEqual({ auto: "true" }); + } + expect(env.DIAGNOSTICS.keys()).toHaveLength(malformed.length); + + // The boundary the shape does allow: 48 characters, the first a letter. + const longest = `a${"b".repeat(47)}`; + await handleDiagnosticsRequest( + uploadRequest({ body: JSON.stringify({ report: REPORT, failureCode: longest }) }), + env, + FIXED_CLOCK, + ); + expect(env.DIAGNOSTICS.objects.get(env.DIAGNOSTICS.keys().at(-1)!)!.customMetadata).toEqual({ + failureCode: longest, + }); + }); + + it("carries the auto flag onto the refusal lines, not just the stored one", async () => { + // The day the fleet budget is first exhausted, "which failure is generating + // all this traffic" has to be answerable from the refusals. + const env = makeEnv({ DIAGNOSTICS_DAILY_GLOBAL_LIMIT: "0" }); + const { lines } = await captureUploadLines(() => + handleDiagnosticsRequest( + uploadRequest({ + body: JSON.stringify({ report: REPORT, auto: true, failureCode: "sync_wedged" }), + }), + env, + FIXED_CLOCK, + ) + ); + expect(lines).toHaveLength(1); + expect(lines[0]).toMatchObject({ + outcome: "rejected", + status: 429, + reason: "global_budget_exhausted", + auto: true, + failureCode: "sync_wedged", + }); + }); + it("is reachable through the Worker entry point without account authentication", async () => { const env = makeEnv(); const response = await worker.fetch( @@ -494,3 +922,54 @@ describe("diagnostics upload route", () => { expect(env.DIAGNOSTICS.keys()).toHaveLength(1); }); }); + +describe("diagnostics budget sweep", () => { + function seedDays(env: { DB: FakeD1Database }, days: Record): void { + for (const [day, count] of Object.entries(days)) env.DB.diagnosticsUploadDays.set(day, count); + } + + function dayOffset(days: number): string { + return new Date(FIXED_NOW + days * 86_400_000).toISOString().slice(0, 10); + } + + it("prunes budget rows past the retention window and never today's", async () => { + const env = makeEnv(); + seedDays(env, { + [dayOffset(-30)]: 400, + [dayOffset(-8)]: 12, + [dayOffset(-DIAGNOSTICS_BUDGET_RETENTION_DAYS)]: 7, + [dayOffset(-1)]: 3, + [FIXED_DAY_KEY]: 5, + }); + + const removed = await cleanupDiagnosticsUploadDays(env, FIXED_NOW); + + expect(removed).toBe(2); + // The cutoff day itself is kept — `day < cutoff`, so retention is inclusive + // — and today is never in range, which is what stops a sweep from handing + // back budget the running day has already spent. + expect([...env.DB.diagnosticsUploadDays.keys()].sort()).toEqual( + [dayOffset(-DIAGNOSTICS_BUDGET_RETENTION_DAYS), dayOffset(-1), FIXED_DAY_KEY].sort(), + ); + expect(env.DB.diagnosticsUploadDays.get(FIXED_DAY_KEY)).toBe(5); + }); + + it("runs from the Worker's scheduled handler alongside the other sweeps", async () => { + // The cron is the only thing that stops this table from growing a row a day + // forever, so it has to be wired into the handler, not just exported. + const env = makeEnv(); + // The handler passes no clock, so "today" here is the real one. + const today = new Date().toISOString().slice(0, 10); + seedDays(env, { "2020-01-01": 9, [today]: 2 }); + let cleanup: Promise = Promise.resolve(); + await worker.scheduled( + {} as ScheduledEvent, + env, + { waitUntil: (promise) => { cleanup = promise; } } as ExecutionContext, + ); + await cleanup; + + expect(env.DB.diagnosticsUploadDays.has("2020-01-01")).toBe(false); + expect(env.DB.diagnosticsUploadDays.get(today)).toBe(2); + }); +}); diff --git a/apps/account-directory/test/fakeD1.ts b/apps/account-directory/test/fakeD1.ts index 51fb598d2..f5af32cd9 100644 --- a/apps/account-directory/test/fakeD1.ts +++ b/apps/account-directory/test/fakeD1.ts @@ -100,6 +100,8 @@ export class FakeD1Database { deviceRows: StoredDeviceAuthorization[] = []; pairingGrants: StoredPairingGrant[] = []; approvalRateLimits = new Map(); + /** `diagnostics_upload_days`: the fleet-wide diagnostics budget, one row per UTC day. */ + diagnosticsUploadDays = new Map(); private rateLimitReadBarrier: { remaining: number; promise: Promise; @@ -224,6 +226,50 @@ export class FakeD1Database { run(sql: string, values: unknown[]): number { const normalized = sql.toLowerCase(); + if (normalized.includes("insert into diagnostics_upload_days")) { + // The fleet budget claim. Mirrors the upsert's `where count < ?` exactly, + // because that predicate IS the cap: a worker that drops it (or checks the + // count in a separate read first) stores past the limit, and the tests + // must see that rather than have the fake absorb it. + const [day, limit] = values; + const key = String(day); + const stored = this.diagnosticsUploadDays.get(key); + if (stored === undefined) { + this.diagnosticsUploadDays.set(key, 1); + return 1; + } + // Read off the STATEMENT, not off the bind values: the cap lives in that + // predicate, and a worker that moved the check into a separate read — or + // dropped it — would otherwise be absorbed here and store past the limit + // with every test still green. + const capped = /where\s+count\s*<\s*\?/.test(normalized); + if (capped && stored >= Number(limit)) return 0; + this.diagnosticsUploadDays.set(key, stored + 1); + return 1; + } + if (normalized.includes("update diagnostics_upload_days")) { + // The refund taken when the R2 write the slot was claimed for failed. + // `count > 0` is mirrored so a refund can never drive the row negative + // and hand out budget nobody claimed. + const [day] = values; + const key = String(day); + const stored = this.diagnosticsUploadDays.get(key); + const floored = /count\s*>\s*0/.test(normalized); + if (stored === undefined || (floored && stored <= 0)) return 0; + this.diagnosticsUploadDays.set(key, stored - 1); + return 1; + } + if (normalized.includes("delete from diagnostics_upload_days")) { + // Cron sweep. Lexicographic on a fixed-width ISO date, same as SQLite. + const cutoff = String(values[0]); + let changes = 0; + for (const day of [...this.diagnosticsUploadDays.keys()]) { + if (day >= cutoff) continue; + this.diagnosticsUploadDays.delete(day); + changes += 1; + } + return changes; + } if (normalized.includes("insert into machine_pairing_grants")) { const [grantHash, userId, machineKey, createdAt, expiresAt] = values; if (this.pairingGrants.some((row) => row.grant_hash === grantHash)) return 0; diff --git a/apps/account-directory/wrangler.jsonc b/apps/account-directory/wrangler.jsonc index 2d9318f30..3fa5a00f7 100644 --- a/apps/account-directory/wrangler.jsonc +++ b/apps/account-directory/wrangler.jsonc @@ -31,7 +31,15 @@ // to skip the public edge. PUSH_RELAY_URL is still required — it is what // builds the request URL — and DIRECTORY_AUTH_SECRET is still required, // because a service binding carries no provenance the relay can verify. - "PUSH_RELAY_URL": "https://ade-push-relay.arulsharma1028.workers.dev" + "PUSH_RELAY_URL": "https://ade-push-relay.arulsharma1028.workers.dev", + // Fleet-wide diagnostic uploads STORED per UTC day. This Worker is the only + // writer the diagnostics bucket has, so this number is the storage bill: + // 400 × 512 KB × the bucket's 30-day expiry lifecycle ≈ 6 GB steady state, + // inside R2's 10 GB free tier. Clients auto-send reports on failure, so + // raising it is a deliberate act with arithmetic attached (README, + // "Diagnostic report uploads"). `0` stops every upload without a code + // deploy; unset or unparseable falls back to the same 400. + "DIAGNOSTICS_DAILY_GLOBAL_LIMIT": "400" }, "d1_databases": [ { @@ -59,7 +67,11 @@ "vars": { "ONLINE_WINDOW_MS": "90000", "WEB_CLIENT_ORIGIN": "https://app.ade-app.dev", - "PUSH_RELAY_URL": "https://ade-push-relay.arulsharma1028.workers.dev" + "PUSH_RELAY_URL": "https://ade-push-relay.arulsharma1028.workers.dev", + // Same ceiling as the default environment, and set explicitly: wrangler + // environments do not inherit `vars`, so an omission here would silently + // fall back to the code default rather than to the value above. + "DIAGNOSTICS_DAILY_GLOBAL_LIMIT": "400" }, "d1_databases": [ { diff --git a/apps/ade-cli/README.md b/apps/ade-cli/README.md index dac3c0663..d5d77c70a 100644 --- a/apps/ade-cli/README.md +++ b/apps/ade-cli/README.md @@ -771,6 +771,7 @@ status row (`ok` / `warn` / `fail`) per check. It exits non-zero when any row is - **Relay** — relay route health as already computed by the brain. `ok` when the relay control is connected, the bridge is validated, and the end-to-end round-trip is verified; `fail` when the route is not fully validated; `warn` when relay is disabled or route health is unavailable. When another ADE process on this machine has claimed the relay slot, the brain deliberately stops redialing and this row reports that suppression ahead of any lower-level close error, so the detail names the fix (quit the rival process) instead of the symptom. `ade sync status --text` shows the same reason on its `relay` line, plus a `relay failing since` row for how long the current outage has run. - **Account** — whether this machine's brain is signed in to an ADE account (and the credential source), read via the brain's `account.call status`. `warn` when signed out or unavailable. - **Credentials** — whether the shared credential store (`$ADE_HOME/secrets/credentials.json.enc`) can be read, and whether an unreadable one was set aside earlier. `fail` when it cannot be read, naming the next step: a store sealed with a key this process cannot obtain is unlocked by opening the ADE app on this computer, while anything else needs a fresh sign-in. `warn` when a quarantined file is still waiting to be restored. Unlike every other row, this one is read **straight from disk** rather than through the brain — the failure it exists for is a brain that cannot start, so a check that needed a running brain would be silent exactly when it matters. It is non-creating: it never mints a machine key or OS key material, so running the diagnostic cannot change the state it reports. `ade brain repair-credentials` acts on the same reading. +- **Diagnostics sharing** — whether ADE may send a redacted diagnostic report by itself when something fails, and how much of today's ceiling has been spent (`on · 1 of 3 automatic reports sent today` / `off`). Read straight from the ledger both senders account against (`$ADE_HOME/secrets/diagnostics-autosend.json`), which is why the count covers the whole computer rather than one process. Always `ok`: it is a preference, not a health check. An absent or unreadable ledger reports the default the next auto-send would act on — on, nothing spent. The setting itself is toggled in the desktop app; there is no CLI flag for it. When a row fails and the checks above do not explain it, `ade doctor --text` points at `ade report-issue`. That command is the headless counterpart to the diff --git a/apps/ade-cli/src/cli.ts b/apps/ade-cli/src/cli.ts index 1a8be3329..078f46f47 100644 --- a/apps/ade-cli/src/cli.ts +++ b/apps/ade-cli/src/cli.ts @@ -16948,6 +16948,17 @@ async function runServe( runtimeMode: "brain", }), ); + /** + * The brain's half of automatic diagnostics. It shares the desktop's consent + * flag and daily budget on disk, and reads the machine's credential store, so + * a report from a headless box lands attributed to the account. + */ + const { createBrainAutoDiagnostics } = await import("./services/diagnostics/autoDiagnosticsSender"); + const brainAutoDiagnostics = createBrainAutoDiagnostics({ + cliVersion: VERSION, + logger: headlessProjectLogger, + capture: (input) => brainProductAnalytics.capture(input), + }); stopBrainLoopWatchdog = startBrainLoopWatchdog({ runtimeDir: layout.runtimeDir, warn: (event, meta) => headlessProjectLogger.warn(event, meta), @@ -17993,6 +18004,21 @@ async function runServe( captureAnalytics: (input) => { brainProductAnalytics.captureInternal(input); }, + // A machine that has been unable to publish for minutes has silently + // dropped out of the account directory, and on a headless box there is + // nobody to press "Report issue" about it. + onSustainedFailure: ({ code }) => { + // `report` is documented never to reject; the catch is what keeps a + // silent-by-design path from ever becoming an unhandled rejection + // that takes the brain down. + void brainAutoDiagnostics + .report({ + failureCode: code, + surface: "account_publisher", + headline: "This computer could not publish to your account", + }) + .catch(() => undefined); + }, }); accountMachinePublisher.start(); }; @@ -18059,6 +18085,16 @@ async function runServe( }, budget: machineCloudRelayStore, logger: headlessProjectLogger, + // The loop has stopped arguing and this computer is still disconnected. + onGaveUp: ({ code }) => { + void brainAutoDiagnostics + .report({ + failureCode: code, + surface: "machine_pairing_recovery", + headline: "This computer could not reconnect to your account", + }) + .catch(() => undefined); + }, }); machinePairingAutoRecovery.start(); } diff --git a/apps/ade-cli/src/commands/doctor.test.ts b/apps/ade-cli/src/commands/doctor.test.ts index c1ec7529d..0b5fc9172 100644 --- a/apps/ade-cli/src/commands/doctor.test.ts +++ b/apps/ade-cli/src/commands/doctor.test.ts @@ -1,3 +1,6 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { compareDoctorVersions, @@ -5,6 +8,7 @@ import { evaluateDoctorRows, parseWindowsDesktopInstallProbe, probeDoctorBrain, + readAutoDiagnosticsSharingForDoctor, type DoctorInput, } from "./doctor"; import { createSyncAccountDirectoryHealth } from "../../../desktop/src/shared/types/sync"; @@ -190,6 +194,7 @@ describe("doctor row evaluation", () => { ["relay", "ok"], ["account", "ok"], ["credentials", "ok"], + ["diagnostics", "ok"], ]); }); @@ -443,6 +448,71 @@ describe("doctor row evaluation", () => { expect(relay?.detail).toBe("Relay echo never came back."); }); + it("states diagnostics sharing as a preference, never as a problem", () => { + const on = healthyInput(); + on.diagnostics = { enabled: true, sendsInWindow: 1, limit: 3 }; + const off = healthyInput(); + off.diagnostics = { enabled: false, sendsInWindow: 0, limit: 3 }; + + expect(evaluateDoctorRows(on).find((row) => row.key === "diagnostics")).toEqual({ + key: "diagnostics", + label: "Diagnostics sharing", + status: "ok", + detail: "on · 1 of 3 automatic reports sent today", + }); + // "Off" is a choice the user made, so it stays green: a diagnostic that + // paints a respected preference yellow trains people to ignore the colour. + expect(evaluateDoctorRows(off).find((row) => row.key === "diagnostics")).toEqual({ + key: "diagnostics", + label: "Diagnostics sharing", + status: "ok", + detail: "off · no automatic reports are sent", + }); + // Omitted by a caller that did not read the ledger: say so, do not guess. + expect(evaluateDoctorRows(healthyInput()).find((row) => row.key === "diagnostics")) + .toMatchObject({ status: "ok", detail: "not checked" }); + }); + + it("reads diagnostics sharing off the shared ledger, defaulting on when it is absent or unreadable", () => { + const adeDir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-doctor-diagnostics-")); + try { + const statePath = path.join(adeDir, "secrets", "diagnostics-autosend.json"); + fs.mkdirSync(path.dirname(statePath), { recursive: true }); + + // Never auto-sent: the setting is on and the budget is untouched. + expect(readAutoDiagnosticsSharingForDoctor(adeDir, {})).toEqual({ + enabled: true, + sendsInWindow: 0, + limit: 3, + }); + + fs.writeFileSync( + statePath, + JSON.stringify({ + version: 1, + enabled: false, + sends: [{ code: "brain_wedge", atMs: Date.now(), source: "brain", pending: false }], + }), + ); + expect(readAutoDiagnosticsSharingForDoctor(adeDir, {})).toMatchObject({ + enabled: false, + sendsInWindow: 1, + }); + + // Unreadable is reported as the default the next auto-send would act on, + // rather than as a failure of the machine's health. + fs.rmSync(statePath); + fs.writeFileSync(statePath, "{ not json"); + expect(readAutoDiagnosticsSharingForDoctor(adeDir, {})).toEqual({ + enabled: true, + sendsInWindow: 0, + limit: 3, + }); + } finally { + fs.rmSync(adeDir, { recursive: true, force: true }); + } + }); + it("compares release versions without depending on tag formatting", () => { expect(compareDoctorVersions("v1.2.36", "1.2.35")).toBe(1); expect(compareDoctorVersions("1.2.35", "v1.2.35")).toBe(0); diff --git a/apps/ade-cli/src/commands/doctor.ts b/apps/ade-cli/src/commands/doctor.ts index b3f13f966..9b25cdd15 100644 --- a/apps/ade-cli/src/commands/doctor.ts +++ b/apps/ade-cli/src/commands/doctor.ts @@ -20,6 +20,10 @@ import { inspectCredentialStoreHealth, type CredentialStoreHealth, } from "../services/credentials/credentialStore"; +import { + readAutoDiagnosticsState, + resolveAutoDiagnosticsStateFile, +} from "../../../desktop/src/main/services/diagnostics/autoDiagnosticsStore"; import { resolveMachineAdeLayout } from "../services/projects/machineLayout"; import { readBrainStartupState } from "../services/runtime/brainStartupState"; import { DEFAULT_SYNC_HOST_PORT } from "../services/sync/syncProtocol"; @@ -38,7 +42,8 @@ export type DoctorRow = { | "publish" | "relay" | "account" - | "credentials"; + | "credentials" + | "diagnostics"; label: string; status: DoctorRowStatus; detail: string; @@ -97,8 +102,19 @@ export type DoctorInput = { * running brain to answer would be silent in exactly the case it is for. */ credentials: CredentialStoreHealth | null; + /** + * Automatic diagnostics sharing: the consent flag and today's spend, read off + * the same ledger both senders account against. + * + * OPTIONAL on purpose. This is an additive input, and a caller that does not + * supply it gets a truthful "not checked" row rather than a fabricated one. + */ + diagnostics?: DoctorDiagnosticsSharing | null; }; +/** What the shared auto-diagnostics ledger says, verbatim. */ +export type DoctorDiagnosticsSharing = ReturnType; + export type DoctorCommandOptions = { role: "cto" | "orchestrator" | "agent" | "external" | "evaluator"; socketPath: string | null; @@ -158,6 +174,7 @@ export type DoctorCommandResult = { relayHealth: DoctorInput["relayHealth"]; account: DoctorInput["account"]; credentials: DoctorInput["credentials"]; + diagnostics: DoctorInput["diagnostics"]; }; type DoctorBrainProbe = { @@ -864,6 +881,47 @@ function credentialsRow(health: CredentialStoreHealth | null): DoctorRow { }; } +/** + * The consent flag and today's spend, read off disk through the store itself. + * + * Deliberately NOT a second parser: `readAutoDiagnosticsState` is the same + * reader the desktop settings pane and both senders use, and its documented + * degradation — an absent or unparseable ledger reads as the default (on, no + * sends spent) — is the honest answer here too, because that is exactly what + * the next auto-send would act on. + */ +export function readAutoDiagnosticsSharingForDoctor( + adeDir: string, + env: NodeJS.ProcessEnv = process.env, +): DoctorDiagnosticsSharing | null { + try { + return readAutoDiagnosticsState(resolveAutoDiagnosticsStateFile(adeDir, env)); + } catch { + return null; + } +} + +/** + * A preference, not a health check, so it is never `warn` or `fail`. + * + * "Off" is a state the user chose, and a diagnostic that paints a respected + * choice yellow teaches people to ignore the colour. + */ +function diagnosticsRow(sharing: DoctorInput["diagnostics"]): DoctorRow { + const label = "Diagnostics sharing"; + if (!sharing) { + return { key: "diagnostics", label, status: "ok", detail: "not checked" }; + } + return { + key: "diagnostics", + label, + status: "ok", + detail: sharing.enabled + ? `on · ${sharing.sendsInWindow} of ${sharing.limit} automatic reports sent today` + : "off · no automatic reports are sent", + }; +} + export function evaluateDoctorRows(input: DoctorInput): DoctorRow[] { return [ appRow(input.app), @@ -874,6 +932,7 @@ export function evaluateDoctorRows(input: DoctorInput): DoctorRow[] { relayRow(input.relayHealth), accountRow(input.account), credentialsRow(input.credentials), + diagnosticsRow(input.diagnostics), ]; } @@ -948,6 +1007,7 @@ export async function runDoctorCommand( relayHealth, account: brainProbe.account, credentials: readCredentialStoreHealthForDoctor(layout.secretsDir), + diagnostics: readAutoDiagnosticsSharingForDoctor(layout.adeDir), }; const rows = evaluateDoctorRows(input); return { @@ -964,5 +1024,6 @@ export async function runDoctorCommand( relayHealth, account: input.account, credentials: input.credentials, + diagnostics: input.diagnostics, }; } diff --git a/apps/ade-cli/src/commands/reportIssue.ts b/apps/ade-cli/src/commands/reportIssue.ts index 65a1e69b4..33cfb1393 100644 --- a/apps/ade-cli/src/commands/reportIssue.ts +++ b/apps/ade-cli/src/commands/reportIssue.ts @@ -33,6 +33,13 @@ export type ReportIssueOptions = { surface?: string; projectRoot?: string | null; cliVersion?: string | null; + /** + * The failure this report is about, when the caller already knows it. The + * interactive command does not (a person pressed "report", not a subsystem), + * so it stays null there; the automatic sender always has one. + */ + code?: string | null; + headline?: string | null; env?: NodeJS.ProcessEnv; now?: () => Date; }; @@ -100,8 +107,8 @@ export function buildCliDiagnosticReport(options: ReportIssueOptions = {}): Repo identity: { installId }, context: { surface, - headline: null, - code: null, + headline: options.headline?.trim().slice(0, 300) || null, + code: options.code?.trim().slice(0, 120) || null, technicalDetail: null, projectRoot, }, @@ -119,6 +126,8 @@ export function buildCliDiagnosticReport(options: ReportIssueOptions = {}): Repo secretsDir: sources.layout.secretsDir, issueUrl: buildDiagnosticIssueUrl({ surface, + headline: options.headline?.trim().slice(0, 300) || null, + code: options.code?.trim().slice(0, 120) || null, appVersion: options.cliVersion ?? null, platform: process.platform, arch: process.arch, @@ -186,6 +195,9 @@ export async function sendDiagnosticReport( /** Test seam; production resolves the token from the credential store. */ getToken?: () => Promise; fetchImpl?: typeof fetch; + /** Set by the automatic sender; `ade report-issue --send` leaves it off. */ + auto?: boolean; + failureCode?: string | null; } = {}, ): Promise { const env = deps.env ?? process.env; @@ -217,6 +229,8 @@ export async function sendDiagnosticReport( token, installId: built.installId === "unknown" ? null : built.installId, appVersion: built.appVersion, + auto: deps.auto === true, + failureCode: deps.failureCode ?? null, fetchImpl: deps.fetchImpl, }); } diff --git a/apps/ade-cli/src/services/account/accountMachinePublisherService.test.ts b/apps/ade-cli/src/services/account/accountMachinePublisherService.test.ts index 72658574f..b0642c777 100644 --- a/apps/ade-cli/src/services/account/accountMachinePublisherService.test.ts +++ b/apps/ade-cli/src/services/account/accountMachinePublisherService.test.ts @@ -606,6 +606,51 @@ describe("account machine publisher health", () => { expect(captureAnalytics).toHaveBeenCalledTimes(2); }); + it("reports a sustained publish failure once per episode, five minutes in", async () => { + let clock = 0; + let succeeds = false; + const onSustainedFailure = vi.fn(); + const service = createAccountMachinePublisherService({ + getAccessToken: async () => "account-token", + getAccountStatus: () => ({ signedIn: true, sessionReadState: "available" as const }), + getSnapshot: async () => snapshot(), + getMachineKey: () => "machine-studio", + directoryBaseUrl: () => "https://directory.example", + fetchImpl: vi.fn(async () => succeeds + ? new Response(null, { status: 204 }) + : new Response(null, { status: 503 })), + now: () => clock, + onSustainedFailure, + }); + + await service.publishNow(); + // Four minutes of failing is a bad afternoon, not yet a broken machine. + clock = 240_000; + await service.publishNow(); + expect(onSustainedFailure).not.toHaveBeenCalled(); + + clock = 301_000; + await service.publishNow(); + expect(onSustainedFailure).toHaveBeenCalledTimes(1); + expect(onSustainedFailure).toHaveBeenCalledWith({ code: "http_error" }); + + // Still failing: one report per episode, never one per attempt. + clock = 600_000; + await service.publishNow(); + expect(onSustainedFailure).toHaveBeenCalledTimes(1); + + // Recovered and broken again: a genuinely new episode may report again. + succeeds = true; + clock = 700_000; + await service.publishNow(); + succeeds = false; + clock = 800_000; + await service.publishNow(); + clock = 1_101_000; + await service.publishNow(); + expect(onSustainedFailure).toHaveBeenCalledTimes(2); + }); + it("captures one account-session-unreadable event per unreadable episode", async () => { let sessionReadState: "available" | "unreadable" = "unreadable"; const captureAnalytics = vi.fn(); diff --git a/apps/ade-cli/src/services/account/accountMachinePublisherService.ts b/apps/ade-cli/src/services/account/accountMachinePublisherService.ts index 21d84327b..8699a5c19 100644 --- a/apps/ade-cli/src/services/account/accountMachinePublisherService.ts +++ b/apps/ade-cli/src/services/account/accountMachinePublisherService.ts @@ -62,6 +62,16 @@ const PUBLISH_INFO_INTERVAL = 10; */ const PRE_SUSPEND_PUBLISH_BUDGET_MS = 2_000; export const PUBLISH_FAILURE_ANALYTICS_THRESHOLD_MS = 120_000; +/** + * How long the publish leg must keep failing before it is worth a diagnostic + * report rather than just a coarse analytics event. + * + * Deliberately its own constant rather than a reuse of the analytics threshold + * above: that one is tuned to "measurable", this one to "a human would call + * this broken", and the analytics event's `failing_minutes` floor is coupled to + * its own number. + */ +export const PUBLISH_FAILURE_DIAGNOSTICS_THRESHOLD_MS = 5 * 60_000; export type AccountMachineRegistration = { machineKey: string; @@ -619,6 +629,17 @@ export function createAccountMachinePublisherService(options: { now?: () => number; logger?: AccountMachinePublisherLogger; captureAnalytics?: (input: ProductAnalyticsCapture) => void; + /** + * The publish leg has been failing for longer than a person would tolerate. + * Fired once per failure episode with the health state as the code, so + * automatic diagnostics can send one report for a machine that has quietly + * dropped out of the account directory. + * + * Carries only the code. How long it had been failing is already on the + * episode's log line and on the analytics event beside it, and the threshold + * that decides "long enough" lives here rather than in a listener. + */ + onSustainedFailure?: (input: { code: SyncAccountDirectoryHealth["state"] }) => void; }) { const heartbeatMs = Math.max(1_000, Math.floor(options.heartbeatMs ?? ACCOUNT_MACHINE_HEARTBEAT_MS)); const relayStatePollMs = Math.max( @@ -692,6 +713,8 @@ export function createAccountMachinePublisherService(options: { "Account-directory publishing has not started.", ); + /** Edge-triggered like the analytics episode, on its own longer threshold. */ + let sustainedFailureReported = false; const captureAnalytics = () => options.captureAnalytics; const publishFailureAnalytics = createEpisodeAnalytics({ event: "ade_publish_failing", @@ -830,7 +853,22 @@ export function createAccountMachinePublisherService(options: { }; if (health.failingSinceMs == null) { publishFailureAnalytics.end(); - } else if (args.attemptAt - health.failingSinceMs >= PUBLISH_FAILURE_ANALYTICS_THRESHOLD_MS) { + sustainedFailureReported = false; + } else { + const failingMs = args.attemptAt - health.failingSinceMs; + if (!sustainedFailureReported && failingMs >= PUBLISH_FAILURE_DIAGNOSTICS_THRESHOLD_MS) { + // Set before the call so a throwing listener still consumes the + // episode: this must never become a per-attempt loop. + sustainedFailureReported = true; + try { + options.onSustainedFailure?.({ code: state }); + } catch { + // Best effort; health recording is what matters here. + } + } + } + if (health.failingSinceMs != null + && args.attemptAt - health.failingSinceMs >= PUBLISH_FAILURE_ANALYTICS_THRESHOLD_MS) { publishFailureAnalytics.report({ dedupeValue: health.failingSinceMs, properties: { @@ -1739,6 +1777,8 @@ export function createBrainAccountMachinePublisherService(options: { directoryBaseUrl?: () => string | null | undefined; logger: BrainAccountMachinePublisherLogger; captureAnalytics?: (input: ProductAnalyticsCapture) => void; + /** See the same option on `createAccountMachinePublisherService`. */ + onSustainedFailure?: (input: { code: SyncAccountDirectoryHealth["state"] }) => void; /** * Override the machine power source. Tests pass `null` to keep the brain's * poll and gap timers out of a suite; a host with a precise suspend hook can @@ -1822,5 +1862,6 @@ export function createBrainAccountMachinePublisherService(options: { subscribeToSignIn: (listener) => accountAuthService.onSignedIn(listener), logger: options.logger, captureAnalytics: options.captureAnalytics, + onSustainedFailure: options.onSustainedFailure, }); } diff --git a/apps/ade-cli/src/services/account/machinePairingAutoRecovery.test.ts b/apps/ade-cli/src/services/account/machinePairingAutoRecovery.test.ts index 03ae9a86b..bd33397b3 100644 --- a/apps/ade-cli/src/services/account/machinePairingAutoRecovery.test.ts +++ b/apps/ade-cli/src/services/account/machinePairingAutoRecovery.test.ts @@ -47,6 +47,7 @@ function harness(options: { repair: () => Promise; hasAccountSession?: () => boolean; budgetLimit?: number; + onGaveUp?: (input: { code: string }) => void; now: () => number; }) { let spent = 0; @@ -69,6 +70,7 @@ function harness(options: { return { allowed: true, countInWindow: spent, limit }; }, }, + onGaveUp: options.onGaveUp, now: options.now, }); return { recovery, spentRepairs: () => spent }; @@ -150,6 +152,71 @@ describe("machinePairingAutoRecovery", () => { expect(recovery.getState().settled).toBe(true); }); + it("reports giving up once, with the refusal code, when the budget runs out", async () => { + let clock = 0; + const onGaveUp = vi.fn(); + const { recovery } = harness({ + health: () => REFUSED_HEALTH("machine_revoked"), + revoked: () => true, + repair: async () => REPAIR_FAILED, + budgetLimit: 1, + onGaveUp, + now: () => clock, + }); + + await recovery.tick(); + for (let attempt = 0; attempt < 4; attempt += 1) { + clock += 24 * 60 * 60 * 1_000; + await recovery.tick(); + } + + // Once for the episode, not once per tick: this machine is disconnected and + // saying so repeatedly is what the send budget exists to prevent. + expect(onGaveUp).toHaveBeenCalledTimes(1); + expect(onGaveUp).toHaveBeenCalledWith({ code: "machine_revoked" }); + }); + + it("reports giving up after the single snapshot_failed cycle does not fix it", async () => { + let clock = 0; + const onGaveUp = vi.fn(); + const { recovery } = harness({ + health: () => createSyncAccountDirectoryHealth("snapshot_failed", "No snapshot.", { + lastAttemptAt: 0, + failingSinceMs: 0, + }), + revoked: () => false, + repair: async () => REPAIR_FAILED, + onGaveUp, + now: () => clock, + }); + + clock = PAIRING_AUTO_REPAIR_SNAPSHOT_GRACE_MS + 1; + // First tick opens the episode; the repair itself runs after the first delay. + await recovery.tick(); + clock += PAIRING_AUTO_REPAIR_DELAYS_MS[0] + 1; + await recovery.tick(); + + expect(onGaveUp).toHaveBeenCalledWith({ code: "snapshot_failed" }); + }); + + it("says nothing when the machine recovers on its own", async () => { + let clock = 0; + const onGaveUp = vi.fn(); + const { recovery } = harness({ + health: () => REFUSED_HEALTH("machine_revoked"), + revoked: () => true, + repair: async () => REPAIR_OK, + onGaveUp, + now: () => clock, + }); + + await recovery.tick(); + clock += 1; + await recovery.tick(); + + expect(onGaveUp).not.toHaveBeenCalled(); + }); + it("waits for a signed-in session instead of burning the budget proving there is none", async () => { let clock = 0; let signedIn = false; diff --git a/apps/ade-cli/src/services/account/machinePairingAutoRecovery.ts b/apps/ade-cli/src/services/account/machinePairingAutoRecovery.ts index c7e9b7ad5..e7a22f5ea 100644 --- a/apps/ade-cli/src/services/account/machinePairingAutoRecovery.ts +++ b/apps/ade-cli/src/services/account/machinePairingAutoRecovery.ts @@ -90,6 +90,19 @@ export type MachinePairingAutoRecoveryArgs = { hasAccountSession: () => boolean; budget: MachinePairingAutoRecoveryBudget; logger?: AutoRecoveryLogger; + /** + * The episode ended without recovering: the budget ran out, or the single + * allowed `snapshot_failed` cycle did not fix it. This is the point where the + * loop stops arguing and a machine stays disconnected with nobody at the + * console to notice, which is exactly the report worth having. + * + * Called at most once per episode, and never for an episode that recovered. + * + * Carries only the refusal code. `trigger` and `attempts` are already in the + * episode's own log lines, and a notification callback that ships the whole + * episode invites a listener to start making decisions from it. + */ + onGaveUp?: (input: { code: string }) => void; /** Test seams. */ pollMs?: number; delaysMs?: readonly number[]; @@ -160,6 +173,14 @@ export function createMachinePairingAutoRecovery( const delayFor = (attempt: number): number => delays[Math.min(attempt, delays.length - 1)] ?? delays[delays.length - 1] ?? DEFAULT_POLL_MS; + const reportGaveUp = (code: string): void => { + try { + args.onGaveUp?.({ code }); + } catch { + // A listener must never change what the recovery loop does next. + } + }; + const endEpisode = (reason: string): void => { if (!episode) return; const previous = episode; @@ -229,6 +250,7 @@ export function createMachinePairingAutoRecovery( code, limit: spend.limit, }); + reportGaveUp(code); } return; } @@ -274,6 +296,7 @@ export function createMachinePairingAutoRecovery( // One cycle only: a publish leg that cannot read a snapshot is not a // pairing problem, and repeating the request will not make it one. episode.settled = true; + reportGaveUp(code); return; } episode.nextAttemptAtMs = now() + delayFor(episode.attempts); diff --git a/apps/ade-cli/src/services/diagnostics/autoDiagnosticsSender.test.ts b/apps/ade-cli/src/services/diagnostics/autoDiagnosticsSender.test.ts new file mode 100644 index 000000000..d5c274771 --- /dev/null +++ b/apps/ade-cli/src/services/diagnostics/autoDiagnosticsSender.test.ts @@ -0,0 +1,118 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + listPendingAutoDiagnosticsNotices, + resolveAutoDiagnosticsStateFile, + setAutoDiagnosticsEnabled, +} from "../../../../desktop/src/main/services/diagnostics/autoDiagnosticsStore"; +import { + createBrainAutoDiagnostics, + type BrainDiagnosticsBuild, + type BrainDiagnosticsSend, +} from "./autoDiagnosticsSender"; + +const dirs: string[] = []; +const T0 = Date.parse("2026-08-19T10:00:00.000Z"); + +function adeHome(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-brain-auto-diagnostics-")); + dirs.push(dir); + return dir; +} + +type SendResult = { ok: true; reference: string } | { ok: false; reason: string }; + +function harness(options: { home?: string; sendResult?: SendResult } = {}) { + const home = options.home ?? adeHome(); + const env = { ADE_HOME: home } as NodeJS.ProcessEnv; + const stateFilePath = resolveAutoDiagnosticsStateFile(home, env); + // No casts: the seams are structural, so these stubs are checked against the + // shape the sender actually calls them with. + const send = vi.fn, ReturnType>( + async () => options.sendResult ?? { ok: true as const, reference: "abcd1234" }, + ); + const build = vi.fn, ReturnType>(() => ({ + report: "# brain report", + installId: "install-1", + appVersion: "1.2.3", + secretsDir: path.join(home, "secrets"), + })); + const writeReportFile = vi.fn((_filePath: string, _report: string) => true); + const sender = createBrainAutoDiagnostics({ + env, + cliVersion: "1.2.3", + now: () => T0, + build, + send, + writeReportFile, + }); + return { sender, stateFilePath, send, build, writeReportFile, home }; +} + +afterEach(() => { + vi.restoreAllMocks(); + for (const dir of dirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); +}); + +describe("createBrainAutoDiagnostics", () => { + it("sends with the failure code and leaves the toast for the desktop to show", async () => { + const { sender, send, build, writeReportFile, stateFilePath, home } = harness(); + + await expect( + sender.report({ failureCode: "machine_revoked", surface: "machine_pairing_recovery" }), + ).resolves.toBe("completed"); + + expect(build.mock.calls[0]?.[0]).toMatchObject({ + surface: "machine_pairing_recovery", + code: "machine_revoked", + cliVersion: "1.2.3", + }); + expect(send.mock.calls[0]?.[1]).toMatchObject({ auto: true, failureCode: "machine_revoked" }); + expect(writeReportFile.mock.calls[0]?.[0]).toContain( + path.join(home, "diagnostic-reports"), + ); + + // No renderer here, so the send waits to be shown rather than vanishing. + expect(listPendingAutoDiagnosticsNotices(stateFilePath)).toEqual([ + { + failureCode: "machine_revoked", + reportPath: writeReportFile.mock.calls[0]?.[0] ?? null, + reference: "abcd1234", + }, + ]); + }); + + it("shares the desktop's off switch", async () => { + const home = adeHome(); + setAutoDiagnosticsEnabled(resolveAutoDiagnosticsStateFile(home, { ADE_HOME: home }), false, { + now: () => T0, + }); + const { sender, send } = harness({ home }); + + await expect(sender.report({ failureCode: "snapshot_failed", surface: "account_publisher" })) + .resolves.toBe("skipped_disabled"); + expect(send).not.toHaveBeenCalled(); + }); + + it("shares the desktop's daily budget rather than keeping its own", async () => { + const { sender, send, stateFilePath } = harness(); + + await expect(sender.report({ failureCode: "snapshot_failed", surface: "account_publisher" })) + .resolves.toBe("completed"); + await expect(sender.report({ failureCode: "snapshot_failed", surface: "account_publisher" })) + .resolves.toBe("skipped_budget"); + expect(send).toHaveBeenCalledTimes(1); + expect(fs.existsSync(stateFilePath)).toBe(true); + }); + + it("treats a refused upload as a silent skip and offers no toast for it", async () => { + const { sender, stateFilePath } = harness({ sendResult: { ok: false, reason: "rate_limited" } }); + + await expect(sender.report({ failureCode: "snapshot_failed", surface: "account_publisher" })) + .resolves.toBe("failed"); + // Nothing succeeded, so there is nothing to tell the user about. + expect(listPendingAutoDiagnosticsNotices(stateFilePath)).toEqual([]); + }); +}); diff --git a/apps/ade-cli/src/services/diagnostics/autoDiagnosticsSender.ts b/apps/ade-cli/src/services/diagnostics/autoDiagnosticsSender.ts new file mode 100644 index 000000000..9f1fa9242 --- /dev/null +++ b/apps/ade-cli/src/services/diagnostics/autoDiagnosticsSender.ts @@ -0,0 +1,151 @@ +import path from "node:path"; +import { + runAutoDiagnosticsSend, + type AutoDiagnosticsLogger, + type AutoDiagnosticsOutcome, + type AutoDiagnosticsUploadResult, +} from "../../../../desktop/src/main/services/diagnostics/autoDiagnosticsSend"; +import { resolveAutoDiagnosticsStateFile } from "../../../../desktop/src/main/services/diagnostics/autoDiagnosticsStore"; +import type { ProductAnalyticsCapture } from "../../../../desktop/src/shared/types/productAnalytics"; +import { + buildCliDiagnosticReport, + sendDiagnosticReport, + type ReportIssueResult, +} from "../../commands/reportIssue"; +import { resolveMachineAdeLayout } from "../projects/machineLayout"; +import { diagnosticReportFilePath, writeDiagnosticReportFile } from "./diagnosticReport"; + +/** + * The brain's half of automatic diagnostics. + * + * Some of the failures worth a report are ones the desktop never sees: a + * headless machine whose pairing recovery gave up, an account publisher that + * has been failing for minutes with nobody logged in at the console. The brain + * also has something the renderer does not — the machine's own credential + * store — so its reports carry an account token and land attributed. + * + * It shares the desktop's consent flag, its budget file and its send policy + * (`runAutoDiagnosticsSend`), so "three a day" really is three a day for the + * computer rather than three per process, and the guarantees are not restated + * here to drift. What it cannot do is show a toast: there may be no window at + * all. Successful sends are left pending in the shared ledger, and the desktop + * shows them in the same toast the next time a renderer subscribes — the + * renderer's acknowledgement is what finally retires the entry. + * + * The desktop and the brain can both report the same incident — a brain that + * cannot publish is also a brain the recovery screen may diagnose. They carry + * different failure codes and different surfaces, so both are individually + * useful, and the per-install ceiling of three a day bounds the duplication. + * Nothing coordinates them beyond that, deliberately. + */ + +export type BrainAutoDiagnosticsRequest = { + failureCode: string; + surface: string; + headline?: string | null; +}; + +export type BrainAutoDiagnosticsOutcome = AutoDiagnosticsOutcome; + +/** Everything the send needs out of a built CLI report, and nothing more. */ +export type BrainDiagnosticReport = Pick< + ReportIssueResult, + "report" | "installId" | "appVersion" | "secretsDir" +>; + +/** + * The two seams, written STRUCTURALLY rather than as `typeof + * buildCliDiagnosticReport` / `typeof sendDiagnosticReport`. + * + * The real functions are assignable to both, and a test can hand in an ordinary + * stub — the previous `as never` casts silently disabled every check on the + * shape these are called with, which is the one thing a seam exists to keep. + */ +export type BrainDiagnosticsBuild = (options: { + surface: string; + code: string; + headline: string | null; + cliVersion: string | null; + env: NodeJS.ProcessEnv; + now: () => Date; +}) => BrainDiagnosticReport; + +export type BrainDiagnosticsSend = ( + built: BrainDiagnosticReport, + deps: { env: NodeJS.ProcessEnv; auto: boolean; failureCode: string }, +) => Promise; + +export type BrainAutoDiagnosticsDeps = { + cliVersion?: string | null; + env?: NodeJS.ProcessEnv; + logger?: AutoDiagnosticsLogger; + capture?: (input: ProductAnalyticsCapture) => void; + /** Test seams. */ + stateFilePath?: string; + reportsDir?: string; + build?: BrainDiagnosticsBuild; + send?: BrainDiagnosticsSend; + writeReportFile?: (filePath: string, report: string) => boolean; + now?: () => number; +}; + +export type BrainAutoDiagnostics = { + report: (request: BrainAutoDiagnosticsRequest) => Promise; +}; + +export function createBrainAutoDiagnostics( + deps: BrainAutoDiagnosticsDeps = {}, +): BrainAutoDiagnostics { + const env = deps.env ?? process.env; + const now = deps.now ?? Date.now; + const layout = resolveMachineAdeLayout(env); + const stateFilePath = deps.stateFilePath ?? resolveAutoDiagnosticsStateFile(layout.adeDir, env); + const reportsDir = deps.reportsDir ?? path.join(layout.adeDir, "diagnostic-reports"); + const build = deps.build ?? buildCliDiagnosticReport; + const send = deps.send ?? sendDiagnosticReport; + const writeReportFile = deps.writeReportFile ?? writeDiagnosticReportFile; + let inFlight = false; + + const report = async ( + request: BrainAutoDiagnosticsRequest, + ): Promise => { + if (inFlight) return "skipped_ineligible"; + inFlight = true; + // One timestamp for the report's own header and for the file it is saved + // under, so the two never disagree by a tick. + const at = new Date(now()); + try { + return await runAutoDiagnosticsSend({ + stateFilePath, + source: "brain", + // The brain's own surface, as every other headless emitter reports it. + // Attributing a send nobody was at the keyboard for to `desktop` would + // read as a person's app doing it. + analyticsSurface: "api", + failureCode: request.failureCode, + surface: request.surface, + build: (failureCode) => + build({ + surface: request.surface, + code: failureCode, + headline: request.headline ?? null, + cliVersion: deps.cliVersion ?? null, + env, + now: () => at, + }), + reportPathOf: (built) => { + const filePath = diagnosticReportFilePath(reportsDir, request.surface, at); + return writeReportFile(filePath, built.report) ? filePath : null; + }, + send: (built, failureCode) => send(built, { env, auto: true, failureCode }), + logger: deps.logger, + capture: deps.capture, + now, + }); + } finally { + inFlight = false; + } + }; + + return { report }; +} diff --git a/apps/desktop/src/main/main.ts b/apps/desktop/src/main/main.ts index 50595330f..5d73ba8c8 100644 --- a/apps/desktop/src/main/main.ts +++ b/apps/desktop/src/main/main.ts @@ -219,6 +219,7 @@ import { normalizeProjectRootPath } from "../../../ade-cli/src/services/projects import { ACCOUNT_SESSION_CREDENTIAL_KEY, getSignedInAccountAccessToken, + type AccountAuthService, } from "../../../ade-cli/src/services/account/accountAuthService"; import { createPushRelayClient } from "../../../ade-cli/src/services/push/pushRelayClient"; import { createPushRegistrationStore } from "../../../ade-cli/src/services/push/pushRegistrationStore"; @@ -304,6 +305,9 @@ import { getPowerStateService } from "./services/power/powerStateService"; import { createMachinePowerBrainBridge } from "./services/power/machinePowerBrainBridge"; import { runUpdateTransaction } from "./services/updates/updateTransaction"; import { createProjectRecoveryService } from "./services/runtime/projectRecoveryService"; +import { createAutoDiagnosticsService } from "./services/diagnostics/autoDiagnosticsService"; +import { resolveAutoDiagnosticsStateFile } from "./services/diagnostics/autoDiagnosticsStore"; +import { collectDiagnosticReport } from "./services/diagnostics/diagnosticReportService"; import { createAgentToolsCacheService } from "./services/tools/agentToolsCacheService"; import { DEFAULT_RELEASE_REPOSITORY } from "./services/updates/autoUpdateVersions"; import { cleanupStaleTempArtifacts } from "./services/runtime/tempCleanupService"; @@ -1642,6 +1646,26 @@ app.whenReady().then(async () => { const mobileSyncHandoffLeaseTimers = new Map>(); const mobileSyncPreparationPromises = new Map>(); const localRuntimeLogger = createFileLogger(path.join(app.getPath("userData"), "local-runtime.jsonl")); + /** + * The signed-in account owner, once there is an auth service to ask. + * + * Late-bound because the account services are built far below, while the + * diagnostics report builder that hashes this id is built up here next to its + * triggers. Null before then simply means an unattributed report. + * + * It has to stay late-bound: `getSharedAccountAuthService` caches ONE service + * per secrets directory and the FIRST caller's options are the ones that + * survive, so calling it here — before `accountBridge` and `runtimeBridge` + * pass theirs — would silently pin the whole app to a default-configured + * account service. The reference is filled in beside the call that legitimately + * constructs it; the reader itself is a const so no later assignment can + * quietly repoint it somewhere else. + */ + let accountAuthServiceForOwnerId: AccountAuthService | null = null; + const readAccountOwnerId = (): string | null => { + const status = accountAuthServiceForOwnerId?.getStatus(); + return status?.signedIn ? status.userId?.trim() || null : null; + }; const productAnalyticsStateFile = defaultProductAnalyticsStateFile(machineAdeLayout.adeDir); const productAnalyticsService = getSharedProductAnalyticsService(productAnalyticsStateFile, () => createProductAnalyticsService({ @@ -2543,6 +2567,63 @@ app.whenReady().then(async () => { }); } + /** + * Automatic diagnostics: the report nobody was going to press the button for. + * + * Built here, ahead of the recovery service and the post-update transaction, + * because those two are triggers. It owns the Settings toggle, the per-install + * daily budget (shared on disk with the brain) and the send; every trigger is + * one call that cannot throw. + */ + const autoDiagnosticsService = createAutoDiagnosticsService({ + stateFilePath: resolveAutoDiagnosticsStateFile(machineAdeLayout.adeDir), + appVersion: app.getVersion(), + logger: localRuntimeLogger, + capture: (input) => productAnalyticsService.capture(input), + buildReport: async (request) => { + const projectRoot = request.projectRoot?.trim() || null; + const result = await collectDiagnosticReport( + { + appVersion: app.getVersion(), + packageChannel: normalizeAppPackageChannel(process.env.ADE_PACKAGE_CHANNEL), + isPackaged: app.isPackaged, + userDataPath: app.getPath("userData"), + reportsDir: path.join(app.getPath("userData"), "diagnostic-reports"), + installId: productAnalyticsService.getDistinctId(), + accountUserId: readAccountOwnerId(), + projectLogsDir: projectRoot ? resolveAdeLayout(projectRoot).logsDir : null, + getLocalRuntimeStatus: () => localRuntimePool.getStatus(), + // Deliberately no `diagnoseProject`. The recovery diagnosis is itself + // one of the triggers, so asking for a fresh one while building the + // report about it would re-enter the code path that asked for it. + // The diagnosis that fired this is already in the report's context. + }, + { + surface: request.surface, + headline: request.headline ?? null, + code: request.failureCode, + technicalDetail: request.technicalDetail ?? null, + projectRoot, + }, + ); + return { report: result.report, filePath: result.filePath, installId: result.installId }; + }, + // Fast path only. `webContents.send` does not throw when the renderer has + // crashed or has not mounted its toast host, so nothing here can tell that + // the user was actually shown anything — which is why the send stays marked + // pending regardless and only a renderer's acknowledgement retires it. A + // window that gets both keys the toast on the same id and sees one. + onSent: (notice) => { + for (const win of BrowserWindow.getAllWindows()) { + try { + win.webContents.send(IPC.diagnosticsAutoSent, notice); + } catch { + // A window tearing down simply does not get this toast. + } + } + }, + }); + // The one recovery service for this machine. It owns `restartServiceAndWait`, // the verified restart sequence behind the Repair button, and the post-update // transaction below reuses it rather than growing a second restart path. @@ -2551,6 +2632,21 @@ app.whenReady().then(async () => { adeHome: machineAdeLayout.adeDir, logger: localRuntimeLogger, connectionPool: localRuntimePool, + // One call at the point the diagnosis is final. The service's own budget + // makes a re-diagnosed screen cost nothing. + onTerminalDiagnosis: ({ code, projectRoot }) => { + // `report` is documented never to reject, and the same belt-and-braces + // catch the update transaction below carries applies here: a rejection + // from a path that is meant to be silent must not become an unhandled + // rejection in the main process. + void autoDiagnosticsService + .report({ + failureCode: code, + surface: "project_recovery", + projectRoot, + }) + .catch(() => undefined); + }, }); const shouldRefreshRuntimeServiceAfterUpdate = @@ -2616,12 +2712,23 @@ app.whenReady().then(async () => { }); return; } + const failedStep = result.steps.find((step) => step.status === "failed")?.id ?? null; updateLogger.error("autoUpdate.transaction_failed", { version: result.version, - failedStep: result.steps.find((step) => step.status === "failed")?.id ?? null, + failedStep, failureMessage: result.failureMessage, steps: result.steps, }); + // An update that half-landed is the failure people least often report + // and the one hardest to reconstruct afterwards. + if (failedStep) { + void autoDiagnosticsService + .report({ + failureCode: `update_${failedStep}`, + surface: "update_transaction", + }) + .catch(() => undefined); + } }) .catch((error) => { // runUpdateTransaction never rejects; this only guards a broken @@ -7251,6 +7358,7 @@ app.whenReady().then(async () => { const shouldForwardAttentionNotchToast = createAttentionNotchToastDeduper(); let attentionIpcBridge: ReturnType | null = null; const attentionAccountAuthService = getSharedAccountAuthService(); + accountAuthServiceForOwnerId = attentionAccountAuthService; const attentionRelayClient = createPushRelayClient({ store: createPushRegistrationStore({ filePath: resolvePushRelayStateFile(machineAdeLayout.secretsDir), @@ -7642,6 +7750,7 @@ app.whenReady().then(async () => { : localRuntimePool, projectRecoveryConnectionPool: localRuntimePool, injectedProjectRecoveryService: machineRecoveryService, + autoDiagnosticsService, createWindow: openAdeWindow, closeWindow: closeAdeWindow, switchProjectFromDialog, @@ -7685,10 +7794,7 @@ app.whenReady().then(async () => { ); }, accountAttentionClient: attentionRelayClient, - getCurrentAccountOwnerId: () => { - const status = attentionAccountAuthService.getStatus(); - return status.signedIn ? status.userId?.trim() || null : null; - }, + getCurrentAccountOwnerId: () => readAccountOwnerId(), }); // Explicit project launches still bind a project before the renderer boots; diff --git a/apps/desktop/src/main/services/analytics/productAnalyticsPolicy.ts b/apps/desktop/src/main/services/analytics/productAnalyticsPolicy.ts index c57dc30d5..4a52b3bfe 100644 --- a/apps/desktop/src/main/services/analytics/productAnalyticsPolicy.ts +++ b/apps/desktop/src/main/services/analytics/productAnalyticsPolicy.ts @@ -145,6 +145,11 @@ const ANALYTICS_ONLY_ACTIONS = new Set([ // and the one that produced no telemetry at all last time. "machine_removed", "machine_register_refused", + // One coarse fact per automatic diagnostic send: whether it went, was refused + // by the client budget, or failed. Never the failure code that triggered it, + // never the surface, never the report or its upload reference — those are the + // local file and the upload the user was toasted about, not analytics. + "auto_sent", ]); const EVENT_PROPERTY_KEYS: Record> = { @@ -274,6 +279,10 @@ const SAFE_STRING_VALUES: Partial>> = { // `AdeUsageScope` and nothing else: a fourth spelling is dropped, not // widened, so the scope control can never carry free text. "machine", "project", "account", + // An automatic diagnostic report the client budget refused. Distinct from + // `failed` on purpose: "we chose not to send" and "we tried and could not" + // answer different questions, and the first is the guardrail working. + "skipped_budget", ]), provider: new Set(["codex", "openai", "claude", "cursor", "droid", "opencode", "pi", "gemini", "lmstudio", "local", "other"]), model_family: new Set([ diff --git a/apps/desktop/src/main/services/analytics/productAnalyticsService.test.ts b/apps/desktop/src/main/services/analytics/productAnalyticsService.test.ts index 6dca43c13..36a303461 100644 --- a/apps/desktop/src/main/services/analytics/productAnalyticsService.test.ts +++ b/apps/desktop/src/main/services/analytics/productAnalyticsService.test.ts @@ -1498,6 +1498,31 @@ describe("product analytics producers", () => { })).not.toHaveProperty("outcome"); }); + it("keeps only the three coarse outcomes of an automatic diagnostic send", () => { + // Auto-send answers one question: does the thing that fires without anyone + // asking actually go, get held back by its own budget, or fail. The failure + // code that triggered it, the surface, the upload reference and the saved + // report path are the local artifact and the toast — never the event. + for (const outcome of ["completed", "skipped_budget", "failed"]) { + expect(sanitizeProductAnalyticsProperties("ade_feature_used", { + feature: "connections", + action: "auto_sent", + outcome, + })).toEqual({ feature: "connections", action: "auto_sent", outcome }); + } + + const leaky = sanitizeProductAnalyticsProperties("ade_feature_used", { + feature: "connections", + action: "auto_sent", + outcome: "completed", + code: "brain_crash_looping", + surface: "project_recovery", + reference: "abcd1234", + report_path: "/Users/ada/Library/Application Support/ADE/diagnostic-reports/x.md", + }); + expect(leaky).toEqual({ feature: "connections", action: "auto_sent", outcome: "completed" }); + }); + it("maps automation completion and failed chat turns into canonical bounded outcomes", () => { const captures: ProductAnalyticsCapture[] = []; const analytics = settledAnalytics(captures); diff --git a/apps/desktop/src/main/services/diagnostics/autoDiagnosticsSend.ts b/apps/desktop/src/main/services/diagnostics/autoDiagnosticsSend.ts new file mode 100644 index 000000000..d7093f15b --- /dev/null +++ b/apps/desktop/src/main/services/diagnostics/autoDiagnosticsSend.ts @@ -0,0 +1,229 @@ +import type { + ProductAnalyticsCapture, + ProductAnalyticsSurface, +} from "../../../shared/types/productAnalytics"; +import { + claimAutoDiagnosticsSend, + completeAutoDiagnosticsSend, + isAutoDiagnosticsEnabled, + normalizeAutoDiagnosticsFailureCode, + type AutoDiagnosticsSource, +} from "./autoDiagnosticsStore"; + +/** + * The policy every automatic diagnostic send obeys, written once. + * + * There are two senders — the desktop main process and the brain — and they + * differ in exactly three ways: what they build, how they upload it, and which + * analytics surface they report as. Everything else (consent, the reservation, + * the local copy, silence on failure, the pending flag, the log lines, the + * dedupe key) is a promise made to the user, and a promise kept in two places + * is a promise that will eventually be kept in one. So the senders bring the + * three differences and this brings the policy. + * + * The seams are STRUCTURAL — `build` and `send` are typed over what they + * actually return, not over one sender's concrete functions — so a test can + * hand in an ordinary object instead of casting a fake through `as never`. + */ + +/** One hour, so a machine failing repeatedly does not narrate every attempt. */ +export const AUTO_DIAGNOSTICS_ANALYTICS_DEDUPE_MS = 60 * 60 * 1_000; + +export type AutoDiagnosticsOutcome = + /** Sent, and the ledger has a pending notice for it. */ + | "completed" + /** Consent is withdrawn. Nothing was built, sent, or counted. */ + | "skipped_disabled" + /** + * The reservation was refused: the code's daily slot or the install's three + * are gone, or the ledger could not be read or locked and so fails closed. + */ + | "skipped_budget" + /** + * The request never became a candidate: an unusable failure code, or another + * send already in flight. Distinct from `skipped_budget` because nothing was + * spent and nothing was refused — and, like today, it emits no analytics. + */ + | "skipped_ineligible" + /** Built or uploaded and did not work. Silent to the user by design. */ + | "failed"; + +export type AutoDiagnosticsLogger = { + info?: (event: string, meta?: Record) => void; + warn?: (event: string, meta?: Record) => void; +}; + +export type AutoDiagnosticsSentNotice = { + failureCode: string; + /** Empty when the local copy could not be written; the toast hides "View". */ + reportPath: string; + reference: string; +}; + +/** + * Structural shape of an upload answer. Both senders' concrete result types are + * assignable to it, and neither has to be imported here. + */ +export type AutoDiagnosticsUploadResult = + | { ok: true; reference: string } + | { ok: false; reason: string }; + +export type AutoDiagnosticsSendArgs = { + /** `/secrets/diagnostics-autosend.json`. */ + stateFilePath: string; + source: AutoDiagnosticsSource; + /** `desktop` for the app, `api` for the brain — a headless send is not a person. */ + analyticsSurface: ProductAnalyticsSurface; + /** Raw code from the trigger; normalized here. */ + failureCode: string; + /** Which screen or subsystem produced it; matches the manual surfaces. */ + surface: string; + build: (failureCode: string) => TBuilt | Promise; + /** + * Saves the local copy and answers with its path, or `null` when it could not + * be written. Called before the upload on purpose: the local copy is the half + * of this the user can always still get to, so it must not depend on the + * network half working. + */ + reportPathOf: (built: TBuilt) => string | null; + send: (built: TBuilt, failureCode: string) => Promise; + /** + * Fast path to a toast for a send that happened while a window was up. It is + * an optimization and NOT the record of delivery — see `pending` below. + */ + onSent?: (notice: AutoDiagnosticsSentNotice) => void; + logger?: AutoDiagnosticsLogger; + capture?: (input: ProductAnalyticsCapture) => void; + now?: () => number; +}; + +/** + * Runs one automatic send. Never throws and never rejects. + * + * PENDING IS ALWAYS TRUE ON SUCCESS, for both senders, and that is the single + * definition of the flag. It used to derive from whether `onSent` claimed the + * toast was delivered, which cannot be known: `webContents.send` does not throw + * when the receiving renderer has crashed or has not mounted its toast host, so + * "a window existed" was being recorded as "the user was told". Nothing on this + * side can retire it, therefore: pending means "no renderer has said it showed + * this", and it is cleared only when one does say so + * (`IPC.diagnosticsAckAutoSent`, sent after the toast is rendered). Listing the + * pending notices on subscribe (`IPC.diagnosticsFlushAutoSent`) deliberately + * clears nothing. + * + * The fast-path `onSent` above may deliver the same notice ahead of that list — + * safe twice over: the renderer keys the toast on + * `diagnostics-auto-sent-${reference}` so a repeat replaces it in place, and + * the ack is idempotent. The record is written BEFORE `onSent` fires so an ack + * can never arrive ahead of the entry it names. + */ +export async function runAutoDiagnosticsSend( + args: AutoDiagnosticsSendArgs, +): Promise { + const now = args.now ?? Date.now; + const failureCode = normalizeAutoDiagnosticsFailureCode(args.failureCode); + if (!failureCode) return "skipped_ineligible"; + + const captureOutcome = (outcome: "completed" | "skipped_budget" | "failed"): void => { + try { + args.capture?.({ + event: "ade_feature_used", + surface: args.analyticsSurface, + properties: { feature: "connections", action: "auto_sent", outcome }, + projectId: null, + dedupeKey: `diagnostics_auto_sent:${outcome}`, + minimumIntervalMs: AUTO_DIAGNOSTICS_ANALYTICS_DEDUPE_MS, + }); + } catch { + // Analytics must never change what the diagnostics path does. + } + }; + + if (!isAutoDiagnosticsEnabled(args.stateFilePath)) return "skipped_disabled"; + const claim = claimAutoDiagnosticsSend({ + filePath: args.stateFilePath, + failureCode, + source: args.source, + now, + }); + if (!claim.allowed) { + args.logger?.info?.("diagnostics.auto_send_skipped", { + failureCode, + surface: args.surface, + reason: claim.reason, + }); + // Consent withdrawn is not a budget event and is not worth an event of its + // own either: nothing goes when the user has said no, including telemetry. + if (claim.reason !== "disabled") captureOutcome("skipped_budget"); + return claim.reason === "disabled" ? "skipped_disabled" : "skipped_budget"; + } + + let built: TBuilt; + try { + built = await args.build(failureCode); + } catch (error) { + args.logger?.warn?.("diagnostics.auto_send_build_failed", { + failureCode, + error: error instanceof Error ? error.message : String(error), + }); + // The reservation is deliberately NOT given back. What the budget bounds is + // how often this computer tries on its own, and a collector that wedges + // every time would otherwise retry forever. + captureOutcome("failed"); + return "failed"; + } + + let reportPath: string | null; + try { + reportPath = args.reportPathOf(built); + } catch { + reportPath = null; + } + + let result: AutoDiagnosticsUploadResult; + try { + result = await args.send(built, failureCode); + } catch { + result = { ok: false, reason: "network" }; + } + + // Recorded before the toast, so the durable half never depends on the + // cosmetic half. + completeAutoDiagnosticsSend({ + filePath: args.stateFilePath, + failureCode, + atMs: claim.atMs, + reportPath, + reference: result.ok ? result.reference : null, + pending: result.ok, + now, + }); + + if (!result.ok) { + // Every failure is the same failure here, including a 429 from either the + // per-user or the fleet budget: the send does not happen, the log records + // why, and nothing reaches the screen. There is no retry — the reservation + // is already spent, which is what keeps a server saying no from turning + // into a machine asking repeatedly. + args.logger?.warn?.("diagnostics.auto_send_failed", { + failureCode, + surface: args.surface, + reason: result.reason, + }); + captureOutcome("failed"); + return "failed"; + } + + try { + args.onSent?.({ failureCode, reportPath: reportPath ?? "", reference: result.reference }); + } catch { + // A broken listener must not turn a successful send into a failure. + } + args.logger?.info?.("diagnostics.auto_sent", { + failureCode, + surface: args.surface, + reference: result.reference, + }); + captureOutcome("completed"); + return "completed"; +} diff --git a/apps/desktop/src/main/services/diagnostics/autoDiagnosticsService.test.ts b/apps/desktop/src/main/services/diagnostics/autoDiagnosticsService.test.ts new file mode 100644 index 000000000..b5dda0a82 --- /dev/null +++ b/apps/desktop/src/main/services/diagnostics/autoDiagnosticsService.test.ts @@ -0,0 +1,260 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { ProductAnalyticsCapture } from "../../../shared/types/productAnalytics"; +import { + createAutoDiagnosticsService, + type AutoDiagnosticsServiceDeps, +} from "./autoDiagnosticsService"; +import { + resolveAutoDiagnosticsStateFile, + setAutoDiagnosticsEnabled, +} from "./autoDiagnosticsStore"; + +const dirs: string[] = []; +const T0 = Date.parse("2026-08-19T10:00:00.000Z"); + +function stateFile(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-auto-diagnostics-service-")); + dirs.push(dir); + return resolveAutoDiagnosticsStateFile(dir, {}); +} + +function harness(overrides: Partial = {}) { + const filePath = overrides.stateFilePath ?? stateFile(); + const upload = vi.fn(async (_request: { auto?: boolean; failureCode?: string | null }) => + ({ ok: true as const, id: "abcd1234-rest", reference: "abcd1234" })); + const onSent = vi.fn(() => true); + const capture = vi.fn<[ProductAnalyticsCapture], void>(); + const writeReportFile = vi.fn(() => true); + const service = createAutoDiagnosticsService({ + stateFilePath: filePath, + appVersion: "1.2.3", + env: {}, + now: () => T0, + upload, + writeReportFile, + onSent, + capture, + buildReport: async (request) => ({ + report: `# report for ${request.failureCode}`, + filePath: "/tmp/reports/report.md", + installId: "install-1", + }), + ...overrides, + }); + return { service, filePath, upload, onSent, capture, writeReportFile }; +} + +afterEach(() => { + vi.restoreAllMocks(); + for (const dir of dirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); +}); + +describe("createAutoDiagnosticsService", () => { + it("sends the report, saves the local copy, and says so exactly once", async () => { + const { service, upload, onSent, writeReportFile, capture } = harness(); + + await expect(service.report({ failureCode: "disk_full", surface: "project_recovery" })) + .resolves.toBe("completed"); + + expect(writeReportFile).toHaveBeenCalledWith("/tmp/reports/report.md", "# report for disk_full"); + expect(upload).toHaveBeenCalledTimes(1); + // The server needs both to keep automatic reports separable from filed ones. + expect(upload.mock.calls[0]?.[0]).toMatchObject({ + auto: true, + failureCode: "disk_full", + report: "# report for disk_full", + installId: "install-1", + appVersion: "1.2.3", + }); + expect(onSent).toHaveBeenCalledWith({ + failureCode: "disk_full", + reportPath: "/tmp/reports/report.md", + reference: "abcd1234", + }); + expect(capture.mock.calls[0]?.[0]).toMatchObject({ + event: "ade_feature_used", + properties: { feature: "connections", action: "auto_sent", outcome: "completed" }, + }); + }); + + it("does nothing at all while the setting is off", async () => { + const filePath = stateFile(); + setAutoDiagnosticsEnabled(filePath, false, { now: () => T0 }); + const { service, upload, onSent, capture } = harness({ stateFilePath: filePath }); + + await expect(service.report({ failureCode: "disk_full", surface: "project_recovery" })) + .resolves.toBe("skipped_disabled"); + + expect(upload).not.toHaveBeenCalled(); + expect(onSent).not.toHaveBeenCalled(); + // Not even a "we chose not to" event: consent is withdrawn, so nothing goes. + expect(capture).not.toHaveBeenCalled(); + }); + + it("stops at the budget instead of reporting the same failure twice", async () => { + const { service, upload, capture } = harness(); + + await expect(service.report({ failureCode: "disk_full", surface: "project_recovery" })) + .resolves.toBe("completed"); + await expect(service.report({ failureCode: "disk_full", surface: "project_recovery" })) + .resolves.toBe("skipped_budget"); + + expect(upload).toHaveBeenCalledTimes(1); + expect(capture.mock.calls.at(-1)?.[0]).toMatchObject({ + properties: { feature: "connections", action: "auto_sent", outcome: "skipped_budget" }, + }); + }); + + it("treats a rate-limited server as a silent skip, with no toast and no retry", async () => { + const upload = vi.fn(async () => ({ ok: false as const, reason: "rate_limited" as const })); + const { service, onSent, capture } = harness({ + upload, + }); + + await expect(service.report({ failureCode: "disk_full", surface: "project_recovery" })) + .resolves.toBe("failed"); + + expect(onSent).not.toHaveBeenCalled(); + expect(capture.mock.calls.at(-1)?.[0]).toMatchObject({ + properties: { outcome: "failed" }, + }); + + // The refusal already spent the reservation: a machine ADE said no to does + // not get to ask again about the same failure today. + await expect(service.report({ failureCode: "disk_full", surface: "project_recovery" })) + .resolves.toBe("skipped_budget"); + expect(upload).toHaveBeenCalledTimes(1); + }); + + it("stays silent when the network throws rather than answering", async () => { + const upload = vi.fn(async () => { + throw new Error("socket hang up"); + }); + const { service, onSent } = harness({ + upload, + }); + + await expect(service.report({ failureCode: "disk_full", surface: "project_recovery" })) + .resolves.toBe("failed"); + expect(onSent).not.toHaveBeenCalled(); + }); + + it("does not attempt an upload when the report cannot be built", async () => { + const upload = vi.fn(); + const { service } = harness({ + upload, + buildReport: async () => { + throw new Error("collector wedged"); + }, + }); + + await expect(service.report({ failureCode: "disk_full", surface: "project_recovery" })) + .resolves.toBe("failed"); + expect(upload).not.toHaveBeenCalled(); + // The reservation is NOT given back. What the budget bounds is how often + // this computer tries on its own, and a collector that wedges every time + // would otherwise retry the same failure forever. + expect(service.getStatus().sendsInWindow).toBe(1); + await expect(service.report({ failureCode: "disk_full", surface: "project_recovery" })) + .resolves.toBe("skipped_budget"); + }); + + it("holds a send nobody was listening for until a window says it showed it", async () => { + const { service, filePath } = harness({ onSent: undefined }); + + await expect(service.report({ failureCode: "disk_full", surface: "project_recovery" })) + .resolves.toBe("completed"); + + const notice = { + failureCode: "disk_full", + reportPath: "/tmp/reports/report.md", + reference: "abcd1234", + }; + expect(service.listPendingNotices()).toEqual([notice]); + // No renderer acknowledged it, so it is still on offer — being handed to a + // window that then dies is not the same as being seen. + expect(service.listPendingNotices()).toEqual([notice]); + + service.ackNotices(["abcd1234"]); + expect(service.listPendingNotices()).toEqual([]); + expect(fs.existsSync(filePath)).toBe(true); + }); + + it("stops re-toasting a fast-path send across a restart once it is acknowledged", async () => { + // Regression, both halves. `webContents.send` does not throw when the + // renderer has crashed or has not mounted its toast host, so "a window + // existed" must not be recorded as "the user was told" — but leaving it + // pending forever meant the live toast came back at every launch. The + // renderer's ack is what closes it, and it survives the process. + const { service, filePath, onSent } = harness(); + + await expect(service.report({ failureCode: "disk_full", surface: "project_recovery" })) + .resolves.toBe("completed"); + expect(onSent).toHaveBeenCalledTimes(1); + // The window that got the fast path toasted it and said so. + service.ackNotices(["abcd1234"]); + + // Next launch: a brand new service over the same ledger. + const { service: afterRestart } = harness({ stateFilePath: filePath }); + expect(afterRestart.listPendingNotices()).toEqual([]); + }); + + it("records the send even when the toast listener throws", async () => { + const { service } = harness({ + onSent: () => { + throw new Error("renderer is gone"); + }, + }); + + await expect(service.report({ failureCode: "disk_full", surface: "project_recovery" })) + .resolves.toBe("completed"); + expect(service.listPendingNotices()).toHaveLength(1); + }); + + it("exposes the toggle the settings pane and the toast both write", async () => { + const { service } = harness(); + expect(service.isEnabled()).toBe(true); + expect(service.setEnabled(false)).toBe(false); + expect(service.getStatus()).toEqual({ enabled: false, sendsInWindow: 0, limit: 3 }); + }); + + it("drops a failure code the server would refuse without touching the budget", async () => { + const { service, upload, capture } = harness(); + // `skipped_ineligible`, not `skipped_budget`: nothing was refused and + // nothing was spent, and — as before — nothing is reported either. + await expect(service.report({ failureCode: " ", surface: "project_recovery" })) + .resolves.toBe("skipped_ineligible"); + expect(upload).not.toHaveBeenCalled(); + expect(capture).not.toHaveBeenCalled(); + expect(service.getStatus().sendsInWindow).toBe(0); + }); + + it("skips a second failure that fires while the first is still sending", async () => { + let release: () => void = () => undefined; + const gate = new Promise((resolve) => { + release = resolve; + }); + const { service, upload } = harness({ + buildReport: async (request) => { + await gate; + return { + report: `# report for ${request.failureCode}`, + filePath: "/tmp/reports/report.md", + installId: "install-1", + }; + }, + }); + + const first = service.report({ failureCode: "disk_full", surface: "project_recovery" }); + // Same reason as above: never sent, never counted, never announced. + await expect(service.report({ failureCode: "db_integrity", surface: "project_recovery" })) + .resolves.toBe("skipped_ineligible"); + release(); + await expect(first).resolves.toBe("completed"); + expect(upload).toHaveBeenCalledTimes(1); + expect(service.getStatus().sendsInWindow).toBe(1); + }); +}); diff --git a/apps/desktop/src/main/services/diagnostics/autoDiagnosticsService.ts b/apps/desktop/src/main/services/diagnostics/autoDiagnosticsService.ts new file mode 100644 index 000000000..16d03d072 --- /dev/null +++ b/apps/desktop/src/main/services/diagnostics/autoDiagnosticsService.ts @@ -0,0 +1,157 @@ +import type { ProductAnalyticsCapture } from "../../../shared/types/productAnalytics"; +import { + resolveDiagnosticsUploadBaseUrl, + uploadDiagnosticReport, + type DiagnosticUploadRequest, +} from "../../../shared/diagnosticsUpload"; +import { writeDiagnosticReportFile } from "./diagnosticReportService"; +import { + runAutoDiagnosticsSend, + type AutoDiagnosticsLogger, + type AutoDiagnosticsOutcome, + type AutoDiagnosticsSentNotice, + type AutoDiagnosticsUploadResult, +} from "./autoDiagnosticsSend"; +import { + ackAutoDiagnosticsNotices, + isAutoDiagnosticsEnabled, + listPendingAutoDiagnosticsNotices, + readAutoDiagnosticsState, + setAutoDiagnosticsEnabled, + type AutoDiagnosticsNotice, +} from "./autoDiagnosticsStore"; + +/** + * Sends the diagnostic report nobody was ever going to press the button for. + * + * "Report issue" only ever fires when somebody notices the button, decides the + * failure is worth reporting, and follows through — on a screen that already + * told them ADE is broken. The reports that would explain the worst failures + * are exactly the ones that never arrive. So when ADE hits a failure it already + * classified, it sends the same already-redacted report by itself. + * + * The guardrails are the feature, not decoration around it: + * - a Settings toggle, default on, honoured by this process and by the brain; + * - a hard client budget (one per failure code, three total, per day, per + * install) reserved before the request so nothing here can become a loop; + * - a toast on every send, so it is never something that happened silently; + * - and total silence on failure. A user staring at a broken app must not + * also be told that the thing they did not ask for did not work. + * + * All of which is `runAutoDiagnosticsSend`, shared with the brain's sender. + * What lives here is only what is specific to the desktop: how a report gets + * built, that it uploads anonymously, and the toggle the settings pane reads. + */ + +export type AutoDiagnosticsRequest = { + /** Short machine code, e.g. `brain_crash_looping`. Never free text. */ + failureCode: string; + /** Which screen or subsystem produced it; matches the manual surfaces. */ + surface: string; + projectRoot?: string | null; + headline?: string | null; + technicalDetail?: string | null; +}; + +export type { AutoDiagnosticsOutcome, AutoDiagnosticsSentNotice }; + +export type AutoDiagnosticsReport = { + report: string; + filePath: string; + installId: string; +}; + +export type AutoDiagnosticsServiceDeps = { + /** `/secrets/diagnostics-autosend.json`. */ + stateFilePath: string; + buildReport: (request: AutoDiagnosticsRequest) => Promise; + appVersion: string | null; + /** + * Fires once per successful send so a window that is up can toast + * immediately. It is a fast path only: the send is recorded pending either + * way and the renderer's acknowledgement is what actually retires it. + */ + onSent?: (notice: AutoDiagnosticsSentNotice) => void; + capture?: (input: ProductAnalyticsCapture) => void; + logger?: AutoDiagnosticsLogger; + /** + * Test seams. `upload` is typed by what this service needs of an answer + * rather than as `typeof uploadDiagnosticReport`, so a test can pass a plain + * stub instead of casting one through `as unknown as`. + */ + env?: NodeJS.ProcessEnv; + upload?: (request: DiagnosticUploadRequest) => Promise; + writeReportFile?: (filePath: string, report: string) => boolean; + now?: () => number; +}; + +export type AutoDiagnosticsService = { + /** One call per failure point. Never throws and never rejects. */ + report: (request: AutoDiagnosticsRequest) => Promise; + isEnabled: () => boolean; + setEnabled: (enabled: boolean) => boolean; + getStatus: () => { enabled: boolean; sendsInWindow: number; limit: number }; + /** + * The sends no renderer has acknowledged showing yet — the brain's, and any + * this process made while no window was listening. Read when a renderer + * subscribes, so there is no timer behind it. Listing does NOT retire them: + * `ackNotices` does, once the toast is on screen. + */ + listPendingNotices: () => AutoDiagnosticsNotice[]; + /** Retires the notices a renderer has actually rendered. Idempotent. */ + ackNotices: (references: readonly string[]) => void; +}; + +export function createAutoDiagnosticsService( + deps: AutoDiagnosticsServiceDeps, +): AutoDiagnosticsService { + const now = deps.now ?? Date.now; + const upload = deps.upload ?? uploadDiagnosticReport; + const writeReportFile = deps.writeReportFile ?? writeDiagnosticReportFile; + const env = deps.env ?? process.env; + // One at a time. Two failures that fire together would otherwise race on the + // budget file and, worse, run two report collections at once on a machine + // that is already unwell. + let inFlight = false; + + const report = async (request: AutoDiagnosticsRequest): Promise => { + if (inFlight) return "skipped_ineligible"; + inFlight = true; + try { + return await runAutoDiagnosticsSend({ + stateFilePath: deps.stateFilePath, + source: "desktop", + analyticsSurface: "desktop", + failureCode: request.failureCode, + surface: request.surface, + build: (failureCode) => deps.buildReport({ ...request, failureCode }), + reportPathOf: (built) => (writeReportFile(built.filePath, built.report) ? built.filePath : null), + send: async (built, failureCode) => + upload({ + baseUrl: resolveDiagnosticsUploadBaseUrl(env.ADE_ACCOUNT_DIRECTORY_URL), + report: built.report, + installId: built.installId === "unknown" ? null : built.installId, + appVersion: deps.appVersion, + auto: true, + failureCode, + }), + onSent: deps.onSent, + logger: deps.logger, + capture: deps.capture, + now, + }); + } finally { + inFlight = false; + } + }; + + return { + report, + isEnabled: () => isAutoDiagnosticsEnabled(deps.stateFilePath), + setEnabled: (enabled) => setAutoDiagnosticsEnabled(deps.stateFilePath, enabled, { now }), + getStatus: () => readAutoDiagnosticsState(deps.stateFilePath, { now }), + listPendingNotices: () => listPendingAutoDiagnosticsNotices(deps.stateFilePath), + ackNotices: (references) => + ackAutoDiagnosticsNotices(deps.stateFilePath, references, { now }), + }; +} diff --git a/apps/desktop/src/main/services/diagnostics/autoDiagnosticsStore.test.ts b/apps/desktop/src/main/services/diagnostics/autoDiagnosticsStore.test.ts new file mode 100644 index 000000000..3ca777d53 --- /dev/null +++ b/apps/desktop/src/main/services/diagnostics/autoDiagnosticsStore.test.ts @@ -0,0 +1,280 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { + ackAutoDiagnosticsNotices, + AUTO_DIAGNOSTICS_WINDOW_MS, + claimAutoDiagnosticsSend, + completeAutoDiagnosticsSend, + isAutoDiagnosticsEnabled, + listPendingAutoDiagnosticsNotices, + normalizeAutoDiagnosticsFailureCode, + readAutoDiagnosticsState, + resolveAutoDiagnosticsStateFile, + setAutoDiagnosticsEnabled, +} from "./autoDiagnosticsStore"; + +const dirs: string[] = []; + +function stateFile(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-auto-diagnostics-")); + dirs.push(dir); + return resolveAutoDiagnosticsStateFile(dir, {}); +} + +afterEach(() => { + for (const dir of dirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); +}); + +const T0 = Date.parse("2026-08-19T10:00:00.000Z"); + +describe("auto diagnostics budget", () => { + it("defaults to on for a machine that has never auto-sent", () => { + const filePath = stateFile(); + expect(isAutoDiagnosticsEnabled(filePath)).toBe(true); + expect(readAutoDiagnosticsState(filePath, { now: () => T0 })).toEqual({ + enabled: true, + sendsInWindow: 0, + limit: 3, + }); + }); + + it("allows one send per failure code per day and three in total", () => { + const filePath = stateFile(); + const claim = (failureCode: string, atMs: number) => + claimAutoDiagnosticsSend({ filePath, failureCode, source: "desktop", now: () => atMs }); + + expect(claim("disk_full", T0).allowed).toBe(true); + // Same failure again, minutes later: the user is told about a problem once. + expect(claim("disk_full", T0 + 60_000)).toEqual({ allowed: false, reason: "code_limit" }); + expect(claim("db_integrity", T0 + 60_000).allowed).toBe(true); + expect(claim("renderer_crash", T0 + 120_000).allowed).toBe(true); + // Fourth distinct failure in the same day: the daily ceiling, not the code. + expect(claim("update_service", T0 + 180_000)).toEqual({ allowed: false, reason: "daily_limit" }); + }); + + it("keeps the budget across restarts and releases it when the window rolls", () => { + const filePath = stateFile(); + const claim = (failureCode: string, atMs: number) => + claimAutoDiagnosticsSend({ filePath, failureCode, source: "desktop", now: () => atMs }); + + expect(claim("disk_full", T0).allowed).toBe(true); + expect(claim("db_integrity", T0).allowed).toBe(true); + expect(claim("renderer_crash", T0).allowed).toBe(true); + + // A fresh process reads the same file: the ledger is the file, not memory. + expect(claim("update_health", T0 + 60_000)).toEqual({ allowed: false, reason: "daily_limit" }); + expect(readAutoDiagnosticsState(filePath, { now: () => T0 }).sendsInWindow).toBe(3); + + const nextDay = T0 + AUTO_DIAGNOSTICS_WINDOW_MS + 1; + expect(claim("disk_full", nextDay).allowed).toBe(true); + expect(readAutoDiagnosticsState(filePath, { now: () => nextDay }).sendsInWindow).toBe(1); + }); + + it("refuses every send while the setting is off, and resumes when it is back on", () => { + const filePath = stateFile(); + setAutoDiagnosticsEnabled(filePath, false, { now: () => T0 }); + expect(isAutoDiagnosticsEnabled(filePath)).toBe(false); + expect( + claimAutoDiagnosticsSend({ filePath, failureCode: "disk_full", source: "desktop", now: () => T0 }), + ).toEqual({ allowed: false, reason: "disabled" }); + + setAutoDiagnosticsEnabled(filePath, true, { now: () => T0 }); + expect( + claimAutoDiagnosticsSend({ filePath, failureCode: "disk_full", source: "desktop", now: () => T0 }).allowed, + ).toBe(true); + }); + + it("treats an unreadable ledger as spent rather than as untouched", () => { + const filePath = stateFile(); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, "{ this is not json", "utf8"); + // Forgiving a garbled counter is the same as not keeping one. + expect( + claimAutoDiagnosticsSend({ filePath, failureCode: "disk_full", source: "desktop", now: () => T0 }), + ).toEqual({ allowed: false, reason: "state_unavailable" }); + }); + + it("fails closed while another process holds the lock", () => { + const filePath = stateFile(); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.mkdirSync(`${filePath}.lock`); + // Held right now on the same clock the claim reads, so it is not stale. + fs.utimesSync(`${filePath}.lock`, T0 / 1_000, T0 / 1_000); + expect( + claimAutoDiagnosticsSend({ filePath, failureCode: "disk_full", source: "desktop", now: () => T0 }), + ).toEqual({ allowed: false, reason: "state_unavailable" }); + }); + + it("waits out a lock the holder releases instead of dropping the write", () => { + // The lock used to be one shot: a holder that released microseconds later + // still cost the caller its whole operation. Simulated by releasing on the + // second `now()` read, which is the retry loop's own tick. + const filePath = stateFile(); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + const lockPath = `${filePath}.lock`; + fs.mkdirSync(lockPath); + fs.utimesSync(lockPath, T0 / 1_000, T0 / 1_000); + let reads = 0; + const now = () => { + reads += 1; + if (reads === 1) fs.rmdirSync(lockPath); + return T0; + }; + + expect( + claimAutoDiagnosticsSend({ filePath, failureCode: "disk_full", source: "desktop", now }).allowed, + ).toBe(true); + }); + + it("lands a contended withdrawal of consent without eating the ledger", () => { + const filePath = stateFile(); + const claim = claimAutoDiagnosticsSend({ + filePath, + failureCode: "disk_full", + source: "desktop", + now: () => T0, + }); + expect(claim.allowed).toBe(true); + + // Another process is holding the lock and does not let go inside the wait. + const lockPath = `${filePath}.lock`; + fs.mkdirSync(lockPath); + fs.utimesSync(lockPath, T0 / 1_000, T0 / 1_000); + + // Consent is the one write that may not be dropped, so it still lands — + // and it must not take the spend ledger down with it. The old fallback + // wrote whatever it had read and could silently reset the day's count. + expect(setAutoDiagnosticsEnabled(filePath, false, { now: () => T0 })).toBe(false); + expect(readAutoDiagnosticsState(filePath, { now: () => T0 })).toEqual({ + enabled: false, + sendsInWindow: 1, + limit: 3, + }); + }); + + // Directory permissions are the lever here, and `chmod` is a no-op on + // Windows; the behaviour it pins is platform-independent. + it.skipIf(process.platform === "win32")("reports the state on disk when a toggle cannot be persisted at all", () => { + const filePath = stateFile(); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, JSON.stringify({ version: 1, enabled: true }), "utf8"); + // Lock held AND the file itself unwritable: nothing can land. A consent + // pane must not then render an "off" that is really on. + const lockPath = `${filePath}.lock`; + fs.mkdirSync(lockPath); + fs.utimesSync(lockPath, T0 / 1_000, T0 / 1_000); + fs.chmodSync(path.dirname(filePath), 0o500); + try { + expect(setAutoDiagnosticsEnabled(filePath, false, { now: () => T0 })).toBe(true); + } finally { + fs.chmodSync(path.dirname(filePath), 0o700); + } + }); + + function recordPendingSend( + filePath: string, + failureCode: string, + reference: string, + ): void { + const claim = claimAutoDiagnosticsSend({ + filePath, + failureCode, + source: "brain", + now: () => T0, + }); + expect(claim.allowed).toBe(true); + if (!claim.allowed) return; + completeAutoDiagnosticsSend({ + filePath, + failureCode, + atMs: claim.atMs, + reportPath: "/tmp/report.md", + reference, + pending: true, + now: () => T0, + }); + } + + it("refuses to mark a send pending when nothing can ever acknowledge it", () => { + // An ack names an upload reference. A pending entry without one could never + // be retired, so it would be replayed to every renderer on every launch + // forever — the one failure mode worse than a missed toast. + for (const reference of [null, "", " "]) { + const filePath = stateFile(); + const claim = claimAutoDiagnosticsSend({ + filePath, + failureCode: "snapshot_failed", + source: "brain", + now: () => T0, + }); + expect(claim.allowed).toBe(true); + if (!claim.allowed) return; + completeAutoDiagnosticsSend({ + filePath, + failureCode: "snapshot_failed", + atMs: claim.atMs, + reportPath: "/tmp/report.md", + reference, + pending: true, + now: () => T0, + }); + expect(listPendingAutoDiagnosticsNotices(filePath)).toEqual([]); + } + }); + + it("keeps offering a brain-side send until a renderer says it showed it", () => { + const filePath = stateFile(); + recordPendingSend(filePath, "snapshot_failed", "abcd1234"); + + const notice = { + failureCode: "snapshot_failed", + reportPath: "/tmp/report.md", + reference: "abcd1234", + }; + expect(listPendingAutoDiagnosticsNotices(filePath)).toEqual([notice]); + // Listing is not showing. Nothing has claimed to have put this on screen, + // so it is still on offer — including to a window that opens later. + expect(listPendingAutoDiagnosticsNotices(filePath)).toEqual([notice]); + + ackAutoDiagnosticsNotices(filePath, ["abcd1234"], { now: () => T0 }); + // Acknowledged means shown: the next launch must not toast it again. + expect(listPendingAutoDiagnosticsNotices(filePath)).toEqual([]); + }); + + it("takes an acknowledgement from either window and ignores one for nothing", () => { + const filePath = stateFile(); + recordPendingSend(filePath, "snapshot_failed", "abcd1234"); + recordPendingSend(filePath, "disk_full", "efgh5678"); + + // Two windows both got the fast-path notice and both toasted it; the second + // ack is a no-op rather than an error, and so is one for a send that never + // existed or was already retired. + ackAutoDiagnosticsNotices(filePath, ["abcd1234"], { now: () => T0 }); + ackAutoDiagnosticsNotices(filePath, ["abcd1234"], { now: () => T0 }); + ackAutoDiagnosticsNotices(filePath, ["nosuchref", ""], { now: () => T0 }); + + expect(listPendingAutoDiagnosticsNotices(filePath)).toEqual([ + { failureCode: "disk_full", reportPath: "/tmp/report.md", reference: "efgh5678" }, + ]); + ackAutoDiagnosticsNotices(filePath, ["efgh5678"], { now: () => T0 }); + expect(listPendingAutoDiagnosticsNotices(filePath)).toEqual([]); + }); +}); + +describe("normalizeAutoDiagnosticsFailureCode", () => { + it("passes the codes ADE actually produces", () => { + for (const code of ["disk_full", "brain_crash_looping", "snapshot_failed", "update_service"]) { + expect(normalizeAutoDiagnosticsFailureCode(code)).toBe(code); + } + }); + + it("coerces or rejects anything that would not survive the server", () => { + expect(normalizeAutoDiagnosticsFailureCode("Disk Full")).toBe("disk_full"); + expect(normalizeAutoDiagnosticsFailureCode(" ")).toBeNull(); + expect(normalizeAutoDiagnosticsFailureCode("123")).toBeNull(); + expect(normalizeAutoDiagnosticsFailureCode(null)).toBeNull(); + expect(normalizeAutoDiagnosticsFailureCode("a".repeat(80))).toBe("a".repeat(48)); + }); +}); diff --git a/apps/desktop/src/main/services/diagnostics/autoDiagnosticsStore.ts b/apps/desktop/src/main/services/diagnostics/autoDiagnosticsStore.ts new file mode 100644 index 000000000..9df690806 --- /dev/null +++ b/apps/desktop/src/main/services/diagnostics/autoDiagnosticsStore.ts @@ -0,0 +1,570 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { FAILURE_CODE_PATTERN } from "../../../shared/diagnosticsUpload"; +import { writeFileAtomic } from "../state/durableFile"; + +/** + * The consent flag and the spend ledger for automatic diagnostic uploads. + * + * ONE file for BOTH senders. The desktop main process and the brain + * (`apps/ade-cli`) each detect different failures and each can auto-send, but + * the budget the user is promised — "at most three a day from this computer" — + * is a property of the install, not of a process. Two private ledgers would + * quietly mean six. So this module is deliberately dependency-free (plain + * `node:fs`, no Electron, no logger) and is imported by both, exactly the way + * both already share `~/.ade/secrets/product-analytics.json`. + * + * The JSON shape and the atomic replace behave identically on Windows. The + * mkdir lock does NOT quite: see `isLockContention` below for the delete-pending + * window that makes Windows report contention as EPERM/EACCES/EBUSY instead of + * EEXIST, which this module handles explicitly rather than assuming away. + */ + +/** `/secrets/diagnostics-autosend.json`. */ +export function resolveAutoDiagnosticsStateFile( + adeHome?: string | null, + env: NodeJS.ProcessEnv = process.env, +): string { + const home = adeHome?.trim() || env.ADE_HOME?.trim() || path.join(os.homedir(), ".ade"); + return path.join(path.resolve(home), "secrets", "diagnostics-autosend.json"); +} + +/** Rolling window for both budgets. */ +export const AUTO_DIAGNOSTICS_WINDOW_MS = 24 * 60 * 60 * 1_000; +/** At most one automatic report per distinct failure code per window. */ +export const MAX_AUTO_DIAGNOSTICS_PER_CODE = 1; +/** At most this many automatic reports in total per window, per install. */ +export const MAX_AUTO_DIAGNOSTICS_PER_WINDOW = 3; + +/** + * Shape the server accepts for `failureCode`. Checked here so a caller that + * invents a code never spends a send on a request the Worker will refuse. + * + * Re-exported from the uploader rather than written out a second time: the + * uploader already has to know this shape to decide what it puts on the wire, + * and two copies in the same process is how they drift. + */ +export const AUTO_DIAGNOSTICS_FAILURE_CODE_PATTERN = FAILURE_CODE_PATTERN; + +/** + * Coerces a caller's code into the server's shape, or null when it cannot be. + * + * Failure codes come from typed unions (`AdeRecoveryErrorCode`, an update step + * id, a pairing refusal code) that already look like this; the normalization is + * for the ones assembled by hand at a call site. + */ +export function normalizeAutoDiagnosticsFailureCode(value: string | null | undefined): string | null { + const raw = value?.trim().toLowerCase(); + if (!raw) return null; + const cleaned = raw.replace(/[^a-z0-9_-]+/g, "_").replace(/^[^a-z]+/, "").slice(0, 48); + return AUTO_DIAGNOSTICS_FAILURE_CODE_PATTERN.test(cleaned) ? cleaned : null; +} + +export type AutoDiagnosticsSource = "desktop" | "brain"; + +/** One spent send. Codes and timestamps only — never a report or its text. */ +export type AutoDiagnosticsSend = { + code: string; + atMs: number; + source: AutoDiagnosticsSource; + /** Local path of the saved `.md`, so the toast's "View" can reveal it. */ + reportPath: string | null; + /** Short upload handle, present once the upload succeeded. */ + reference: string | null; + /** + * A successful send no renderer has acknowledged RENDERING yet. + * + * Set on EVERY successful send, including one made while a window was open: + * `webContents.send` does not throw when the receiving renderer has crashed + * or has not mounted its toast host, so nothing on the sending side can know + * the user was shown anything. What clears it is the renderer saying so, once + * the toast exists (`ackAutoDiagnosticsNotices`). A renderer that dies between + * rendering and acknowledging costs one repeated toast, which is the honest + * side of the trade: the alternative is a send the user never hears about. + */ + pending: boolean; +}; + +export type AutoDiagnosticsState = { + enabled: boolean; + sends: AutoDiagnosticsSend[]; +}; + +export type AutoDiagnosticsClaim = + | { allowed: true; atMs: number } + | { allowed: false; reason: "disabled" | "code_limit" | "daily_limit" | "state_unavailable" }; + +const LOCK_STALE_MS = 5_000; +/** Hard cap on retained entries so a long-lived install cannot grow the file. */ +const MAX_RETAINED_SENDS = 24; + +function readTimestamp(value: unknown): number | null { + if (typeof value !== "number" || !Number.isFinite(value) || value <= 0) return null; + return Math.floor(value); +} + +function readSend(value: unknown): AutoDiagnosticsSend | null { + if (!value || typeof value !== "object" || Array.isArray(value)) return null; + const record = value as Record; + const code = normalizeAutoDiagnosticsFailureCode( + typeof record.code === "string" ? record.code : null, + ); + const atMs = readTimestamp(record.atMs); + if (!code || atMs == null) return null; + return { + code, + atMs, + source: record.source === "brain" ? "brain" : "desktop", + reportPath: typeof record.reportPath === "string" && record.reportPath.trim() + ? record.reportPath + : null, + reference: typeof record.reference === "string" && record.reference.trim() + ? record.reference.trim() + : null, + pending: record.pending === true, + }; +} + +/** + * Reads the file, or reports that it could not be read. + * + * The distinction is the whole point. An ABSENT file is a machine that has + * never auto-sent: the setting is on and the budget is untouched. A file that + * exists but cannot be parsed is a machine whose spend history is unknown, and + * forgiving an unknown counter is the same as not keeping one — so callers + * treat it as spent. Only an explicit `enabled: false` turns the feature off, + * which is what makes the setting default ON. + */ +function readState(filePath: string): { state: AutoDiagnosticsState; readable: boolean } { + let text: string; + try { + text = fs.readFileSync(filePath, "utf8"); + } catch (error) { + const code = (error as NodeJS.ErrnoException | null)?.code; + // ENOENT is the untouched machine; anything else (EACCES, EISDIR, EIO) is a + // store we cannot account against. + return { state: { enabled: true, sends: [] }, readable: code === "ENOENT" }; + } + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + return { state: { enabled: true, sends: [] }, readable: false }; + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + return { state: { enabled: true, sends: [] }, readable: false }; + } + const record = parsed as Record; + const sends = Array.isArray(record.sends) + ? record.sends.map(readSend).filter((entry): entry is AutoDiagnosticsSend => entry != null) + : []; + return { + // Default ON: only an explicit `false` is a withdrawal of consent. + state: { enabled: record.enabled !== false, sends }, + readable: true, + }; +} + +function serialize(state: AutoDiagnosticsState): string { + return `${JSON.stringify( + { + version: 1, + enabled: state.enabled, + ...(state.sends.length ? { sends: state.sends } : {}), + }, + null, + 2, + )}\n`; +} + +function writeState(filePath: string, state: AutoDiagnosticsState): void { + fs.mkdirSync(path.dirname(filePath), { recursive: true, mode: 0o700 }); + writeFileAtomic(filePath, serialize(state), { fsync: true, mode: 0o600 }); +} + +/** + * Is this error "somebody else holds the lock", rather than a real fault? + * + * MIRRORS `isLockContention` in + * `apps/ade-cli/src/services/credentials/credentialFileIo.ts`, copied rather + * than imported to keep this module's one dependency rule — plain `node:fs`, + * nothing from either app's service tree, because both apps import it. Change + * one, change the other. + * + * POSIX reports a taken lock name as EEXIST. Windows does not, always: removing + * a directory entry there only frees the name once every handle to it closes, + * so between one holder's `rmdir` and the last handle drop the name sits in a + * "delete pending" state and a concurrent `mkdir` fails with EPERM, EACCES or + * EBUSY. Those are the same condition, and treating them as fatal would make + * every concurrent toggle on Windows a coin flip on whether consent persists. + */ +function isLockContention(error: unknown): boolean { + if (typeof error !== "object" || error === null || !("code" in error)) return false; + const code = (error as NodeJS.ErrnoException).code; + if (code === "EEXIST") return true; + if (process.platform !== "win32") return false; + return code === "EPERM" || code === "EACCES" || code === "EBUSY"; +} + +/** Bounded, synchronous wait. Same `Atomics.wait` idiom as the credential store. */ +function sleepSync(ms: number): void { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); +} + +const LOCK_RETRY_MS = 20; +const LOCK_RETRY_ATTEMPTS = 5; +/** + * The patient variant, for the one caller that must not give up early: ~1s of + * total wait, still bounded. See `setAutoDiagnosticsEnabled`. + */ +const LOCK_PATIENT_RETRY_ATTEMPTS = 50; + +/** + * mkdir-based mutual exclusion, same idiom as the identity/relay store. + * + * `mkdir` is atomic on every filesystem ADE runs on, and a crashed holder is + * reclaimed after `LOCK_STALE_MS` rather than wedging the feature forever. + * + * The default wait is deliberately tiny — five tries, 20ms apart, so at most + * ~100ms of blocking PER ACQUIRE. The critical section is one small file + * rewritten a handful of times a day, so a contended lock is nearly always the + * other process finishing its own write microseconds from now; a spin that + * short converts almost every collision into a success while still refusing to + * queue. Callers keep their fail-closed fallbacks for the case it does not: + * giving up on a claim is the correct outcome, and losing a consent flip is not + * (see `setAutoDiagnosticsEnabled`, which is why `attempts` exists). + */ +function acquireLock( + filePath: string, + now: () => number, + attempts: number = LOCK_RETRY_ATTEMPTS, +): (() => void) | null { + const lockPath = `${filePath}.lock`; + for (let attempt = 0; attempt < attempts; attempt += 1) { + try { + fs.mkdirSync(path.dirname(lockPath), { recursive: true, mode: 0o700 }); + fs.mkdirSync(lockPath); + return () => { + try { + fs.rmdirSync(lockPath); + } catch { + // Another process already reclaimed it as stale; nothing to undo. + } + }; + } catch (error) { + if (!isLockContention(error)) return null; + let reclaimed = false; + try { + // A negative age is a clock that moved backwards after the lock was + // taken; treating it as fresh would wedge the feature until the clock + // caught up, so both directions past the window count as stale. + const age = now() - fs.statSync(lockPath).mtimeMs; + if (age > LOCK_STALE_MS || age < -LOCK_STALE_MS) { + fs.rmdirSync(lockPath); + reclaimed = true; + } + } catch { + // Released or reclaimed underneath us; the next attempt settles it. + reclaimed = true; + } + // A lock we just reclaimed is free right now, so retry immediately; a + // live one needs the holder to finish, which is what the pause is for. + if (!reclaimed && attempt < attempts - 1) sleepSync(LOCK_RETRY_MS); + } + } + return null; +} + +function withinWindow(sends: readonly AutoDiagnosticsSend[], nowMs: number): AutoDiagnosticsSend[] { + // A stamp in the future is a clock that moved; it still counts as spent so a + // corrupted or shifted clock cannot mint extra sends. + return sends.filter((entry) => nowMs - entry.atMs < AUTO_DIAGNOSTICS_WINDOW_MS); +} + +function mutate( + filePath: string, + now: () => number, + fn: (state: AutoDiagnosticsState, readable: boolean) => { state: AutoDiagnosticsState | null; result: T }, + onLocked: () => T, + lockAttempts: number = LOCK_RETRY_ATTEMPTS, +): T { + const release = acquireLock(filePath, now, lockAttempts); + if (!release) return onLocked(); + try { + const { state, readable } = readState(filePath); + const outcome = fn(state, readable); + if (outcome.state) writeState(filePath, outcome.state); + return outcome.result; + } catch { + return onLocked(); + } finally { + release(); + } +} + +/** Is automatic sending on? Default ON; an unreadable store still reads ON. */ +export function isAutoDiagnosticsEnabled(filePath: string): boolean { + return readState(filePath).state.enabled; +} + +/** + * Flips the setting, preserving whatever budget has already been spent. + * + * Returns what is actually persisted, which is not always what was asked for. + * This is a consent control: a toggle that reports a state it did not manage to + * save would show the user an "off" that is really on. + * + * Four attempts, weakest third. The lock's own short spin absorbs ordinary + * contention; a patient retry (~1s) absorbs a holder that is slow rather than + * gone; only past that does the unlocked write run, because dropping a + * withdrawal of consent is worse than the narrow race that write carries. + */ +export function setAutoDiagnosticsEnabled( + filePath: string, + enabled: boolean, + deps: { now?: () => number } = {}, +): boolean { + const now = deps.now ?? Date.now; + + /** The correct path. `null` means the lock never came free. */ + const writeUnderLock = (attempts?: number): boolean | null => + mutate( + filePath, + now, + (state) => ({ state: { ...state, enabled }, result: enabled }), + () => null, + attempts, + ); + + const sendsFingerprint = (state: AutoDiagnosticsState): string => JSON.stringify(state.sends); + + /** + * The same read-modify-write with no lock held, which can clobber a + * concurrent ledger write. + * + * So it refuses to run into one it can SEE: the ledger is read twice and the + * replace is abandoned if it moved in between. That does not make the write + * safe. A holder still mid-write — one that has read but not yet replaced — + * is invisible to both reads and gets clobbered by this one, which is exactly + * the case that got us here, since reaching this line means a holder has + * already outlived ~1.1s of waiting. What bounds the damage is what the ledger + * holds: at worst one send's entry is lost, costing a repeated toast or one + * extra send against the day's budget, while the consent flip itself — the + * thing the user is waiting on — still lands. An abandoned attempt also still + * has a locked retry behind it. `null` is "did not write". + */ + const writeWithoutLock = (): boolean | null => { + try { + const before = readState(filePath).state; + if (sendsFingerprint(readState(filePath).state) !== sendsFingerprint(before)) return null; + writeState(filePath, { ...before, enabled }); + // Read back rather than assume: if a writer landed on top of this one, + // their value is the truth and the caller has to be told it. + return readState(filePath).state.enabled; + } catch { + return null; + } + }; + + // Worst case is ~1.2s of synchronous blocking on this call: ~100ms for the + // ordinary acquire, ~1s for the patient one, ~100ms for the final retry. That + // is a user-initiated toggle contending with a process that will not let go, + // so waiting a beat is better than persisting the wrong answer — and every + // wait here is bounded, none of them queue. + const persisted = + writeUnderLock() + ?? writeUnderLock(LOCK_PATIENT_RETRY_ATTEMPTS) + ?? writeWithoutLock() + ?? writeUnderLock(); + if (persisted != null) return persisted; + // Nothing landed. Fail loudly in the only way a boolean can: report the + // state on disk, so the pane redraws to what is real. + return isAutoDiagnosticsEnabled(filePath); +} + +/** + * Reserves one send, or explains why there is none to reserve. + * + * The reservation happens BEFORE the upload, not after it. A budget that only + * counted successes would let a machine whose uploads all fail retry the same + * failure every time it recurs — precisely the loop auto-send is not allowed to + * become. What the user is promised is a ceiling on how often their computer + * talks to ADE by itself, and an attempt is what costs them. + */ +export function claimAutoDiagnosticsSend(args: { + filePath: string; + failureCode: string; + source: AutoDiagnosticsSource; + now?: () => number; +}): AutoDiagnosticsClaim { + const now = args.now ?? Date.now; + const code = normalizeAutoDiagnosticsFailureCode(args.failureCode); + if (!code) return { allowed: false, reason: "state_unavailable" }; + return mutate( + args.filePath, + now, + (state, readable) => { + if (!state.enabled) return { state: null, result: { allowed: false, reason: "disabled" } }; + if (!readable) { + return { state: null, result: { allowed: false, reason: "state_unavailable" } }; + } + const nowMs = now(); + const recent = withinWindow(state.sends, nowMs); + if (recent.filter((entry) => entry.code === code).length >= MAX_AUTO_DIAGNOSTICS_PER_CODE) { + return { state: null, result: { allowed: false, reason: "code_limit" } }; + } + if (recent.length >= MAX_AUTO_DIAGNOSTICS_PER_WINDOW) { + return { state: null, result: { allowed: false, reason: "daily_limit" } }; + } + const entry: AutoDiagnosticsSend = { + code, + atMs: nowMs, + source: args.source, + reportPath: null, + reference: null, + pending: false, + }; + return { + state: { ...state, sends: [...recent, entry].slice(-MAX_RETAINED_SENDS) }, + result: { allowed: true, atMs: nowMs }, + }; + }, + // A store we cannot lock is a store we cannot account against: fail closed + // rather than authorize an unbounded send. + () => ({ allowed: false, reason: "state_unavailable" }), + ); +} + +/** + * Records the result of a claimed send. + * + * `pending` is how a send reaches the user's screen at all. The brain has no + * renderer and the desktop cannot tell whether one received its notice, so a + * successful send is left pending either way, offered to the next renderer that + * subscribes, and cleared only when one acknowledges having toasted it. + * + * Which is why `pending` REQUIRES a reference here rather than trusting the + * caller for it. The only thing that clears the flag is an ack naming the + * upload, so a pending entry with no reference could never be retired: it would + * be replayed to every renderer on every launch, forever. Both senders today + * only set `pending` alongside a successful upload (which always carries one), + * so this costs nothing and closes the trap for the next one. + */ +export function completeAutoDiagnosticsSend(args: { + filePath: string; + failureCode: string; + atMs: number; + reportPath: string | null; + reference: string | null; + pending: boolean; + now?: () => number; +}): void { + const now = args.now ?? Date.now; + const code = normalizeAutoDiagnosticsFailureCode(args.failureCode); + if (!code) return; + const reference = args.reference?.trim() || null; + const pending = args.pending && reference != null; + mutate( + args.filePath, + now, + (state) => { + const index = state.sends.findIndex((entry) => entry.code === code && entry.atMs === args.atMs); + if (index < 0) return { state: null, result: undefined }; + const sends = [...state.sends]; + sends[index] = { + ...sends[index]!, + reportPath: args.reportPath, + reference, + pending, + }; + return { state: { ...state, sends }, result: undefined }; + }, + // Losing the annotation only costs a toast, never a duplicate send: the + // reservation itself is already durable. + () => undefined, + ); +} + +export type AutoDiagnosticsNotice = { + failureCode: string; + reportPath: string | null; + reference: string | null; +}; + +/** + * The successful sends nobody has been shown yet. + * + * Read-only ON PURPOSE. Handing a notice to a renderer is not the same as the + * user seeing it — the window can go away, or never mount its toast host, + * between the two — so listing retires nothing. `ackAutoDiagnosticsNotices` is + * the only thing that clears `pending`, and it is called after the toast exists. + */ +export function listPendingAutoDiagnosticsNotices(filePath: string): AutoDiagnosticsNotice[] { + return readState(filePath).state.sends + .filter((entry) => entry.pending) + .map((entry) => ({ + failureCode: entry.code, + reportPath: entry.reportPath, + reference: entry.reference, + })); +} + +/** + * Retires the notices a renderer has actually put on screen. + * + * Matched by upload reference, which every pending entry has: `pending` is only + * ever set alongside a successful upload, and a successful upload always + * carries one. Unknown or already-cleared references are a no-op, so two + * windows that both toasted the same send can both acknowledge it, in any + * order, without the second one being an error. + * + * A lock we cannot take simply leaves the entry pending: repeating a toast once + * is the cheap failure, and it is the one this direction chooses. + */ +export function ackAutoDiagnosticsNotices( + filePath: string, + references: readonly string[], + deps: { now?: () => number } = {}, +): void { + const now = deps.now ?? Date.now; + const wanted = new Set( + references.map((reference) => reference?.trim()).filter((reference): reference is string => !!reference), + ); + if (wanted.size === 0) return; + mutate( + filePath, + now, + (state) => { + if (!state.sends.some((entry) => entry.pending && entry.reference && wanted.has(entry.reference))) { + return { state: null, result: undefined }; + } + return { + state: { + ...state, + sends: state.sends.map((entry) => + entry.pending && entry.reference && wanted.has(entry.reference) + ? { ...entry, pending: false } + : entry, + ), + }, + result: undefined, + }; + }, + () => undefined, + ); +} + +/** Read-only view for tests and for the settings pane's spend line. */ +export function readAutoDiagnosticsState( + filePath: string, + deps: { now?: () => number } = {}, +): { enabled: boolean; sendsInWindow: number; limit: number } { + const now = deps.now ?? Date.now; + const { state } = readState(filePath); + return { + enabled: state.enabled, + sendsInWindow: withinWindow(state.sends, now()).length, + limit: MAX_AUTO_DIAGNOSTICS_PER_WINDOW, + }; +} diff --git a/apps/desktop/src/main/services/diagnostics/diagnosticReportService.test.ts b/apps/desktop/src/main/services/diagnostics/diagnosticReportService.test.ts index 773dc89c6..1e595d856 100644 --- a/apps/desktop/src/main/services/diagnostics/diagnosticReportService.test.ts +++ b/apps/desktop/src/main/services/diagnostics/diagnosticReportService.test.ts @@ -2,7 +2,11 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterAll, describe, expect, it, vi } from "vitest"; -import { collectDiagnosticReport } from "./diagnosticReportService"; +import { + collectDiagnosticReport, + diagnosticReportRoots, + resolveRevealableDiagnosticReport, +} from "./diagnosticReportService"; const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-diag-report-")); @@ -136,3 +140,47 @@ describe("collectDiagnosticReport", () => { expect(report).not.toContain("requested project root was not recognised"); }); }); + +describe("resolveRevealableDiagnosticReport", () => { + // Regression: the reveal handler carried only the desktop's reports + // directory, so "View" on every toast for a BRAIN send — the sends nobody + // was present for, and therefore the ones most worth opening — threw. + function roots() { + const userDataDir = fs.mkdtempSync(path.join(tempRoot, "userData-")); + const adeDir = fs.mkdtempSync(path.join(tempRoot, "adeHome-")); + for (const dir of diagnosticReportRoots({ userDataDir, adeDir })) { + fs.mkdirSync(dir, { recursive: true }); + } + return { userDataDir, adeDir, list: diagnosticReportRoots({ userDataDir, adeDir }) }; + } + + it("reveals a report written by either sender", () => { + const { userDataDir, adeDir, list } = roots(); + const desktopReport = path.join(userDataDir, "diagnostic-reports", "ade-desktop.md"); + const brainReport = path.join(adeDir, "diagnostic-reports", "ade-brain.md"); + fs.writeFileSync(desktopReport, "# desktop", "utf8"); + fs.writeFileSync(brainReport, "# brain", "utf8"); + + expect(resolveRevealableDiagnosticReport(list, desktopReport)).toBe(desktopReport); + expect(resolveRevealableDiagnosticReport(list, brainReport)).toBe(brainReport); + }); + + it("refuses anything outside both reports directories", () => { + const { adeDir, list } = roots(); + // A neighbour of a reports directory, a walk out of one, and an absolute + // path the renderer simply invented. + const neighbour = path.join(adeDir, "secrets", "credentials.json"); + fs.mkdirSync(path.dirname(neighbour), { recursive: true }); + fs.writeFileSync(neighbour, "{}", "utf8"); + + expect(resolveRevealableDiagnosticReport(list, neighbour)).toBeNull(); + expect( + resolveRevealableDiagnosticReport( + list, + path.join(adeDir, "diagnostic-reports", "..", "secrets", "credentials.json"), + ), + ).toBeNull(); + expect(resolveRevealableDiagnosticReport(list, path.join(os.homedir(), ".ssh", "id_rsa"))) + .toBeNull(); + }); +}); diff --git a/apps/desktop/src/main/services/diagnostics/diagnosticReportService.ts b/apps/desktop/src/main/services/diagnostics/diagnosticReportService.ts index 176bb1ce5..789fc4ccd 100644 --- a/apps/desktop/src/main/services/diagnostics/diagnosticReportService.ts +++ b/apps/desktop/src/main/services/diagnostics/diagnosticReportService.ts @@ -16,6 +16,7 @@ import { } from "../../../../../ade-cli/src/services/diagnostics/diagnosticSources"; import { readVolumeSpace } from "../storage/volume"; import { readLastFailure } from "../runtime/lastFailureStore"; +import { resolvePathWithinRoot } from "../shared/utils"; export { buildDiagnosticIssueUrl, @@ -25,6 +26,46 @@ export { writeDiagnosticReportFile, } from "../../../../../ade-cli/src/services/diagnostics/diagnosticReport"; +/** + * The two directories automatic reports are written to. + * + * There are two because there are two senders: the desktop saves beside the + * app's user data, and the brain — which has no `app.getPath` — saves under the + * machine's `~/.ade`. The toast's "View" has to reach BOTH, and the brain's are + * the ones a user most wants, since a headless send is the one nobody was there + * for. + */ +export function diagnosticReportRoots(args: { userDataDir: string; adeDir: string }): string[] { + return [ + path.join(args.userDataDir, "diagnostic-reports"), + path.join(args.adeDir, "diagnostic-reports"), + ]; +} + +/** + * The absolute path to reveal, or `null` when it is not one of ours. + * + * Same `roots.some(...)` shape as `appRevealPath`'s allowlist check, and for + * the same reason: a renderer hands over a string, and the only paths this may + * ever open are the ones ADE itself wrote. Anything else — `~/.ssh/id_rsa`, a + * `..` walk out of a reports directory — is refused rather than revealed. + */ +export function resolveRevealableDiagnosticReport( + roots: readonly string[], + candidate: string, +): string | null { + const normalized = path.resolve(candidate); + const allowed = roots.some((root) => { + try { + resolvePathWithinRoot(root, normalized); + return true; + } catch { + return false; + } + }); + return allowed ? normalized : null; +} + export type DiagnosticReportRequest = DiagnosticReportContext & { /** Verbatim `UpdateTransactionResult` (or anything JSON) from the caller. */ updateTransaction?: unknown; diff --git a/apps/desktop/src/main/services/ipc/registerIpc.ts b/apps/desktop/src/main/services/ipc/registerIpc.ts index 880c42fb7..732ecf3af 100644 --- a/apps/desktop/src/main/services/ipc/registerIpc.ts +++ b/apps/desktop/src/main/services/ipc/registerIpc.ts @@ -743,6 +743,7 @@ import type { createAgentToolsService } from "../agentTools/agentToolsService"; import type { createDevToolsService } from "../devTools/devToolsService"; import type { createOnboardingService } from "../onboarding/onboardingService"; import { getSharedAccountAuthService } from "../../../../../ade-cli/src/services/account/sharedAccountAuthService"; +import { resolveMachineAdeLayout } from "../../../../../ade-cli/src/services/projects/machineLayout"; import type { PushRelayClient } from "../../../../../ade-cli/src/services/push/pushRelayClient"; import type { DevToolsCheckResult } from "../../../shared/types/devTools"; import type { createAutomationService } from "../automations/automationService"; @@ -811,11 +812,17 @@ import { openExternalUrl } from "../shared/externalLinks"; import { resolveAdeLayout } from "../../../shared/adeLayout"; import { collectDiagnosticReport, + diagnosticReportRoots, + resolveRevealableDiagnosticReport, writeDiagnosticReportFile, } from "../diagnostics/diagnosticReportService"; +import type { AutoDiagnosticsService } from "../diagnostics/autoDiagnosticsService"; +import { MAX_AUTO_DIAGNOSTICS_PER_WINDOW } from "../diagnostics/autoDiagnosticsStore"; import type { DiagnosticReportPayload, DiagnosticReportRequestPayload, + DiagnosticsAutoSentPayload, + DiagnosticsSharingStatus, } from "../../../shared/types/diagnostics"; const APP_RESOURCE_USAGE_CACHE_MS = 900; @@ -1662,6 +1669,7 @@ export function registerIpc({ releaseRepository = DEFAULT_RELEASE_REPOSITORY, builtInBrowserService, productAnalyticsService, + autoDiagnosticsService, publishAttentionNotchSnapshot, publishAttentionNotchToast, updateAttentionNotchSettings, @@ -1707,6 +1715,11 @@ export function registerIpc({ releaseRepository?: string; builtInBrowserService?: ReturnType | null; productAnalyticsService?: ProductAnalyticsService; + /** + * Owns the auto-send setting, the budget and the send itself. Absent only in + * tests and in runtime modes that never built one; every call site guards. + */ + autoDiagnosticsService?: AutoDiagnosticsService; publishAttentionNotchSnapshot?: (snapshot: AttentionSnapshot) => void; publishAttentionNotchToast?: (toast: AttentionNotchToast) => void; updateAttentionNotchSettings?: (settings: AttentionNotchSettings) => void; @@ -4782,6 +4795,123 @@ export function registerIpc({ }, ); + /** + * The renderer's failure surfaces (the crash boundaries) asking for one + * automatic send. Main decides: the setting, the budget and the send all live + * there, so a renderer that fires this repeatedly changes nothing. + * + * `projectRoot` is deliberately NOT taken from the payload. It selects which + * project's log directory gets read into the report, and a renderer is the + * one participant here that must not choose that — the only caller + * (`RendererErrorBoundary`) never sends one anyway. Main uses the project it + * already has open, or none. The manual `openIssue` path is unchanged: there + * a person is choosing to file about the screen they are looking at. + */ + ipcMain.handle( + IPC.diagnosticsAutoReport, + async (_event, arg: DiagnosticReportRequestPayload | undefined): Promise => { + const code = typeof arg?.code === "string" ? arg.code : ""; + if (!autoDiagnosticsService || !code.trim()) return; + await autoDiagnosticsService.report({ + failureCode: code, + surface: typeof arg?.surface === "string" && arg.surface.trim() ? arg.surface.trim() : "unknown", + headline: typeof arg?.headline === "string" ? arg.headline.slice(0, 300) : null, + technicalDetail: typeof arg?.technicalDetail === "string" + ? arg.technicalDetail.slice(0, 16_000) + : null, + projectRoot: getCtx().project?.rootPath ?? null, + }); + }, + ); + + const diagnosticsSharingStatus = (): DiagnosticsSharingStatus => + autoDiagnosticsService?.getStatus() + ?? { enabled: true, sendsInWindow: 0, limit: MAX_AUTO_DIAGNOSTICS_PER_WINDOW }; + + ipcMain.handle(IPC.diagnosticsGetSharing, async (): Promise => + diagnosticsSharingStatus()); + + ipcMain.handle( + IPC.diagnosticsSetSharing, + async (_event, arg: { enabled?: boolean } | undefined): Promise => { + autoDiagnosticsService?.setEnabled(arg?.enabled === true); + return diagnosticsSharingStatus(); + }, + ); + + /** + * Hands over the notices nobody has been shown, and clears NOTHING. + * + * Clearing here would record "asked" as "displayed" — the same mistake the + * fast path already cannot make — and this handler is the one place where the + * difference is visible: the window can vanish between this loop and the + * toast. The renderer acknowledges each reference once it has rendered it + * (`diagnosticsAckAutoSent`), and that is the only thing that retires a + * notice. + */ + ipcMain.handle(IPC.diagnosticsFlushAutoSent, async (event): Promise => { + const pending = autoDiagnosticsService?.listPendingNotices() ?? []; + for (const notice of pending) { + try { + event.sender.send(IPC.diagnosticsAutoSent, { + failureCode: notice.failureCode, + reportPath: notice.reportPath ?? "", + reference: notice.reference ?? "", + } satisfies DiagnosticsAutoSentPayload); + } catch { + // A window that went away simply does not get the toast; the report was + // still sent, and it stays pending for the next window to show. + } + } + }); + + ipcMain.handle( + IPC.diagnosticsAckAutoSent, + async (_event, arg: { references?: unknown } | undefined): Promise => { + const references = Array.isArray(arg?.references) + ? arg.references + .filter((value): value is string => typeof value === "string") + // A renderer only ever holds the handful it was just sent; the cap + // is here so a malformed caller cannot hand this a huge array. + .slice(0, 64) + : []; + autoDiagnosticsService?.ackNotices(references); + }, + ); + + /** + * Reveals a saved auto-report, and nothing else. + * + * Deliberately NOT `appRevealPath`: that one validates against the project + * root and the user's Downloads/Documents/temp, and diagnostic reports live + * outside all of those. Widening that allowlist for every caller to serve one + * toast button would be the wrong trade; this handler carries the two + * directories it needs instead. + * + * BOTH senders write reports, to different places — the desktop under + * `userData/diagnostic-reports`, the brain under + * `/diagnostic-reports` — and the brain's are precisely the ones a + * user is most likely to want, since a headless send is the one they were not + * present for. Allowing only the desktop's root left "View" on every brain + * toast throwing on click. + */ + ipcMain.handle( + IPC.diagnosticsRevealReport, + async (_event, arg: { reportPath?: string } | undefined): Promise => { + const raw = typeof arg?.reportPath === "string" ? arg.reportPath.trim() : ""; + if (!raw) return; + const resolved = resolveRevealableDiagnosticReport( + diagnosticReportRoots({ + userDataDir: app.getPath("userData"), + adeDir: resolveMachineAdeLayout().adeDir, + }), + raw, + ); + if (!resolved) throw new Error("Path is outside allowed directories."); + shell.showItemInFolder(resolved); + }, + ); + ipcMain.handle(IPC.projectStateGetSnapshot, async (): Promise => { const ctx = getCtx(); if (!ctx.adeProjectService) throw new Error("Project state service unavailable."); diff --git a/apps/desktop/src/main/services/runtime/projectRecoveryService.test.ts b/apps/desktop/src/main/services/runtime/projectRecoveryService.test.ts index fc414fdea..08bd2b3d4 100644 --- a/apps/desktop/src/main/services/runtime/projectRecoveryService.test.ts +++ b/apps/desktop/src/main/services/runtime/projectRecoveryService.test.ts @@ -286,6 +286,69 @@ describe("ProjectRecoveryService.diagnose", () => { expect(diagnosis.state).toBe("socket_owned_by_other"); }); + + it("announces a terminal diagnosis with the code that produced it", async () => { + const onTerminalDiagnosis = vi.fn(); + const service = createProjectRecoveryService(deps({ + statfs: vi.fn(async () => ({ bavail: GIB / 2, bsize: 1 })), + onTerminalDiagnosis, + })); + const root = tempRoot(); + + await service.diagnose(root); + + expect(onTerminalDiagnosis).toHaveBeenCalledWith({ code: "disk_full", projectRoot: root }); + }); + + it("stays quiet for a healthy project and for a brain that is still starting", async () => { + const onTerminalDiagnosis = vi.fn(); + const healthy = createProjectRecoveryService(deps({ + probeSocket: vi.fn(async () => true), + pingEndpoint: vi.fn(async () => true), + onTerminalDiagnosis, + })); + await healthy.diagnose(tempRoot()); + + const starting = createProjectRecoveryService(deps({ + connectionPool: pool(status({ + serviceHealth: { + state: "running" as const, + installed: true, + running: true, + path: null, + message: null, + checkedAt: null, + }, + serviceInstall: { + state: "installed" as const, + attempted: true, + path: null, + message: null, + exitCode: null, + updatedAt: null, + attemptStartedAt: new Date(NOW - 1_000).toISOString(), + }, + })), + onTerminalDiagnosis, + })); + const startingDiagnosis = await starting.diagnose(tempRoot()); + + expect(startingDiagnosis.state).toBe("brain_starting"); + // A booting brain is not a failure yet; reporting it would spend the day's + // budget on something that fixes itself seconds later. + expect(onTerminalDiagnosis).not.toHaveBeenCalled(); + }); + + it("keeps the diagnosis when the listener throws", async () => { + const service = createProjectRecoveryService(deps({ + statfs: vi.fn(async () => ({ bavail: GIB / 2, bsize: 1 })), + onTerminalDiagnosis: () => { + throw new Error("listener exploded"); + }, + })); + + await expect(service.diagnose(tempRoot())).resolves.toMatchObject({ state: "disk_full" }); + }); }); describe("ProjectRecoveryService.repair", () => { diff --git a/apps/desktop/src/main/services/runtime/projectRecoveryService.ts b/apps/desktop/src/main/services/runtime/projectRecoveryService.ts index fd0b50e61..823c8dfec 100644 --- a/apps/desktop/src/main/services/runtime/projectRecoveryService.ts +++ b/apps/desktop/src/main/services/runtime/projectRecoveryService.ts @@ -104,6 +104,20 @@ export type ProjectRecoveryServiceDeps = { clearFailureReports?: (projectRoot: string) => Promise; readChatCounts?: (projectRoot: string) => Promise; socketExists?: (socketPath: string) => boolean; + /** + * Fires when a diagnosis settles on a state the user cannot work through — + * everything except `healthy` and the transient `brain_starting`. Automatic + * diagnostics listens here; the callback owns its own budget, so a screen + * that re-diagnoses on every poll costs nothing. + * + * Carries only what the one listener uses. `state` is derivable from `code` + * and was passed unused, which is how a notification callback turns into a + * second, informal copy of the diagnosis type. + */ + onTerminalDiagnosis?: (input: { + code: AdeRecoveryErrorCode; + projectRoot: string; + }) => void; now?: () => number; }; @@ -581,6 +595,14 @@ export class ProjectRecoveryService { code = "unknown"; } + if (state !== "healthy" && state !== "brain_starting") { + try { + this.deps.onTerminalDiagnosis?.({ code, projectRoot: normalizedRoot }); + } catch { + // A listener must never cost the user their diagnosis. + } + } + const required = RECOMMENDED_FREE_BYTES(dbSize); return { state, diff --git a/apps/desktop/src/preload/global.d.ts b/apps/desktop/src/preload/global.d.ts index 3f3d37f9a..e69340df1 100644 --- a/apps/desktop/src/preload/global.d.ts +++ b/apps/desktop/src/preload/global.d.ts @@ -725,7 +725,12 @@ import type { StorageSnapshot, } from "../shared/types/storage"; import type { ProjectRecoveryDiagnosis, ProjectRepairReport, RepairStepResult } from "../shared/types/recovery"; -import type { DiagnosticReportPayload, DiagnosticReportRequestPayload } from "../shared/types/diagnostics"; +import type { + DiagnosticReportPayload, + DiagnosticReportRequestPayload, + DiagnosticsAutoSentPayload, + DiagnosticsSharingStatus, +} from "../shared/types/diagnostics"; import type { AppPackageChannel } from "../shared/packageChannel"; import type { ProductAnalyticsCapture, @@ -899,11 +904,30 @@ declare global { onStateEvent: (cb: (event: AdeProjectEvent) => void) => () => void; }; /** - * Absent on older preloads: every call site must tolerate `undefined` - * and simply not offer the button. + * Optional as a GROUP, because an older preload has no `diagnostics` at + * all: every call site must tolerate `undefined` and simply not offer the + * button. The members inside are not optional — they all shipped + * together, so a build that exposes the group exposes all of them, and + * marking them individually optional would only teach call sites to write + * `?.()` chains that can never fire. */ diagnostics?: { openIssue: (context: DiagnosticReportRequestPayload) => Promise; + /** + * Ask main to consider ONE automatic send for a failure the renderer + * detected. Main owns the setting and the budget, so this is a request, + * not an instruction, and its answer is deliberately uninteresting. + */ + autoReport: (context: DiagnosticReportRequestPayload) => Promise; + getSharing: () => Promise; + setSharing: (enabled: boolean) => Promise; + revealReport: (reportPath: string) => Promise; + onAutoSent: (cb: (payload: DiagnosticsAutoSentPayload) => void) => () => void; + /** + * Confirms these references reached the screen, so main stops offering + * them on the next subscribe. Called after the toast is rendered. + */ + ackAutoSent: (references: string[]) => Promise; }; recovery: { diagnose: (projectRoot: string) => Promise; diff --git a/apps/desktop/src/preload/preload.ts b/apps/desktop/src/preload/preload.ts index f035a87c7..12d58b149 100644 --- a/apps/desktop/src/preload/preload.ts +++ b/apps/desktop/src/preload/preload.ts @@ -28,7 +28,12 @@ import { } from "./pinnedRuntimeEvents"; import type { OrchestrationEventPayload } from "../shared/types/orchestration"; import type { ProjectRecoveryDiagnosis, ProjectRepairReport, RepairStepResult } from "../shared/types/recovery"; -import type { DiagnosticReportPayload, DiagnosticReportRequestPayload } from "../shared/types/diagnostics"; +import type { + DiagnosticReportPayload, + DiagnosticReportRequestPayload, + DiagnosticsAutoSentPayload, + DiagnosticsSharingStatus, +} from "../shared/types/diagnostics"; import type { ProductAnalyticsCapture, ProductAnalyticsCaptureResult, @@ -4106,6 +4111,35 @@ const adeBridge = { context: DiagnosticReportRequestPayload, ): Promise => ipcRenderer.invoke(IPC.diagnosticsOpenIssue, context), + autoReport: (context: DiagnosticReportRequestPayload): Promise => + ipcRenderer.invoke(IPC.diagnosticsAutoReport, context), + getSharing: (): Promise => + ipcRenderer.invoke(IPC.diagnosticsGetSharing), + setSharing: (enabled: boolean): Promise => + ipcRenderer.invoke(IPC.diagnosticsSetSharing, { enabled }), + revealReport: (reportPath: string): Promise => + ipcRenderer.invoke(IPC.diagnosticsRevealReport, { reportPath }), + /** + * "I have shown these to the user." Main stops offering them; anything it + * does not hear about is offered again next time a renderer subscribes. + */ + ackAutoSent: (references: string[]): Promise => + ipcRenderer.invoke(IPC.diagnosticsAckAutoSent, { references }), + onAutoSent: (cb: (payload: DiagnosticsAutoSentPayload) => void) => { + const listener = ( + _event: Electron.IpcRendererEvent, + payload: DiagnosticsAutoSentPayload, + ) => cb(payload); + ipcRenderer.on(IPC.diagnosticsAutoSent, listener); + // Subscribing is what asks for any send the brain made while no window + // was listening, so a headless auto-send still gets its toast and nothing + // has to poll for one. Registered before the call, so the reply cannot + // arrive ahead of the listener. + void ipcRenderer.invoke(IPC.diagnosticsFlushAutoSent).catch(() => undefined); + return () => { + ipcRenderer.removeListener(IPC.diagnosticsAutoSent, listener); + }; + }, }, recovery: { diagnose: (projectRoot: string): Promise => diff --git a/apps/desktop/src/renderer/components/app/AppShell.tsx b/apps/desktop/src/renderer/components/app/AppShell.tsx index a5a4c1cbf..065815f80 100644 --- a/apps/desktop/src/renderer/components/app/AppShell.tsx +++ b/apps/desktop/src/renderer/components/app/AppShell.tsx @@ -77,6 +77,7 @@ import { BrainRecoveryNotice } from "./BrainRecoveryNotice"; import { WorktreeOpenDialog } from "../projects/WorktreeOpenDialog"; import { showToast, useToasts } from "./toast/toastStore"; import { useLaneEventToasts } from "./toast/useLaneEventToasts"; +import { useAutoDiagnosticsToast } from "./toast/useAutoDiagnosticsToast"; import { useProductAnalyticsLifecycle } from "../analytics/ProductAnalyticsLifecycle"; import { useAppWideSessionAttention } from "../../hooks/useAppWideSessionAttention"; import { useCtoAttention } from "../../hooks/useCtoAttention"; @@ -286,6 +287,7 @@ export function AppShell({ children }: { children: React.ReactNode }) { const shellMainRef = useRef(null); const navigate = useNavigate(); useLaneEventToasts(navigate); + useAutoDiagnosticsToast(); const setProject = useAppStore((s) => s.setProject); const setProjectHydrated = useAppStore((s) => s.setProjectHydrated); const setProjectBinding = useAppStore((s) => s.setProjectBinding); diff --git a/apps/desktop/src/renderer/components/app/RendererErrorBoundary.tsx b/apps/desktop/src/renderer/components/app/RendererErrorBoundary.tsx index e2b93d9b9..fd08938d3 100644 --- a/apps/desktop/src/renderer/components/app/RendererErrorBoundary.tsx +++ b/apps/desktop/src/renderer/components/app/RendererErrorBoundary.tsx @@ -32,7 +32,25 @@ export class RendererErrorBoundary extends React.Component<{ children: React.Rea }; } + /** + * One automatic report per crash, not per re-render. `componentDidCatch` is + * React's once-per-caught-error hook (`getDerivedStateFromError` can run + * twice under StrictMode), and this flag covers a boundary that catches again + * after a failed recovery. + */ + private autoReported = false; + componentDidCatch(error: unknown, info: React.ErrorInfo): void { + if (!this.autoReported) { + this.autoReported = true; + // Main owns the setting and the budget; this is a request, and its answer + // is deliberately uninteresting to a screen that is already broken. + void window.ade?.diagnostics?.autoReport({ + surface: "renderer_crash", + code: "renderer_crash", + headline: "ADE needs to reload this window", + }).catch(() => undefined); + } // Keep renderer crashes visible in devtools logs and avoid a blank screen. console.error("renderer.crash", { error: error instanceof Error ? error.message : String(error), diff --git a/apps/desktop/src/renderer/components/app/SettingsPage.test.tsx b/apps/desktop/src/renderer/components/app/SettingsPage.test.tsx index 05fcf8bcb..3cb0ba1ff 100644 --- a/apps/desktop/src/renderer/components/app/SettingsPage.test.tsx +++ b/apps/desktop/src/renderer/components/app/SettingsPage.test.tsx @@ -30,6 +30,7 @@ function stubSection(anchors: string[]) { vi.mock("../settings/ProjectSection", () => ({ ProjectSection: stubSection(["project"]) })); vi.mock("../settings/ProductAnalyticsSection", () => ({ ProductAnalyticsSection: stubSection(["product-analytics"]) })); +vi.mock("../settings/DiagnosticsSharingSection", () => ({ DiagnosticsSharingSection: stubSection(["diagnostics-sharing"]) })); vi.mock("../settings/AboutSection", () => ({ AboutSection: () => (
diff --git a/apps/desktop/src/renderer/components/app/SettingsPage.tsx b/apps/desktop/src/renderer/components/app/SettingsPage.tsx index 1f59d0f62..2af40b379 100644 --- a/apps/desktop/src/renderer/components/app/SettingsPage.tsx +++ b/apps/desktop/src/renderer/components/app/SettingsPage.tsx @@ -29,6 +29,7 @@ import { LinearIntegrationSection } from "../settings/LinearIntegrationSection"; import { NotificationsSection } from "../settings/NotificationsSection"; import { PrChatTranscriptsSection } from "../settings/PrChatTranscriptsSection"; import { ProductAnalyticsSection } from "../settings/ProductAnalyticsSection"; +import { DiagnosticsSharingSection } from "../settings/DiagnosticsSharingSection"; import { ProjectSection } from "../settings/ProjectSection"; import { ProvidersSection } from "../settings/ProvidersSection"; import { SecretsSection } from "../settings/SecretsSection"; @@ -153,6 +154,9 @@ function TabContent({ tab }: { tab: SettingsTabId }) { + + + ); case "appearance": diff --git a/apps/desktop/src/renderer/components/app/toast/ToastStack.tsx b/apps/desktop/src/renderer/components/app/toast/ToastStack.tsx index edc5e2bb0..8b208ba16 100644 --- a/apps/desktop/src/renderer/components/app/toast/ToastStack.tsx +++ b/apps/desktop/src/renderer/components/app/toast/ToastStack.tsx @@ -1,3 +1,4 @@ +import { useEffect } from "react"; import { X } from "@phosphor-icons/react"; import { cn } from "../../ui/cn"; @@ -6,6 +7,7 @@ import { pauseToast, resumeToast, useToasts, + type Toast, type ToastTone, } from "./toastStore"; @@ -41,67 +43,96 @@ export function ToastStack() { return ( <> - {toasts.map((toast) => { - const tone = toneClasses(toast.tone); - return ( -
pauseToast(toast.id)} - onMouseLeave={() => resumeToast(toast.id)} - > -
-
-
- {toast.colorDot ? ( - - ) : null} -
- {toast.title} -
-
- {toast.message ? ( -
- {toast.message} -
- ) : null} - {toast.action ? ( -
- -
- ) : null} -
- + {toasts.map((toast) => ( + + ))} + + ); +} + +/** + * One card, and the only place that can say a toast is really on screen. + * + * Split out of the map purely so `onRendered` can be an effect: it has to run + * after React commits this card, which is the difference between "queued" and + * "shown" for the callers that report delivery upstream. + */ +function ToastCard({ toast }: { toast: Toast }) { + const onRendered = toast.onRendered; + useEffect(() => { + onRendered?.(); + }, [onRendered]); + + const tone = toneClasses(toast.tone); + return ( +
pauseToast(toast.id)} + onMouseLeave={() => resumeToast(toast.id)} + > +
+
+
+ {toast.colorDot ? ( + + ) : null} +
+ {toast.title}
- ); - })} - + {toast.message ? ( +
+ {toast.message} +
+ ) : null} + {toast.action || toast.secondaryAction ? ( +
+ {toast.action ? ( + + ) : null} + {toast.secondaryAction ? ( + + ) : null} +
+ ) : null} +
+ +
+
); } diff --git a/apps/desktop/src/renderer/components/app/toast/toastStore.test.ts b/apps/desktop/src/renderer/components/app/toast/toastStore.test.ts index 8b03a66e1..c40092fc6 100644 --- a/apps/desktop/src/renderer/components/app/toast/toastStore.test.ts +++ b/apps/desktop/src/renderer/components/app/toast/toastStore.test.ts @@ -1,7 +1,7 @@ /* @vitest-environment jsdom */ import React from "react"; -import { cleanup, render } from "@testing-library/react"; +import { act, cleanup, render, waitFor } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { LaneLifecycleEvent, @@ -17,7 +17,10 @@ import { showToast, updateToast, } from "./toastStore"; +import { ToastStack } from "./ToastStack"; import { useLaneEventToasts } from "./useLaneEventToasts"; +import { useAutoDiagnosticsToast } from "./useAutoDiagnosticsToast"; +import type { DiagnosticsAutoSentPayload } from "../../../../shared/types/diagnostics"; // The store is a module-level singleton with no reset hook; each test clears // the stack it created so state doesn't leak between cases. @@ -320,3 +323,162 @@ describe("useLaneEventToasts", () => { }); }); }); + +describe("useAutoDiagnosticsToast", () => { + /** The real pairing: the subscriber and the toast host, as `AppShell` mounts them. */ + function AutoDiagnosticsHarness(): React.ReactElement { + useAutoDiagnosticsToast(); + return React.createElement(ToastStack); + } + + /** Subscriber with NO toast host — a notice that is queued and never shown. */ + function SubscriberOnlyHarness(): React.ReactElement | null { + useAutoDiagnosticsToast(); + return null; + } + + function installDiagnosticsApi() { + let listener: ((payload: DiagnosticsAutoSentPayload) => void) | null = null; + const bridge = { + diagnostics: { + openIssue: vi.fn(), + onAutoSent: vi.fn((cb: (payload: DiagnosticsAutoSentPayload) => void) => { + listener = cb; + return () => { + listener = null; + }; + }), + revealReport: vi.fn(async () => {}), + setSharing: vi.fn(async () => ({ enabled: false, sendsInWindow: 1, limit: 3 })), + ackAutoSent: vi.fn(async () => {}), + }, + }; + Object.defineProperty(window, "ade", { value: bridge, configurable: true, writable: true }); + return { + bridge, + // Wrapped in `act` because the store update this triggers re-renders the + // toast host, and the render is what reports delivery. + emit: (payload: DiagnosticsAutoSentPayload) => { + act(() => { + listener?.(payload); + }); + }, + }; + } + + beforeEach(() => { + // The store is a module singleton and the suites above leave toasts behind. + cleanup(); + clearAll(); + }); + + afterEach(() => { + delete (window as { ade?: unknown }).ade; + }); + + it("tells the user what was sent, and offers the report and the off switch", () => { + const api = installDiagnosticsApi(); + render(React.createElement(AutoDiagnosticsHarness)); + + api.emit({ failureCode: "disk_full", reportPath: "/reports/x.md", reference: "abcd1234" }); + + const [toast] = getToasts(); + expect(toast).toMatchObject({ + title: "A diagnostic report was sent to ADE", + message: "Reference abcd1234", + }); + expect(toast?.action?.label).toBe("View"); + expect(toast?.secondaryAction?.label).toBe("Turn off"); + + toast?.action?.onClick(); + expect(api.bridge.diagnostics.revealReport).toHaveBeenCalledWith("/reports/x.md"); + toast?.secondaryAction?.onClick(); + expect(api.bridge.diagnostics.setSharing).toHaveBeenCalledWith(false); + }); + + it("tells main the toast exists, so the next launch does not repeat it", () => { + // The ack is the ONLY thing that retires a notice: main cannot tell that + // `webContents.send` reached a live toast host, and listing the pending + // ones on subscribe deliberately clears nothing. It goes out after the + // toast, so a renderer that dies mid-render repeats one rather than + // swallowing it. + const api = installDiagnosticsApi(); + render(React.createElement(AutoDiagnosticsHarness)); + + api.emit({ failureCode: "disk_full", reportPath: "/reports/x.md", reference: "abcd1234" }); + + expect(getToasts()).toHaveLength(1); + expect(api.bridge.diagnostics.ackAutoSent).toHaveBeenCalledWith(["abcd1234"]); + }); + + it("acknowledges an automatic diagnostic only after the toast is rendered", () => { + // Regression: the ack used to fire beside `showToast`, which only queues. + // React had committed nothing at that point, so a window that died before + // the paint retired a notice the user never saw — and main, which trusts + // the ack completely, would never offer it again. + const api = installDiagnosticsApi(); + render(React.createElement(SubscriberOnlyHarness)); + + api.emit({ failureCode: "disk_full", reportPath: "/reports/x.md", reference: "abcd1234" }); + + // Queued, not shown: nothing may claim delivery yet. + expect(getToasts()).toHaveLength(1); + expect(api.bridge.diagnostics.ackAutoSent).not.toHaveBeenCalled(); + + // The commit is the claim. + render(React.createElement(ToastStack)); + expect(api.bridge.diagnostics.ackAutoSent).toHaveBeenCalledWith(["abcd1234"]); + }); + + it("says so when turning sharing off did not save", async () => { + // `ToastStack` dismisses the toast as soon as the click returns, so a + // refused write would otherwise leave the user believing auto-send is off. + const api = installDiagnosticsApi(); + api.bridge.diagnostics.setSharing.mockResolvedValue({ + enabled: true, + sendsInWindow: 1, + limit: 3, + }); + render(React.createElement(AutoDiagnosticsHarness)); + + api.emit({ failureCode: "disk_full", reportPath: "/reports/x.md", reference: "abcd1234" }); + const [toast] = getToasts(); + act(() => { + toast?.secondaryAction?.onClick(); + }); + + await waitFor(() => { + expect(getToasts().some((entry) => entry.title === "ADE could not turn this off")).toBe(true); + }); + }); + + it("shows one toast for a report delivered twice", () => { + // Main keeps a successful send marked pending REGARDLESS of whether a + // window was sent the notice, because `webContents.send` cannot report that + // anything received it. So the fast-path send and the replay on subscribe + // can both arrive; keying on the reference is what makes that safe, and the + // repeated ack is a no-op on main. + const api = installDiagnosticsApi(); + render(React.createElement(AutoDiagnosticsHarness)); + + const payload = { failureCode: "disk_full", reportPath: "/reports/x.md", reference: "abcd1234" }; + api.emit(payload); + api.emit(payload); + + expect(getToasts()).toHaveLength(1); + expect(getToasts()[0]?.id).toBe("diagnostics-auto-sent-abcd1234"); + }); + + it("still offers the off switch when the local copy could not be written", () => { + const api = installDiagnosticsApi(); + render(React.createElement(AutoDiagnosticsHarness)); + + api.emit({ failureCode: "disk_full", reportPath: "", reference: "abcd1234" }); + + const [toast] = getToasts(); + // Nothing to reveal, so no dead "View" button — but turning it off must + // always be one click away from the message that says it happened. + expect(toast?.action).toBeUndefined(); + expect(toast?.secondaryAction?.label).toBe("Turn off"); + }); +}); diff --git a/apps/desktop/src/renderer/components/app/toast/toastStore.ts b/apps/desktop/src/renderer/components/app/toast/toastStore.ts index b1c5bd964..be65412f2 100644 --- a/apps/desktop/src/renderer/components/app/toast/toastStore.ts +++ b/apps/desktop/src/renderer/components/app/toast/toastStore.ts @@ -23,8 +23,25 @@ export type ToastInput = { /** CSS color for the small lane dot rendered before the title. */ colorDot?: string; action?: ToastAction; + /** + * A second, quieter action beside the first. Added for notices that offer + * both "look at this" and "stop doing this"; most toasts want neither or one. + */ + secondaryAction?: ToastAction; /** Auto-dismiss delay; <= 0 or non-finite keeps the toast until dismissed. */ durationMs?: number; + /** + * Fires once `ToastStack` has actually committed this toast to the DOM. + * + * Queueing a toast is not the same as showing one: `showToast` only mutates + * this module, and React has not rendered anything at the point it returns. A + * caller that has to state truthfully that the user was shown something — + * `useAutoDiagnosticsToast`, which tells main it may stop offering a notice — + * has to wait for the commit, so the render path reports it rather than the + * queue path guessing. Re-fires when a toast is replaced in place with a new + * callback, which is what makes a repeated notice acknowledge again. + */ + onRendered?: () => void; }; export type Toast = { @@ -34,7 +51,9 @@ export type Toast = { tone: ToastTone; colorDot?: string; action?: ToastAction; + secondaryAction?: ToastAction; durationMs: number; + onRendered?: () => void; }; /** Merge-patch shape for {@link updateToast}. */ @@ -114,7 +133,9 @@ export function showToast(input: ToastInput): string { tone: input.tone ?? "info", colorDot: input.colorDot, action: input.action, + secondaryAction: input.secondaryAction, durationMs, + onRendered: input.onRendered, }; const existingIndex = toasts.findIndex((t) => t.id === id); diff --git a/apps/desktop/src/renderer/components/app/toast/useAutoDiagnosticsToast.ts b/apps/desktop/src/renderer/components/app/toast/useAutoDiagnosticsToast.ts new file mode 100644 index 000000000..cf4017d5a --- /dev/null +++ b/apps/desktop/src/renderer/components/app/toast/useAutoDiagnosticsToast.ts @@ -0,0 +1,89 @@ +import { useEffect } from "react"; +import { showToast } from "./toastStore"; + +/** + * One toast per automatic diagnostic report. + * + * Auto-send only stays acceptable if it is never invisible: something left this + * computer, and the person it belongs to gets told, every time, with the two + * things they might want next — the report itself, and the off switch. + * + * Subscribing is also what asks for reports the brain sent while no window was + * open, so a headless send surfaces the next time ADE is on screen instead of + * being silently dropped. + * + * And this hook is the only thing that can honestly say a send was shown, so it + * says it: every notice — live fast path or replayed on subscribe, they arrive + * through the same callback — is acknowledged once its toast has been COMMITTED + * to the screen, not merely queued. `showToast` returns before React has + * rendered anything, so acknowledging there would claim delivery for a window + * that could still die before the toast appeared; `onRendered` fires from + * `ToastStack`'s own effect instead. Main keeps offering anything + * unacknowledged, so a window that dies mid-render repeats one toast rather + * than swallowing it, and a window that rendered it never sees it again on the + * next launch. + */ +export function useAutoDiagnosticsToast(): void { + useEffect(() => { + const bridge = window.ade?.diagnostics; + if (!bridge) return; + return bridge.onAutoSent((payload) => { + const reportPath = payload.reportPath?.trim() || ""; + const reference = payload.reference?.trim() || ""; + showToast({ + // Per report rather than per failure: two different failures in a day + // are two different things the user was told about. + id: `diagnostics-auto-sent-${reference || payload.failureCode}`, + title: "A diagnostic report was sent to ADE", + message: reference ? `Reference ${reference}` : undefined, + tone: "info", + durationMs: 10_000, + ...(reportPath + ? { + action: { + label: "View", + onClick: () => { + void bridge.revealReport(reportPath).catch(() => undefined); + }, + }, + } + : {}), + secondaryAction: { + label: "Turn off", + onClick: () => { + // `ToastStack` dismisses this toast the moment the click returns, + // so a write that did not land would otherwise leave the user + // believing they turned auto-send off while it is still on. This is + // a consent control: it says so instead. `setSharing` answers with + // what was actually persisted, which is how a refused write shows + // up here — it resolves still-enabled rather than rejecting. + void bridge + .setSharing(false) + .then((status) => { + if (status?.enabled !== false) throw new Error("not_saved"); + }) + .catch(() => { + showToast({ + title: "ADE could not turn this off", + message: "Try again in Settings → General.", + tone: "error", + }); + }); + }, + }, + // The ack is the claim that the toast EXISTS, so it waits for the + // commit rather than firing beside the queueing call. Un-referenced + // notices cannot be acknowledged (nothing to name them by), but they + // also cannot occur — `pending` is only set alongside a successful + // upload, and a successful upload always carries a reference. + ...(reference + ? { + onRendered: () => { + void bridge.ackAutoSent([reference]).catch(() => undefined); + }, + } + : {}), + }); + }); + }, []); +} diff --git a/apps/desktop/src/renderer/components/settings/DiagnosticsSharingSection.tsx b/apps/desktop/src/renderer/components/settings/DiagnosticsSharingSection.tsx new file mode 100644 index 000000000..4d833ca44 --- /dev/null +++ b/apps/desktop/src/renderer/components/settings/DiagnosticsSharingSection.tsx @@ -0,0 +1,32 @@ +import React from "react"; +import { Lifebuoy } from "@phosphor-icons/react"; +import type { DiagnosticsSharingStatus } from "../../../shared/types/diagnostics"; +import { ConsentToggleSection } from "./settingsSectionUi"; + +/** + * The off switch for automatic diagnostic reports. + * + * Same shape as the analytics section next to it, and now literally the same + * component: this is a consent control, so it reads the real persisted state + * rather than assuming, and it says plainly what gets sent and how often. + */ +export function DiagnosticsSharingSection() { + const bridge = window.ade?.diagnostics; + return ( + + id="diagnostics-sharing" + title="Diagnostics sharing" + description="Send ADE a report when something breaks, so it can be fixed." + icon={Lifebuoy} + brandColor="#60A5FA" + label="Share diagnostics with ADE when something breaks" + body={'ADE sends the same report the "Report issue" button makes: app and system versions, recent ADE logs, disk space and the failure code. Paths, names, emails and credentials are removed first. Never your code, chats or terminal output.'} + footnote={(status) => + `At most ${status?.limit ?? 3} a day, one per problem. You get a message every time one is sent.`} + read={bridge ? () => bridge.getSharing() : undefined} + write={bridge ? (enabled) => bridge.setSharing(enabled) : undefined} + readErrorMessage="This setting is unavailable right now." + writeErrorMessage="ADE could not save this setting." + /> + ); +} diff --git a/apps/desktop/src/renderer/components/settings/ProductAnalyticsSection.tsx b/apps/desktop/src/renderer/components/settings/ProductAnalyticsSection.tsx index 433376258..530c44e13 100644 --- a/apps/desktop/src/renderer/components/settings/ProductAnalyticsSection.tsx +++ b/apps/desktop/src/renderer/components/settings/ProductAnalyticsSection.tsx @@ -1,89 +1,26 @@ -import React, { useEffect, useId, useState } from "react"; +import React from "react"; import { ChartLineUp } from "@phosphor-icons/react"; import type { ProductAnalyticsStatus } from "../../../shared/types/productAnalytics"; -import { COLORS, SANS_FONT, cardStyle } from "../lanes/laneDesignTokens"; -import { SettingsSectionShell, SettingsToggle } from "./settingsSectionUi"; +import { ConsentToggleSection } from "./settingsSectionUi"; export function ProductAnalyticsSection() { - const toggleId = useId(); - const [status, setStatus] = useState(null); - const [saving, setSaving] = useState(false); - const [error, setError] = useState(null); - - useEffect(() => { - let cancelled = false; - void window.ade.analytics.getStatus() - .then((next) => { - if (!cancelled) setStatus(next); - }) - .catch(() => { - if (!cancelled) setError("Analytics settings are unavailable right now."); - }); - return () => { - cancelled = true; - }; - }, []); - - const setEnabled = async (enabled: boolean) => { - if (saving) return; - setSaving(true); - setError(null); - try { - setStatus(await window.ade.analytics.setEnabled(enabled)); - } catch { - setError("ADE could not save this analytics preference."); - } finally { - setSaving(false); - } - }; - return ( - id="product-analytics" title="Anonymous product analytics" description="Help improve ADE by sharing bounded, anonymous usage events." icon={ChartLineUp} brandColor="#A78BFA" - > -
-
-
- -

- ADE uses a random installation ID plus installation-salted opaque project and session IDs. It sends only allowlisted feature, screen, outcome, version, and aggregate usage counts—never prompts, code, file or terminal content, repository names or paths, command arguments, or recordings. -

-

- {status?.configured - ? `Daily safety limit: ${status.dailyBudget} events on this ADE installation.` - : "Analytics delivery will remain idle until this ADE build is connected to its analytics project."} -

- {error ? ( -

- {error} -

- ) : null} -
- void setEnabled(enabled)} - /> -
-
- + label="Share anonymous usage analytics" + body="ADE uses a random installation ID plus installation-salted opaque project and session IDs. It sends only allowlisted feature, screen, outcome, version, and aggregate usage counts—never prompts, code, file or terminal content, repository names or paths, command arguments, or recordings." + footnote={(status) => + status?.configured + ? `Daily safety limit: ${status.dailyBudget} events on this ADE installation.` + : "Analytics delivery will remain idle until this ADE build is connected to its analytics project."} + read={() => window.ade.analytics.getStatus()} + write={(enabled) => window.ade.analytics.setEnabled(enabled)} + readErrorMessage="Analytics settings are unavailable right now." + writeErrorMessage="ADE could not save this analytics preference." + /> ); } diff --git a/apps/desktop/src/renderer/components/settings/settingsManifest.test.ts b/apps/desktop/src/renderer/components/settings/settingsManifest.test.ts index 712c1e9db..acb31c74d 100644 --- a/apps/desktop/src/renderer/components/settings/settingsManifest.test.ts +++ b/apps/desktop/src/renderer/components/settings/settingsManifest.test.ts @@ -168,6 +168,7 @@ describe("settings manifest", () => { expect(searchSettingsEntries("banner").map((e) => e.id)).toContain("lanes-git.rebase-suggestions"); expect(searchSettingsEntries("do not disturb").map((e) => e.id)).toContain("notifications.focus-suppression"); expect(searchSettingsEntries("all machines").map((e) => e.id)).toContain("activity.dock-badge"); + expect(searchSettingsEntries("crash").map((e) => e.id)).toContain("general.diagnostics-sharing"); }); it("returns nothing for a blank query rather than every setting", () => { @@ -255,6 +256,7 @@ describe("settings command palette entries", () => { ["scrollback", "setting-appearance.terminal"], ["do not disturb", "setting-notifications.focus-suppression"], ["api key", "setting-secrets.secrets"], + ["crash", "setting-general.diagnostics-sharing"], ]; for (const [query, expectedId] of cases) { const ids = filterPalette(commands, query).map((command) => command.id); diff --git a/apps/desktop/src/renderer/components/settings/settingsManifest.ts b/apps/desktop/src/renderer/components/settings/settingsManifest.ts index 6cf6e6a06..594584062 100644 --- a/apps/desktop/src/renderer/components/settings/settingsManifest.ts +++ b/apps/desktop/src/renderer/components/settings/settingsManifest.ts @@ -161,6 +161,21 @@ export const SETTINGS_ENTRIES: readonly SettingEntry[] = [ web: "browser", group: "Privacy", }, + { + id: "general.diagnostics-sharing", + // Not plain "Diagnostics": `storage.diagnostics` already owns that label, + // and two identically named search hits pointing at different tabs is a + // coin flip for whoever is looking for the off switch. + label: "Diagnostics sharing", + keywords: ["diagnostics", "crash", "report", "privacy", "error", "send", "opt out"], + tab: "general", + anchor: "diagnostics-sharing", + scope: "machine", + // Machine-local consent written into `~/.ade/secrets` by the main process; + // a browser has no such file, so the toggle is not offered there. + web: "hidden", + group: "Privacy", + }, { id: "general.about", label: "About ADE", diff --git a/apps/desktop/src/renderer/components/settings/settingsSectionUi.test.tsx b/apps/desktop/src/renderer/components/settings/settingsSectionUi.test.tsx new file mode 100644 index 000000000..98df66289 --- /dev/null +++ b/apps/desktop/src/renderer/components/settings/settingsSectionUi.test.tsx @@ -0,0 +1,93 @@ +/* @vitest-environment jsdom */ + +import React from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { Lifebuoy } from "@phosphor-icons/react"; +import { ConsentToggleSection } from "./settingsSectionUi"; + +/** + * The two privacy toggles — analytics and automatic diagnostics — are this one + * component. What is tested here is the only thing a consent control must never + * get wrong: the switch shows what was PERSISTED, not what was clicked. + */ + +type Status = { enabled: boolean; limit: number }; + +function renderToggle(overrides: { + read?: () => Promise; + write?: (enabled: boolean) => Promise; +}) { + return render( + + id="consent" + title="Diagnostics" + description="Send ADE a report when something breaks." + icon={Lifebuoy} + brandColor="#60A5FA" + label="Share diagnostics with ADE" + body="What leaves this computer, in one line." + footnote={(status) => `At most ${status?.limit ?? 3} a day.`} + read={overrides.read} + write={overrides.write} + readErrorMessage="This setting is unavailable right now." + writeErrorMessage="ADE could not save this setting." + />, + ); +} + +afterEach(cleanup); + +describe("ConsentToggleSection", () => { + it("renders the state it read and then the state the write returned", async () => { + // The write's answer wins over the click: `setAutoDiagnosticsEnabled` can + // persist something other than what was asked for when the ledger is + // contended, and a toggle that showed the request would show an "off" that + // is really on. + const write = vi.fn(async () => ({ enabled: true, limit: 3 })); + renderToggle({ read: async () => ({ enabled: true, limit: 3 }), write }); + + const toggle = await screen.findByRole("switch"); + await waitFor(() => expect(toggle.hasAttribute("disabled")).toBe(false)); + fireEvent.click(toggle); + + await waitFor(() => expect(write).toHaveBeenCalledWith(false)); + // Asked for off, told it stayed on: the switch says on. + await waitFor(() => expect(screen.getByRole("switch").getAttribute("aria-checked")).toBe("true")); + }); + + it("says so when the setting cannot be read, and does not offer a click", async () => { + renderToggle({ + read: async () => { + throw new Error("bridge down"); + }, + write: async () => ({ enabled: false, limit: 3 }), + }); + + await waitFor(() => + expect(screen.getByRole("alert").textContent).toBe("This setting is unavailable right now."), + ); + // No status means nothing to toggle: the switch stays disabled rather + // than writing against a value nobody has. + expect(screen.getByRole("switch").hasAttribute("disabled")).toBe(true); + }); + + it("surfaces a failed write without pretending the click landed", async () => { + const write = vi.fn(async () => { + throw new Error("no"); + }); + renderToggle({ read: async () => ({ enabled: true, limit: 3 }), write }); + + const toggle = await screen.findByRole("switch"); + // The switch exists before `read` resolves and is disabled until it does; a + // click landing in that window calls nothing and the assertion below would + // time out on an alert that was never going to appear. + await waitFor(() => expect(toggle.hasAttribute("disabled")).toBe(false)); + fireEvent.click(toggle); + + await waitFor(() => + expect(screen.getByRole("alert").textContent).toBe("ADE could not save this setting."), + ); + expect(screen.getByRole("switch").getAttribute("aria-checked")).toBe("true"); + }); +}); diff --git a/apps/desktop/src/renderer/components/settings/settingsSectionUi.tsx b/apps/desktop/src/renderer/components/settings/settingsSectionUi.tsx index 83888c9b8..fb3df58c0 100644 --- a/apps/desktop/src/renderer/components/settings/settingsSectionUi.tsx +++ b/apps/desktop/src/renderer/components/settings/settingsSectionUi.tsx @@ -1,6 +1,6 @@ -import React from "react"; +import React, { useEffect, useId, useState } from "react"; import type { Icon as PhosphorIcon, IconWeight } from "@phosphor-icons/react"; -import { COLORS, SANS_FONT } from "../lanes/laneDesignTokens"; +import { COLORS, SANS_FONT, cardStyle } from "../lanes/laneDesignTokens"; export const settingsSectionTitleStyle: React.CSSProperties = { fontSize: 13, @@ -78,6 +78,135 @@ export function SettingsSectionShell({ ); } +/** + * One privacy consent control: a labelled switch, what it shares in plain + * words, a live footnote, and an error line that never disappears silently. + * + * ADE has two of these — anonymous analytics and automatic diagnostics — and + * they were the same eighty lines twice. They are also the two screens where a + * copy-paste divergence matters most: these read and write the real persisted + * value rather than assuming, because a consent toggle that renders optimism + * is a consent toggle that can show "off" for something that is on. + * + * `read`/`write` may be absent, which is a build whose preload predates the + * setting: the switch renders in its default position and stays disabled + * rather than pretending to work. + */ +export function ConsentToggleSection({ + id, + title, + description, + icon, + brandColor, + label, + body, + footnote, + read, + write, + readErrorMessage, + writeErrorMessage, +}: { + id: string; + title: string; + description: React.ReactNode; + icon: PhosphorIcon; + brandColor: string; + /** The switch's own label: what the user is agreeing to, in one line. */ + label: string; + /** What leaves the computer, and what never does. */ + body: React.ReactNode; + /** The quieter line beneath, given the live status (`null` until it loads). */ + footnote: (status: TStatus | null) => React.ReactNode; + read: (() => Promise) | undefined; + write: ((enabled: boolean) => Promise) | undefined; + readErrorMessage: string; + writeErrorMessage: string; +}) { + const toggleId = useId(); + const [status, setStatus] = useState(null); + const [saving, setSaving] = useState(false); + const [error, setError] = useState(null); + + useEffect(() => { + let cancelled = false; + if (!read) return; + void read() + .then((next) => { + if (!cancelled) setStatus(next); + }) + .catch(() => { + if (!cancelled) setError(readErrorMessage); + }); + return () => { + cancelled = true; + }; + // `read`/`write` are stable per section; re-running on identity alone would + // refetch on every parent render. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + const setEnabled = async (enabled: boolean) => { + if (saving || !write) return; + setSaving(true); + setError(null); + try { + setStatus(await write(enabled)); + } catch { + setError(writeErrorMessage); + } finally { + setSaving(false); + } + }; + + return ( + +
+
+
+ +

+ {body} +

+

+ {footnote(status)} +

+ {error ? ( +

+ {error} +

+ ) : null} +
+ void setEnabled(enabled)} + /> +
+
+ + ); +} + export function SettingsToggle({ checked, onChange, diff --git a/apps/desktop/src/shared/diagnosticsUpload.ts b/apps/desktop/src/shared/diagnosticsUpload.ts index 48f8dedf0..a727fc971 100644 --- a/apps/desktop/src/shared/diagnosticsUpload.ts +++ b/apps/desktop/src/shared/diagnosticsUpload.ts @@ -107,20 +107,48 @@ export type DiagnosticUploadRequest = { token?: string | null; /** Already resolved by the caller; see `resolveDiagnosticsUploadBaseUrl`. */ baseUrl: string; + /** + * True when ADE decided to send this, rather than a person pressing a button. + * + * The route stores it so the two populations stay separable: an automatic + * report is one nobody chose to file, and reading them as if a user had + * would badly misread which failures people actually care about. + */ + auto?: boolean; + /** + * The failure that triggered an automatic send — a short code such as + * `brain_crash_looping`, never text. Shape is `/^[a-z][a-z0-9_-]{0,47}$/`; + * anything else is dropped here rather than sent for the Worker to refuse. + */ + failureCode?: string | null; fetchImpl?: typeof fetch; timeoutMs?: number; }; +/** + * Mirror of the account directory route's `failureCode` shape. + * + * Exported because the auto-send ledger has to reject a code BEFORE it spends + * one of the day's three sends on a request this uploader would then strip. The + * Worker keeps its own copy (`apps/account-directory/src/diagnostics.ts`) on + * purpose — it is a separate deploy unit and must not import from the app — but + * inside this process there is exactly one. + */ +export const FAILURE_CODE_PATTERN = /^[a-z][a-z0-9_-]{0,47}$/; + export async function uploadDiagnosticReport( request: DiagnosticUploadRequest, ): Promise { const report = request.report; if (!report.trim()) return { ok: false, reason: "rejected" }; + const failureCode = request.failureCode?.trim() ?? ""; const body = JSON.stringify({ report, ...(request.installId ? { installId: request.installId } : {}), ...(request.appVersion ? { appVersion: request.appVersion } : {}), + ...(request.auto ? { auto: true } : {}), + ...(FAILURE_CODE_PATTERN.test(failureCode) ? { failureCode } : {}), }); // Checked here as well as on the Worker so an oversized report fails without // spending one of the user's few daily uploads on a doomed request. The exact diff --git a/apps/desktop/src/shared/ipc.ts b/apps/desktop/src/shared/ipc.ts index 948465392..8fe118bfc 100644 --- a/apps/desktop/src/shared/ipc.ts +++ b/apps/desktop/src/shared/ipc.ts @@ -81,6 +81,30 @@ export const IPC = { recoveryRepairStep: "ade.recovery.repairStep", /** Assemble, save, copy to the clipboard, and open a prefilled GitHub issue. */ diagnosticsOpenIssue: "ade.diagnostics.openIssue", + /** Renderer-detected failure asking main to consider one automatic send. */ + diagnosticsAutoReport: "ade.diagnostics.autoReport", + /** Read the "share diagnostics automatically" setting. */ + diagnosticsGetSharing: "ade.diagnostics.getSharing", + /** Flip that setting; also what the toast's "Turn off" action calls. */ + diagnosticsSetSharing: "ade.diagnostics.setSharing", + /** Reveal a saved auto-report in Finder/Explorer. Reports directory only. */ + diagnosticsRevealReport: "ade.diagnostics.revealReport", + /** Main → renderer: one automatic report was sent, so a toast can say so. */ + diagnosticsAutoSent: "ade.diagnostics.autoSent", + /** + * List the sends nobody has been shown yet — the brain's, and any made while + * no window was listening — and deliver them. Called by a renderer as it + * subscribes, so a headless auto-send still gets its toast without anything + * polling for one. Reading does not retire anything; the ack below does. + */ + diagnosticsFlushAutoSent: "ade.diagnostics.flushAutoSent", + /** + * Renderer → main: these references are now on screen, stop offering them. + * Sent AFTER the toast is rendered, by whichever path delivered it, which is + * what keeps a notice from being toasted again on the next launch and what + * keeps "pending" meaning shown rather than merely handed over. + */ + diagnosticsAckAutoSent: "ade.diagnostics.ackAutoSent", projectForgetRecent: "ade.project.forgetRecent", projectReorderRecent: "ade.project.reorderRecent", projectSetRecentPinned: "ade.project.setRecentPinned", diff --git a/apps/desktop/src/shared/types/diagnostics.ts b/apps/desktop/src/shared/types/diagnostics.ts index 39fbcd578..f83817ed3 100644 --- a/apps/desktop/src/shared/types/diagnostics.ts +++ b/apps/desktop/src/shared/types/diagnostics.ts @@ -27,6 +27,29 @@ export type DiagnosticReportRequestPayload = { projectRoot?: string | null; }; +/** + * "Share diagnostics with ADE when something breaks", as the renderer sees it. + * + * Default on. The same flag gates the brain's own automatic sends, because both + * read one file — see `main/services/diagnostics/autoDiagnosticsStore.ts`. + */ +export type DiagnosticsSharingStatus = { + enabled: boolean; + /** Automatic reports already sent in the last 24 hours. */ + sendsInWindow: number; + /** The daily ceiling those are counted against. */ + limit: number; +}; + +/** Main → renderer, once per automatic send. Codes and handles only. */ +export type DiagnosticsAutoSentPayload = { + failureCode: string; + /** Saved report path; empty when the local copy could not be written. */ + reportPath: string; + /** Short upload handle to read back to support. */ + reference: string; +}; + export type DiagnosticReportPayload = { /** The full redacted report, ready to paste. */ report: string; diff --git a/docs/features/onboarding-and-settings/README.md b/docs/features/onboarding-and-settings/README.md index c711ac1bd..fbab89434 100644 --- a/docs/features/onboarding-and-settings/README.md +++ b/docs/features/onboarding-and-settings/README.md @@ -385,6 +385,17 @@ Renderer — settings: never exposes or accepts credentials. Native iOS is independently default-on without an in-app preference; hosted web keeps its own affirmative browser choice. See [logging and product analytics](../../logging.md). +- `apps/desktop/src/renderer/components/settings/DiagnosticsSharingSection.tsx` + — the second Privacy consent control (`general.diagnostics-sharing`, anchored + `#diagnostics-sharing`): **"Share diagnostics with ADE when something + breaks"**, default **on**, machine-scoped. It is the off switch for the + automatic redacted report ADE sends by itself when it hits a failure it has + already classified; the same switch appears on the toast that announces each + send. `web: "hidden"` in the manifest, because the consent lives in + `~/.ade/secrets/diagnostics-autosend.json` and a browser has no such file. + Analytics consent and diagnostics consent are deliberately separate flags, so + turning one off never silently turns off the other. See + [storage and recovery → Auto-send](../storage-and-recovery/README.md#auto-send). - `apps/desktop/src/renderer/components/settings/GitHubIntegrationSection.tsx` and `GitHubSection.tsx` — ADE GitHub App / environment / GitHub CLI / PAT auth, credential-specific permission diagnostics, structured validation @@ -514,7 +525,14 @@ Renderer — settings: CLI availability is app basics, not an integration. The `EnvironmentSection.tsx` wrapper that used to pair them is gone. - `apps/desktop/src/renderer/components/settings/settingsSectionUi.tsx` - — shared section headers (`SettingsSectionShell`) and toggle styling. + — shared section headers (`SettingsSectionShell`), toggle styling, and + `ConsentToggleSection`: the one privacy consent control — labelled switch, + plain-words body, live footnote, persistent error line — that both + `ProductAnalyticsSection` and `DiagnosticsSharingSection` render. It always + reads the real persisted value rather than rendering optimism, and when the + preload bridge predates the setting it renders disabled instead of pretending + to work; a consent switch showing "off" for something that is on is the one + failure mode this component exists to prevent. - `apps/desktop/src/renderer/components/settings/AppearanceSection.tsx` — theme, chat appearance, and terminal text. Renders `ChatAppearancePreview` and writes local user preferences through `appStore` (font size, @@ -1266,7 +1284,7 @@ changing rather than which service backs it: | Tab | Section file | What lives here | |---|---|---| -| General | `ProjectSection.tsx`, `AdeCliSection.tsx`, `AutoUpdatesSection.tsx`, `KeepAwakeSection.tsx`, `ProductAnalyticsSection.tsx`, `AboutSection.tsx` | The top ADE card shows running/installed/downloaded versions, the runtime service, and update controls; below it are project health, the `ade` command line (`#ade-cli`), **Sleep** (`#keep-awake`, hidden on hosted web — a browser holds no power lock), and privacy. Legacy `?tab=workspace`, `?tab=project`, `?tab=context`, `?tab=onboarding`, `?tab=help`, and `?tab=tours` land here. | +| General | `ProjectSection.tsx`, `AdeCliSection.tsx`, `AutoUpdatesSection.tsx`, `KeepAwakeSection.tsx`, `ProductAnalyticsSection.tsx`, `DiagnosticsSharingSection.tsx`, `AboutSection.tsx` | The top ADE card shows running/installed/downloaded versions, the runtime service, and update controls; below it are project health, the `ade` command line (`#ade-cli`), **Sleep** (`#keep-awake`, hidden on hosted web — a browser holds no power lock), and the two Privacy consents — anonymous analytics and diagnostics sharing (`#diagnostics-sharing`, hidden on hosted web). Legacy `?tab=workspace`, `?tab=project`, `?tab=context`, `?tab=onboarding`, `?tab=help`, and `?tab=tours` land here. | | Appearance | `AppearanceSection.tsx`, `LaunchPromptSection.tsx` (renders `ChatAppearancePreview`) | Theme, chat typography and density, chat surface (tint, corners), chat details (copy-button position, message minimap, prompt-stash bookmark, launch-prompt clipboard, live preview), and terminal text. Rebuilt on the primitives — the old version used `font-mono` for every prose line and four different control idioms. Persisted to `localStorage` under `ade.userPreferences.v1`. | | Agents & Models | `ProvidersSection.tsx`, `OAuthConnectModal.tsx`, `AiFeaturesSection.tsx`, `BudgetCapEditor.tsx`, `DictationSection.tsx` | Provider connections, model routing, background helpers, spend cap, and voice input — merged because provider auth and per-task model routing are one mental model. **Coding Agents** cards (Claude Code, Codex CLI, Cursor, Droid, Pi — Pi's card also carries in-app provider sign-in) and **OpenCode — Universal Model Access**. Background helpers cover summaries, PR descriptions, commit messages, auto-naming, and scheduled-work recovery. Legacy `?tab=ai`, `?tab=providers`, `?tab=background-jobs`, and `?tab=automations` land here. | | Lanes | `LaneBehaviorSection.tsx`, `LaneTemplatesSection.tsx`, `PrChatTranscriptsSection.tsx` | How lanes start (`new lane base`), stay current (`auto-rebase`), and tell you they fell behind (`rebase suggestions` off/badge/banner + min-behind threshold), plus lane init recipes and PR transcript gists. Legacy `?tab=lane-templates` lands here. | diff --git a/docs/features/storage-and-recovery/README.md b/docs/features/storage-and-recovery/README.md index b72978c1e..904a171bf 100644 --- a/docs/features/storage-and-recovery/README.md +++ b/docs/features/storage-and-recovery/README.md @@ -17,7 +17,7 @@ | `apps/ade-cli/src/services/runtime/brainFreshnessMonitor.ts` | The running brain stats its own CLI entrypoint every 5 min (`ADE_BRAIN_FRESHNESS_INTERVAL_MS`), hashes only after the stat changes, and — when the on-disk hash no longer matches the baked runtime hash — waits for the brain to go idle (bounded) before triggering the brain-update service restart so an in-place upgrade takes effect without interrupting active work. Disable with `ADE_DISABLE_BRAIN_FRESHNESS=1`. | | `apps/ade-cli/src/services/runtime/runtimeBuildIdentity.ts` | `computeRuntimeBuildHash` / `computeRuntimeBuildHashAsync` — the SHA-256 of the CLI entrypoint used as the brain build identity by the freshness monitor and the desktop compatibility handshake. | | `apps/ade-cli/src/services/runtime/brainLogger.ts` | The machine-brain logger: reuses the desktop `createFileLogger` to write `~/.ade/runtime/brain.jsonl` (10 MiB `.1` rotation) and additionally mirrors timestamped `warn`/`error` lines to stderr so launchd captures them. | -| `apps/ade-cli/src/commands/doctor.ts` | `ade doctor [--online]` — connects to the brain over the local socket and prints one `ok`/`warn`/`fail` row per subsystem (App version, Brain, Wedge history, Sync port, Publish health, Relay, Account); exits non-zero on any `fail`. `evaluateDoctorRows` is pure and dependency-injected so the desktop connection-doctor card and the CLI share one verdict. | +| `apps/ade-cli/src/commands/doctor.ts` | `ade doctor [--online]` — connects to the brain over the local socket and prints one `ok`/`warn`/`fail` row per subsystem (App version, Brain, Wedge history, Sync port, Publish health, Relay, Account, Diagnostics sharing); exits non-zero on any `fail`. The **Diagnostics sharing** row reads the shared auto-send ledger through `readAutoDiagnosticsState` — never a second parser — and is always `ok`: consent is a preference, not a fault, so it reports `on · N of 3 automatic reports sent today` or `off · no automatic reports are sent` and never colours a healthy machine. `evaluateDoctorRows` is pure and dependency-injected — every row's inputs are read at the edge (`runDoctorCommand`) and handed in — so the verdict is testable without a machine and a second surface can reuse it. Today the CLI is its only caller: the desktop's **Connection doctor** card (`remoteTargets/ConnectionDoctorPanel.tsx` → `remoteRuntime.runDoctor`) is a different check about reaching a *remote* machine, not this one. | | `apps/desktop/src/shared/adeRuntimeProtocol.ts` | Shared runtime-protocol contract: `RUNTIME_COMPAT_LEVEL` + `isRuntimeProtocolCompatible` (the integer compatibility-window check), and the tolerant parsers `parseRuntimePublishHealth` / `parseRuntimeLastWedge` that decode `runtimeInfo.publishHealth` and `runtimeInfo.lastWedge` for the connection pool, the doctor, and the desktop status surfaces. | | `apps/desktop/src/main/services/runtime/projectRecoveryService.ts` | Brain-independent diagnosis and ordered repair: space, ownership, database validation, migration recovery, service restart, endpoint/project verification, and chat reconciliation. Also owns `restartBrain()` — the machine-scoped restart behind the Connections **Repair** button — which shares one `restartServiceAndWait()` sequence (install → wait ≤90 s for the endpoint → `ping`) with `repair()`'s restart_service/verify_endpoint steps. The two are mutually exclusive: `restartBrain()` rejects while a `repair()` is in flight, because repair stops the service and then does exclusive database work that a reinstall would put a second writer on top of. A forced restart also treats a *skipped* install as a failure ("A newer ADE runtime is already running — quit and reopen ADE instead."), where `repair()` tolerates one, since a protocol-compatible brain that is already running satisfies its step. `main.ts` constructs exactly one of these and shares it with `registerIpc`, so the mutual exclusion actually holds — the post-update transaction's `restart` step (see [desktop auto-update](../onboarding-and-settings/desktop-auto-update.md#applying-an-update-is-one-transaction)) binds to the same instance rather than a second one that could run alongside a repair. | | `apps/desktop/src/main/services/storage/diskPressure.ts` | Samples all ADE storage roots, classifies pressure with recovery hysteresis, and gates write-producing operation classes via `canPerform(kind)`. Exports the `DiskPressureMonitor` type and refusal-message copy. | @@ -37,10 +37,16 @@ | `apps/ade-cli/src/services/diagnostics/diagnosticReport.ts` | The pure report builder, the redactor (`redactDiagnosticText`), and `buildDiagnosticIssueUrl`. No I/O, so both the desktop and the CLI produce byte-identical documents from the same sources. | | `apps/ade-cli/src/services/diagnostics/diagnosticSources.ts` | `collectMachineDiagnosticSources` — the machine-level logs, layout, disk figures and redaction context both surfaces read, so a log added for one appears in both. | | `apps/ade-cli/src/commands/reportIssue.ts` | `ade report-issue [--open] [--send]`, the headless equivalent. `--send` posts the same redacted report to ADE (Clerk token when the machine is signed in, anonymous otherwise) and prints a short reference id. Local files only: it never starts or contacts the brain, so it still works where ADE will not come up and on hosts with no error screen to press. | +| `apps/desktop/src/main/services/diagnostics/autoDiagnosticsStore.ts` | The consent flag **and** the spend ledger for automatic uploads, in one file (`/secrets/diagnostics-autosend.json`) that both senders open — the desktop main process and the brain — because "three a day from this computer" is a property of the install, not of a process, and two private ledgers would quietly mean six. Deliberately dependency-free (`node:fs`, no Electron, no logger) for exactly that reason. Owns `AUTO_DIAGNOSTICS_WINDOW_MS` (24 h), `MAX_AUTO_DIAGNOSTICS_PER_CODE` (1), `MAX_AUTO_DIAGNOSTICS_PER_WINDOW` (3), `normalizeAutoDiagnosticsFailureCode` (coerced to the Worker's `FAILURE_CODE_PATTERN`, re-exported rather than rewritten), the mkdir lock — whose `isLockContention` names the Windows delete-pending `EPERM`/`EACCES`/`EBUSY` window as well as `EEXIST` — and the pending-notice queue the toast acknowledgement retires. Consent defaults **on**; an unreadable or locked ledger fails closed. | +| `apps/desktop/src/main/services/diagnostics/autoDiagnosticsSend.ts` | `runAutoDiagnosticsSend` — the policy every automatic send obeys, written once. The two senders differ in exactly three things (what they build, how they upload, which analytics surface they report as) and bring those as structural seams; consent, the pre-request reservation, the local copy, silence on failure, the pending flag, the log lines and the analytics dedupe key live here. Also owns the `AutoDiagnosticsOutcome` vocabulary (`completed`, `skipped_disabled`, `skipped_budget`, `skipped_ineligible`, `failed`) and `AUTO_DIAGNOSTICS_ANALYTICS_DEDUPE_MS` (1 h). | +| `apps/desktop/src/main/services/diagnostics/autoDiagnosticsService.ts` | The desktop sender: what is specific to this process — how a report gets built (no `diagnoseProject`, since a diagnosis is itself a trigger), that it uploads anonymously, the `onSent` fast path for an open window, and the getter/setter the Settings toggle reads and writes. | +| `apps/ade-cli/src/services/diagnostics/autoDiagnosticsSender.ts` | The brain's sender, for the failures the desktop never sees — a headless machine whose pairing recovery gave up, a publisher failing for minutes with nobody at the console. It reads the machine credential store, so its reports land attributed rather than anonymous. It has no window, so successful sends stay pending in the shared ledger until a renderer subscribes and acknowledges the toast. | | `apps/ade-cli/src/lib/externalLinks.ts` | `normalizeExternalUrl` / `openExternalUrl` for the CLI: allows only `http(s)` and `mailto:`, opens through the platform helper (`open` / `rundll32` via the trusted-tool resolver / `xdg-open`), and falls back to Electron's `shell.openExternal` only when actually running inside Electron — a static `electron` import crashes headless startup. | | `apps/desktop/src/shared/diagnosticsUpload.ts` | The one **Send to ADE** client, shared by the renderer button and the CLI: `uploadDiagnosticReport`, the `DiagnosticUploadFailure` vocabulary and its one-sentence copy, `resolveDiagnosticsUploadBaseUrl`, and `diagnosticReference` (the first 8 characters of the returned id — a full uuid is unreadable over a phone call). It lives in `shared/` because that is the only tree the renderer, the main process and the CLI can all import (Vite refuses to serve files outside `apps/desktop`), and it is deliberately free of Node built-ins and `import.meta` so the identical module loads in all three. It posts the report's exact bytes and transforms nothing: the thing that is sent has to be the thing that was shown. | | `apps/desktop/src/shared/types/diagnostics.ts` | The `DiagnosticSurface` / request / payload contract shared by main, preload and renderer. | | `apps/desktop/src/renderer/components/app/ReportIssueButton.tsx` | The button itself, on every error surface. One press assembles, saves, copies, and opens the issue; it reports what actually happened rather than claiming success. | +| `apps/desktop/src/renderer/components/settings/DiagnosticsSharingSection.tsx` | The off switch, in Settings → General → Privacy (`general.diagnostics-sharing`, anchored `#diagnostics-sharing`, `web: "hidden"` because the consent lives in a file a browser does not have). It renders `ConsentToggleSection` from `settings/settingsSectionUi.tsx` — the shared consent control it and `ProductAnalyticsSection` both use, which reads the real persisted value instead of rendering optimism and renders disabled when the preload bridge predates the setting. | +| `apps/desktop/src/renderer/components/app/toast/useAutoDiagnosticsToast.ts` | The renderer half of the delivery contract: subscribes, asks for the outstanding notices (`IPC.diagnosticsFlushAutoSent`), raises the *"A diagnostic report was sent to ADE"* toast with **View** / **Turn off**, and only then acknowledges it (`IPC.diagnosticsAckAutoSent`). Mounted from `AppShell.tsx`. | | `apps/desktop/src/renderer/components/app/errorSurfaceKit.tsx` | Shared parts for the full-screen error surfaces — `ErrorSurfaceCard`, `WhatToDo`, `TechnicalDetailsFold`, `ERROR_PRIMARY_BUTTON` — so the recovery screen, the renderer/page boundaries and the CTO wake failure keep the raw text behind a fold and the plain-language account on top. | | `apps/desktop/src/renderer/components/chat/ChatContinuityRecoveryCard.tsx` | In-transcript choices to retry the original thread, rebuild from ADE history, or start a separate chat. `AgentChatMessageList` renders it in place of a plain notice chip when a `system_notice` event's `detail.kind` is `"continuity_recovery"`. | | `apps/desktop/src/renderer/components/settings/StorageSection.tsx` | Storage dashboard: plain-language lane cleanup rules, last/next safety-scan status, and a review table for archived lanes, orphaned worktrees, DerivedData, and build output with ownership, age, blocked reasons, and reclaim estimates. Archive & Reclaim has a typed confirmation and explains exactly what stays and what restore recreates. The page also keeps the category totals, Health & diagnostics strip, project-database breakdown, cleanup preview, recent-cleanups journal, and manual history compression. | @@ -604,6 +610,11 @@ rides the clipboard. | IPC | `IPC.diagnosticsOpenIssue` | | Saved report | `/diagnostic-reports/-.md`, mode `0600` | | Headless equivalent | `ade report-issue [--open] [--send]` | +| Headless state check | `ade doctor` → the **Diagnostics sharing** row (consent + today's spend) | +| Settings toggle | `general.diagnostics-sharing` (General → Privacy, `#diagnostics-sharing`, default **on**, hidden on hosted web) | +| Automatic sending (desktop) | `apps/desktop/src/main/services/diagnostics/autoDiagnosticsService.ts` | +| Automatic sending (brain) | `apps/ade-cli/src/services/diagnostics/autoDiagnosticsSender.ts` | +| Consent flag + shared daily budget | `apps/desktop/src/main/services/diagnostics/autoDiagnosticsStore.ts` → `~/.ade/secrets/diagnostics-autosend.json` | | Upload (opt-in) | `POST /diagnostics/upload` on the account directory Worker (`apps/account-directory/src/diagnostics.ts`); one client for both senders — the renderer button and the CLI — in `apps/desktop/src/shared/diagnosticsUpload.ts` | `ade report-issue` and the desktop button read the same machine sources through @@ -689,9 +700,85 @@ origin the way the brain does also means a self-hosted machine's report and its token are not silently redirected to ADE's directory. The Worker treats the body as opaque: it never parses, indexes or echoes a report, which is what lets it accept anonymous uploads at all, and it bounds one identity (Clerk user, else a -hash of the caller address) to five uploads a UTC day. The button's disclosure -text says so — nothing leaves the computer unless the user posts the issue or -chooses **Send to ADE**. +hash of the caller address) to five uploads a UTC day. + +#### Auto-send + +Nobody presses the button. A person looking at an error screen has to notice the +control, decide the failure is worth reporting, and follow through — so the +reports that would explain the worst failures are exactly the ones that never +arrive. When ADE hits a failure it has **already classified**, it sends the same +finished report by itself, with `auto: true` and the failure code alongside it so +the two populations stay separable on the server. + +**Triggers.** One call each, at the point the failure is already known: + +| Trigger | Where | `failureCode` | +| --- | --- | --- | +| Recovery diagnosis reached a terminal state | `main/services/runtime/projectRecoveryService.ts` (`diagnose`, via `onTerminalDiagnosis`) | the `AdeRecoveryErrorCode` — `disk_full`, `brain_crash_looping`, … | +| Renderer crash | `renderer/components/app/RendererErrorBoundary.tsx` (`componentDidCatch`, via `IPC.diagnosticsAutoReport`) | `renderer_crash` | +| Post-update transaction failed | `main/main.ts`, beside `autoUpdate.transaction_failed` | `update_` | +| Pairing auto-recovery gave up | `ade-cli/.../machinePairingAutoRecovery.ts` (`onGaveUp`) | the refusal code, or `snapshot_failed` | +| Account publisher failing > 5 min | `ade-cli/.../accountMachinePublisherService.ts` (`onSustainedFailure`) | the health state, e.g. `snapshot_failed` | + +`healthy` and `brain_starting` are deliberately not terminal: a booting brain +fixes itself in seconds, and reporting it would spend the day's budget on a +non-event. The auto builder also passes **no** `diagnoseProject`, because the +diagnosis is itself a trigger and asking for a fresh one while building the +report about it would re-enter the path that asked. + +**Budgets.** At most **one report per failure code per 24 hours** and **three in +total per 24 hours, per install** — one rolling ledger in +`~/.ade/secrets/diagnostics-autosend.json`, shared by the desktop and the brain, +so it is three a day for the computer rather than three per process. The +reservation is taken **before** the request: a budget that only counted +successes would let a machine whose uploads all fail retry the same failure +every time it recurs, which is precisely the loop this is not allowed to become. +An unreadable or locked ledger fails closed. The client ceiling sits well inside +the server's five-per-day-per-identity limit and its fleet-wide daily cap +(`DEFAULT_DIAGNOSTICS_DAILY_GLOBAL_LIMIT`, see +`apps/account-directory/README.md` § *Diagnostic report uploads*), so the cost +ceiling is enforced twice and neither side depends on the other. + +**Failure is silence.** Any upload failure — `429` from the per-user or the +fleet budget, `503`, a network error — is logged locally and nothing else. No +toast, no error, no retry. The person is already looking at something broken; +telling them the thing they did not ask for also did not work is not help. + +**Toast and toggle.** Every successful send raises one toast — *"A diagnostic +report was sent to ADE"* — with **View** and **Turn off**. Settings → General → +Privacy carries the same switch, *"Share diagnostics with ADE when something +breaks"*, default **on**. + +**View** reveals the saved `.md` through a handler scoped to the two +directories reports are written to — the desktop's +`userData/diagnostic-reports` and the brain's `/diagnostic-reports` — +rather than by widening `appRevealPath`'s allowlist. Both, because a headless +send is exactly the one the user was not present for, so a brain report is the +one they are most likely to open. + +Delivery is the ledger's job, not the window's, and only the window can close +it. `webContents.send` does not throw when the receiving renderer has crashed or +has not mounted its toast host, so a successful send is ALWAYS recorded pending, +and *pending* means "no renderer has said it showed this". A renderer asks for +the outstanding ones as it subscribes (`IPC.diagnosticsFlushAutoSent` — +event-driven, nothing polls); that read retires nothing, because the window can +still vanish between being handed a notice and rendering it. What retires one is +the renderer acknowledging it after the toast exists +(`IPC.diagnosticsAckAutoSent`). So a toast is never shown twice across restarts, +and a window that dies mid-render repeats one toast rather than swallowing it. +The immediate send to open windows is a fast path on top of that; a window that +gets both keys the toast on `diagnostics-auto-sent-` and sees one, and +acknowledges it either way. The brain has no window at all and waits for the +same acknowledgement. The desktop and the brain can both report one +incident; they carry different codes and surfaces, so both are individually +useful, and the shared three-a-day ceiling bounds the duplication. Nothing else +coordinates them, deliberately. + +The button's disclosure text still holds for the manual path — nothing leaves +the computer unless the user posts the issue or chooses **Send to ADE** — and +the automatic path adds one more way, which is announced every time it happens +and switched off in one click. ## Gotchas diff --git a/docs/features/sync-and-multi-device/README.md b/docs/features/sync-and-multi-device/README.md index da06b716b..71fa752fc 100644 --- a/docs/features/sync-and-multi-device/README.md +++ b/docs/features/sync-and-multi-device/README.md @@ -945,8 +945,21 @@ Runtime support files outside `services/sync/`: directory's exact-origin CORS rule and 404-on-unknown-`OPTIONS` fit neither an unauthenticated Electron renderer nor a CLI. Authentication is optional but never silently downgraded, the body is capped at 512 KB by both - `content-length` and a counted stream, and the quota is five a day per signed- - in user or per `cf-connecting-ip`. See + `content-length` and a counted stream, and there are **two** quotas, because + they bound different things. The per-caller one is five a day per signed-in + user or per `cf-connecting-ip`. The fleet one — + `DIAGNOSTICS_DAILY_GLOBAL_LIMIT`, default 400 stored reports per UTC day + across every caller, claimed from `diagnostics_upload_days` (migration + `0009`) by the same single upsert idiom `device_approval_rate_limits` uses, + refunded when the `put` then fails, and failing **closed** to `503` when D1 is + unavailable — exists because clients now send reports *automatically* on + failure, so one bug firing across the install base multiplies "five each" by + the install base and no per-caller limit can see that coming. The two `429`s + carry **distinct** bodies (`rate limited` versus + `daily diagnostics budget exhausted`) on purpose: an auto-sender that read a + fleet-wide stop as its own quota would retry forever. Uploads also carry + optional `auto` / `failureCode` metadata so automatic and hand-pressed + reports stay separable in the bucket. See [storage and recovery → Diagnostic reports](../storage-and-recovery/README.md#diagnostic-reports-report-issue) for the client half, and `apps/account-directory/README.md` for the full contract and the R2 bucket + lifecycle setup the deploy does not do for you. diff --git a/docs/logging.md b/docs/logging.md index 2b97110f1..166ba34ff 100644 --- a/docs/logging.md +++ b/docs/logging.md @@ -298,16 +298,50 @@ installation per UTC day, inside the existing `ade_feature_used` 140-per-day / 30-per-minute limits and the shared 200-event ceiling; no ceiling was raised. The dashboard spec is deliberately untouched: no card asks this question yet. +When ADE hits a failure it already classified, it sends that same redacted +report by itself, and that decision records the same `ade_feature_used` event at +the owner boundary — the auto-diagnostics service, where the outcome is known — +with `feature: "connections"`, `action: "auto_sent"`, and one of three coarse +outcomes: `completed` when the upload succeeded, `skipped_budget` when the +client's own daily ceiling refused it, `failed` when it was attempted and did +not land (including a `429` from either the per-user or the fleet budget). The +product question is only whether the thing that fires without anyone asking +works and whether its guardrail holds, so nothing else crosses: not the failure +code that triggered it, not the surface, not the upload reference, not the saved +report path, and not whether the user then turned the feature off — that is a +setting, not an event. Two of the five outcomes `runAutoDiagnosticsSend` can +return deliberately emit nothing. `skipped_disabled`: an installation that has +withdrawn consent emits nothing at all, so counting its non-sends would be the +one measurement it declined. `skipped_ineligible` — an unusable failure code, or +a send already in flight — because nothing was built, spent or refused, so there +is no outcome to report; it is a caller bug or a race, and it belongs in the +local log, which is where it goes. A per-outcome one-hour +deduplication key bounds the worst case to 24 accepted events per outcome — 72 +across all three — per installation per UTC day, and the client budget of three +sends a day makes the real number far smaller; this sits inside the existing +`ade_feature_used` 140-per-day / 30-per-minute limits and the shared 200-event +ceiling, and no ceiling was raised. The brain emits the same event for its own +automatic sends through the same shared service — under `surface: "api"`, since +nobody was at the keyboard — and shares the persisted deduplication state, so an +installation's counts are one number rather than two. The dashboard spec is +deliberately untouched: no card asks this question yet. + The diagnostic report itself is a **local** artifact and is not analytics. It deliberately includes the PostHog `distinct_id` for this installation (`productAnalyticsService.getDistinctId()` — the identified account hash when signed in, otherwise the random anonymous install token) so a report someone files by hand can be matched to the events the installation already sent. -Nothing flows the other way: the report is written to disk and copied to the clipboard, and only -the person filing it decides where it goes. Its body is redacted before it is +Nothing flows the other way: no part of a report reaches PostHog, on either +path. A report the user files is written to disk and copied to the clipboard and +only they decide where it goes; a report ADE sends by itself goes to the +diagnostics upload route and nowhere else, and the analytics boundary learns +only that a send happened and how it ended. Its body is redacted before it is written (home directory, project paths, usernames, hostnames and tailnet names, -emails, credentials and routable IP addresses), and the GitHub issue title and -stub body are redacted with the same context. +emails, credentials and routable IP addresses) — the same bytes on both paths, +because redaction happens once in the builder — and the GitHub issue title and +stub body are redacted with the same context. Automatic sending is a separate +consent from analytics: it has its own Settings toggle (default on) and its own +persisted flag, so turning one off does not silently turn off the other. Clicking "Reconnect this computer" on the Account pane's removed-machine banner records the existing `ade_feature_used` event at the IPC owner boundary (the