diff --git a/.changeset/uninstall-feedback-form.md b/.changeset/uninstall-feedback-form.md new file mode 100644 index 000000000..ccc1d7ac0 --- /dev/null +++ b/.changeset/uninstall-feedback-form.md @@ -0,0 +1,7 @@ +--- +"@inkeep/open-knowledge": minor +--- + +Ask why on uninstall. After a successful uninstall, in the macOS desktop flow and `ok uninstall`, an optional, skippable feedback screen asks one reason you're leaving, plus an optional note and an optional email for follow-up. It's shown only once removal succeeds, never blocks or cancels the uninstall (closing the window or interrupting the CLI just proceeds), and submissions reuse the existing feedback intake, tagged so churn feedback is filterable on its own. + +The desktop "OpenKnowledge files were removed" screen is also redesigned as a scannable checklist (kept your content / removed OpenKnowledge files / move the app to the Trash) so the one remaining action stands out, with the cleanup log as a "reveal in Finder" link instead of a raw path. diff --git a/docs/content/reference/cli.mdx b/docs/content/reference/cli.mdx index 931259395..4e27f0142 100644 --- a/docs/content/reference/cli.mdx +++ b/docs/content/reference/cli.mdx @@ -134,6 +134,6 @@ The rest of the CLI exists mostly for tooling and automation. Your AI editor spa `ok uninstall` removes OpenKnowledge from the whole machine — running servers, credentials, the PATH shim, its own editor MCP entries, skill bundles, app data, and `~/.ok`. Your markdown content stays, and skills you authored under `~/.ok/skills` are kept unless you pass `--purge-content`. It never deletes the CLI binary itself; it ends by printing the removal command (`npm uninstall -g @inkeep/open-knowledge` for the install above). -Both commands print their plan and ask before acting; `--dry-run` previews without changing anything. +Both commands print their plan and ask before acting; `--dry-run` previews without changing anything. Once `ok uninstall` has finished successfully, an interactive run also asks one optional question about why you're leaving — you can skip it, and `--yes`, `--json`, and non-interactive runs never ask. Anything you do answer is [sent to us](/docs/reference/what-open-knowledge-writes#what-leaves-your-machine). -On the macOS desktop app, **App menu → Uninstall OpenKnowledge…** covers both steps from one flow: it optionally deinitializes your projects, removes the global footprint, and then guides you through dragging the app to the Trash. +On the macOS desktop app, **App menu → Uninstall OpenKnowledge…** covers both steps from one flow: it optionally deinitializes your projects, removes the global footprint, shows the same optional "why are you leaving?" screen once removal succeeds, and then guides you through dragging the app to the Trash. diff --git a/docs/content/reference/configuration.mdx b/docs/content/reference/configuration.mdx index 81eead25e..45f9c2818 100644 --- a/docs/content/reference/configuration.mdx +++ b/docs/content/reference/configuration.mdx @@ -89,6 +89,7 @@ Most users never set these — the schema settings above cover the common cases. | `OK_BRIDGE_TOLERANCE_TELEMETRY` | Set to `1` to record each bridge-tolerance-class fire as a JSONL line in `/.ok/local/tolerance-telemetry.jsonl` (opt-in, local-only, off by default; unstable diagnostic for triaging markdown-fidelity incidents). **Records doc paths in cleartext** — independent of `telemetry.localSink`'s `attributeDenylist`, which redacts span/log attributes, not this file. Deliberately outside the `.ok/local/telemetry/` subtree `ok diagnose bundle` harvests, so the unredacted paths never ship in a bug bundle. | | `OTEL_SDK_DISABLED` | OpenTelemetry OTLP push gate. Inverted sense: set to `false` to **enable** push (it is off by default). | | `OK_BUG_REPORT_INTAKE_URL` | Base URL of the bug-report intake the desktop app's **Help → Report a Bug…** upload posts to (e.g. `https://openknowledge.ai`). Must be `https:` — plain `http:` is accepted only for loopback hosts (local testing). Unset (the default), Send makes no network request — it opens a prefilled email draft to support@inkeep.com that you send yourself. | +| `OK_FEEDBACK_INTAKE_ORIGIN` | Origin the optional [uninstall feedback](/docs/reference/what-open-knowledge-writes#what-leaves-your-machine) submission posts to (default `https://openknowledge.ai`). Must be `https:` — plain `http:` is accepted only for loopback hosts (local testing). An unusable value drops the submission rather than falling back to the default. | | `OK_EMBEDDINGS_API_KEY` | Fallback embeddings provider API key for [semantic search](#semantic-search), used when none is stored on disk. Prefer `ok embeddings set-key` (writes `~/.ok/secrets.yml`) for normal use; the env var is convenient for CI / scripted runs. | ## Semantic search diff --git a/docs/content/reference/what-open-knowledge-writes.mdx b/docs/content/reference/what-open-knowledge-writes.mdx index 6e6519a81..458c63ea2 100644 --- a/docs/content/reference/what-open-knowledge-writes.mdx +++ b/docs/content/reference/what-open-knowledge-writes.mdx @@ -272,6 +272,7 @@ By default, **nothing.** Everything above is written to your own disk. The only | --- | --- | --- | | Diagnostic bundle | Only when you run `ok diagnose bundle` (you can inspect it first) | Wherever you send it | | In-app bug report | Only when you click **Send** in the desktop app's **Help → Report a Bug…** dialog — it builds a secret-redacted zip you can reveal and inspect first, and nothing is sent without that explicit click. After a crash, the same dialog adds an **off-by-default** "Include crash dump" checkbox: a crash dump is a memory snapshot that can contain document content and can't be redacted, and it is only ever attached when you check that box | By default, Send makes no network request — it opens a prefilled email draft to support@inkeep.com that you send yourself, naming the zip to attach. Only when an operator has configured a bug-report intake endpoint ([`OK_BUG_REPORT_INTAKE_URL`](/docs/reference/configuration#environment-variables)) does Send upload the zip there, falling back to the same email draft if the upload fails | +| Uninstall feedback | Only after an uninstall has already succeeded, and only if you pick a reason, type a note, or give an email address on the optional "Before you go" screen (desktop app) or prompt (`ok uninstall`). Skipping it, `--yes`, `--json`, and any non-interactive run send nothing | `openknowledge.ai/api/feedback` — the reason you picked, your note, your email address if you gave one, plus the app version and platform. It files one ticket for the team; nothing from your notes is included | | Semantic search embeddings | Only when you enable [semantic search](/docs/reference/configuration#semantic-search) **and** set a key — off by default | Your configured embeddings provider (OpenAI by default) | | GitHub sync / share | When you sync, clone, publish, or share | GitHub | | Update check | Automatically — on launch and periodically while the desktop app runs | OpenKnowledge update service (`openknowledge.ai/updates`), which redirects to GitHub; sends the app version and channel | diff --git a/packages/cli/src/commands/uninstall-feedback.test.ts b/packages/cli/src/commands/uninstall-feedback.test.ts new file mode 100644 index 000000000..714d17500 --- /dev/null +++ b/packages/cli/src/commands/uninstall-feedback.test.ts @@ -0,0 +1,428 @@ +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { PassThrough, Readable } from 'node:stream'; +import { + UNINSTALL_FEEDBACK_REASONS, + type UninstallFeedbackAnswers, + type UninstallFeedbackResult, + type UninstallFeedbackSubmission, +} from '@inkeep/open-knowledge-core'; +import { describe, expect, test, vi } from 'vitest'; +import { runUninstall } from './uninstall.ts'; +import { collectUninstallFeedbackAnswers, promptUninstallFeedback } from './uninstall-feedback.ts'; + +function write(path: string, content: string): void { + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, content); +} + +/** Records every submission so a test can assert the wire shape it was handed. */ +function recordingSubmit(result: UninstallFeedbackResult = { ok: true, reference: 'FB-1' }): { + submissions: UninstallFeedbackSubmission[]; + submit: (submission: UninstallFeedbackSubmission) => Promise; +} { + const submissions: UninstallFeedbackSubmission[] = []; + return { + submissions, + submit: async (submission) => { + submissions.push(submission); + return result; + }, + }; +} + +function interactive(answers: UninstallFeedbackAnswers) { + const collected: string[] = []; + return { + collected, + stdinIsTTY: true, + stdoutIsTTY: true, + appVersion: '1.2.3', + platform: 'darwin', + collect: async (): Promise => { + collected.push('prompted'); + return answers; + }, + }; +} + +const ARROW_UP = ''; +const ENTER = '\r'; + +/** + * Answers each prompt as it appears on the output stream, so the sequence is + * driven by what the user would actually see rather than by timing. + * + * Two details make the fake faithful. `_isStdio` is what stops inquirer's + * piped MuteStream from closing the output when the select resolves — Node's + * legacy `pipe` skips end-propagation for stdio, which is why the real stderr + * survives to serve the prompts that follow. And the reply is deferred a tick + * because writing during a render races the keypress listener that same render + * is still installing. + */ +function drive(steps: Array<{ when: string; send: string }>): { + io: { input: PassThrough; output: PassThrough }; + transcript: () => string; +} { + const input = new PassThrough(); + const output = new PassThrough(); + (output as PassThrough & { _isStdio?: boolean })._isStdio = true; + const pending = [...steps]; + let seen = ''; + output.on('data', (chunk) => { + seen += String(chunk); + const next = pending[0]; + if (next && seen.includes(next.when)) { + pending.shift(); + setImmediate(() => input.write(next.send)); + } + }); + return { io: { input, output }, transcript: () => seen }; +} + +describe('collectUninstallFeedbackAnswers', () => { + test('offers every reason plus a skip choice, all on one screen', async () => { + const driver = drive([{ when: 'mind sharing why', send: ENTER }]); + await collectUninstallFeedbackAnswers(driver.io); + const firstFrame = driver.transcript(); + for (const { label } of UNINSTALL_FEEDBACK_REASONS) { + expect(firstFrame).toContain(label); + } + expect(firstFrame).toContain('Skip'); + }); + + test('a bare Enter skips rather than picking a reason for the user', async () => { + const driver = drive([{ when: 'mind sharing why', send: ENTER }]); + expect(await collectUninstallFeedbackAnswers(driver.io)).toEqual({}); + }); + + test('a chosen reason is followed by the optional note and email', async () => { + const driver = drive([ + { when: 'mind sharing why', send: ARROW_UP + ENTER }, + { when: 'Anything else', send: 'the sync kept breaking\n' }, + { when: 'Email, if we may', send: 'me@example.com\n' }, + ]); + expect(await collectUninstallFeedbackAnswers(driver.io)).toEqual({ + reason: 'other', + note: 'the sync kept breaking', + email: 'me@example.com', + }); + }); + + test('skipping the note and email leaves a reason-only answer', async () => { + const driver = drive([ + { when: 'mind sharing why', send: ARROW_UP + ENTER }, + { when: 'Anything else', send: '\n' }, + { when: 'Email, if we may', send: '\n' }, + ]); + expect(await collectUninstallFeedbackAnswers(driver.io)).toEqual({ + reason: 'other', + note: undefined, + email: undefined, + }); + }); +}); + +describe('promptUninstallFeedback gating', () => { + const cases: Array<{ name: string; gate: Record }> = [ + { name: 'stdin is not a TTY', gate: { stdinIsTTY: false, stdoutIsTTY: true } }, + { name: 'stdout is not a TTY', gate: { stdinIsTTY: true, stdoutIsTTY: false } }, + { name: '--yes (the desktop cleanup shell-out)', gate: { yes: true } }, + { name: '--json', gate: { json: true } }, + ]; + + for (const { name, gate } of cases) { + test(`does not prompt or post when ${name}`, async () => { + const base = interactive({ reason: 'unreliable' }); + const { submissions, submit } = recordingSubmit(); + const outcome = await promptUninstallFeedback({ ...base, ...gate, submit }); + expect(outcome).toBe('not-prompted'); + expect(base.collected).toEqual([]); + expect(submissions).toEqual([]); + }); + } + + test('prompts when both streams are interactive and no flag suppresses it', async () => { + const base = interactive({ reason: 'unreliable' }); + const { submit } = recordingSubmit(); + await promptUninstallFeedback({ ...base, submit }); + expect(base.collected).toEqual(['prompted']); + }); +}); + +describe('promptUninstallFeedback submission', () => { + test('posts the chosen reason tagged as the CLI surface', async () => { + const { submissions, submit } = recordingSubmit(); + const outcome = await promptUninstallFeedback({ + ...interactive({ reason: 'switched-tool', note: 'Moved to something else', email: 'a@b.co' }), + submit, + }); + expect(outcome).toBe('submitted'); + expect(submissions).toEqual([ + { + reason: 'switched-tool', + note: 'Moved to something else', + email: 'a@b.co', + source: 'cli_uninstall', + appVersion: '1.2.3', + platform: 'darwin', + }, + ]); + }); + + test('posts nothing when the user skipped every question', async () => { + const { submissions, submit } = recordingSubmit(); + const outcome = await promptUninstallFeedback({ ...interactive({}), submit }); + expect(outcome).toBe('skipped'); + expect(submissions).toEqual([]); + }); + + test('posts a note with no reason', async () => { + const { submissions, submit } = recordingSubmit(); + const outcome = await promptUninstallFeedback({ + ...interactive({ note: 'the sync broke' }), + submit, + }); + expect(outcome).toBe('submitted'); + expect(submissions[0]?.note).toBe('the sync broke'); + }); + + test('treats a whitespace-only note as nothing to file', async () => { + const { submissions, submit } = recordingSubmit(); + const outcome = await promptUninstallFeedback({ ...interactive({ note: ' ' }), submit }); + expect(outcome).toBe('skipped'); + expect(submissions).toEqual([]); + }); + + // The caller proceeds either way, but the outcome must not call a POST that + // never landed "submitted" — that is the signal a schema drift would show up in. + test('reports a send that never landed as undelivered, not submitted', async () => { + const { submit } = recordingSubmit({ ok: false, reason: 'timeout' }); + const outcome = await promptUninstallFeedback({ + ...interactive({ reason: 'one-off' }), + submit, + }); + expect(outcome).toBe('undelivered'); + }); + + test('a prompt interrupted by SIGINT resolves instead of throwing', async () => { + const { submissions, submit } = recordingSubmit(); + const outcome = await promptUninstallFeedback({ + stdinIsTTY: true, + stdoutIsTTY: true, + appVersion: '1.2.3', + platform: 'darwin', + collect: async () => { + throw new Error('User force closed the prompt with SIGINT'); + }, + submit, + }); + expect(outcome).toBe('failed'); + expect(submissions).toEqual([]); + }); +}); + +describe('ok uninstall feedback step', () => { + /** A temp home with the machine-touching removal primitives stubbed out. */ + function uninstallFixture(): { home: string; cleanup: () => void } { + const home = mkdtempSync(join(tmpdir(), 'ok-uninst-fb-')); + write(join(home, '.ok', 'auth.yml'), 'x\n'); + return { home, cleanup: () => rmSync(home, { recursive: true, force: true }) }; + } + + const stubbedRemoval = { + clearToken: async () => ({ touched: [] }), + clearEmbeddingsKey: async () => ({ touched: [] }), + stopServer: () => ({ stopped: 0, failed: [] }), + }; + + test('asks after the removal report and awaits the send', async () => { + const { home, cleanup } = uninstallFixture(); + try { + const order: string[] = []; + const result = await runUninstall({ + home, + platform: 'darwin', + cwd: home, + isTTY: true, + isStdinTTY: true, + confirmStream: Readable.from(['y\n']), + deps: { + discoverLockDirs: async () => [], + detectInstallMethods: () => [], + runRemovalDeps: stubbedRemoval, + feedback: { + collect: async () => { + order.push('asked'); + return { reason: 'missing-feature' }; + }, + submit: async (submission) => { + // Feedback is asked AFTER removal now, so by the time this settles + // the credentials are already gone — and the send is still awaited, + // so the fully-populated `order` after `runUninstall` proves the + // POST flushed before the process would exit (an un-awaited POST + // would resolve after teardown, i.e. after the process is gone). + await new Promise((resolve) => setTimeout(resolve, 0)); + order.push( + existsSync(join(home, '.ok', 'auth.yml')) + ? `posted-before-cleanup:${submission.source}` + : 'posted-after-cleanup', + ); + return { ok: true, reference: 'FB-2' }; + }, + }, + }, + }); + // The survey is deferred to the caller now, so the removal report prints + // before the prompt. Running it drives the survey and awaits the POST, + // which by now lands after cleanup (auth.yml already gone). + await result.runFeedbackAfterReport?.(); + expect(order).toEqual(['asked', 'posted-after-cleanup']); + expect(result.status).toBe('done'); + expect(existsSync(join(home, '.ok', 'auth.yml'))).toBe(false); + } finally { + cleanup(); + } + }); + + test('does not ask why when the removal itself failed', async () => { + const { home, cleanup } = uninstallFixture(); + try { + const collect = vi.fn( + async (): Promise => ({ + reason: 'missing-feature', + }), + ); + const submit = vi.fn( + async (): Promise => ({ ok: true, reference: 'X' }), + ); + const result = await runUninstall({ + home, + platform: 'darwin', + cwd: home, + isTTY: true, + isStdinTTY: true, + confirmStream: Readable.from(['y\n']), + deps: { + discoverLockDirs: async () => ['/some/proj/.ok/local'], // → a stop-server op + detectInstallMethods: () => [], + runRemovalDeps: { + ...stubbedRemoval, + // The SIGTERM fails → the removal outcome carries a failed op. + stopServer: () => ({ stopped: 0, failed: [{ pid: 99, error: 'EPERM' }] }), + }, + feedback: { collect, submit }, + }, + }); + expect(result.status).toBe('failed'); + // No survey closure is even created on a failed removal — the structural + // contract, stronger than the mock-call-count assertions below. + expect(result.runFeedbackAfterReport).toBeUndefined(); + expect(collect).not.toHaveBeenCalled(); + expect(submit).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test('an interrupted feedback prompt does not abort the confirmed removal', async () => { + const { home, cleanup } = uninstallFixture(); + try { + const result = await runUninstall({ + home, + platform: 'darwin', + cwd: home, + isTTY: true, + isStdinTTY: true, + confirmStream: Readable.from(['y\n']), + deps: { + discoverLockDirs: async () => [], + detectInstallMethods: () => [], + runRemovalDeps: stubbedRemoval, + feedback: { + collect: async () => { + throw new Error('User force closed the prompt with SIGINT'); + }, + }, + }, + }); + // The survey is deferred to the caller now, so run it here to actually + // fire the SIGINT-throwing collect. promptUninstallFeedback absorbs the + // throw (resolve-never-throw), leaving the completed removal undisturbed. + await result.runFeedbackAfterReport?.(); + expect(result.status).toBe('done'); + expect(result.exitCode).toBe(0); + expect(existsSync(join(home, '.ok', 'auth.yml'))).toBe(false); + } finally { + cleanup(); + } + }); + + // `echo y | ok uninstall` — stdout is a terminal but stdin is a pipe, so + // inquirer would render a prompt that can never receive a keystroke. Pins that + // the flow passes the stdin stream through, not stdout twice. + test('a piped stdin on an interactive terminal is not surveyed', async () => { + const { home, cleanup } = uninstallFixture(); + try { + const { submissions, submit } = recordingSubmit(); + const collected: string[] = []; + const result = await runUninstall({ + home, + platform: 'darwin', + cwd: home, + isTTY: true, + isStdinTTY: false, + confirmStream: Readable.from(['y\n']), + deps: { + discoverLockDirs: async () => [], + detectInstallMethods: () => [], + runRemovalDeps: stubbedRemoval, + feedback: { + collect: async () => { + collected.push('prompted'); + return { reason: 'other' }; + }, + submit, + }, + }, + }); + expect(result.status).toBe('done'); + expect(collected).toEqual([]); + expect(submissions).toEqual([]); + } finally { + cleanup(); + } + }); + + test('--yes removes without ever asking', async () => { + const { home, cleanup } = uninstallFixture(); + try { + const { submissions, submit } = recordingSubmit(); + const collected: string[] = []; + const result = await runUninstall({ + home, + platform: 'darwin', + cwd: home, + yes: true, + deps: { + discoverLockDirs: async () => [], + detectInstallMethods: () => [], + runRemovalDeps: stubbedRemoval, + feedback: { + collect: async () => { + collected.push('prompted'); + return { reason: 'other' }; + }, + submit, + }, + }, + }); + expect(result.status).toBe('done'); + expect(collected).toEqual([]); + expect(submissions).toEqual([]); + } finally { + cleanup(); + } + }); +}); diff --git a/packages/cli/src/commands/uninstall-feedback.ts b/packages/cli/src/commands/uninstall-feedback.ts new file mode 100644 index 000000000..0142351db --- /dev/null +++ b/packages/cli/src/commands/uninstall-feedback.ts @@ -0,0 +1,154 @@ +/** + * The optional churn survey `ok uninstall` runs once the removal has already + * succeeded: one reason, an optional note, an optional follow-up address. Only + * a completed uninstall is a departure worth asking about, and by then there is + * nothing left to interrupt. + * + * Two rules shape everything here. The removal is already done by the time this + * runs, so nothing in it may fail the command — every failure path continues. + * And it must stay silent unless a human is actually watching: the desktop + * cleanup script shells out `ok uninstall --yes`, and a prompt there would hang + * a detached process forever. + */ + +import { createInterface } from 'node:readline/promises'; +import { + hasUninstallFeedbackContent, + postUninstallFeedback, + UNINSTALL_FEEDBACK_REASONS, + type UninstallFeedbackAnswers, + type UninstallFeedbackReason, + type UninstallFeedbackResult, + type UninstallFeedbackSubmission, +} from '@inkeep/open-knowledge-core'; +import select from '@inquirer/select'; +import { accent, dim } from '../ui/colors.ts'; + +/** `null` is the skip choice — no slug can stand in for "declined to answer". */ +type ReasonChoice = UninstallFeedbackReason | null; + +interface UninstallFeedbackGate { + /** Defaults to the real stdin; inquirer reads it, so it must be interactive. */ + stdinIsTTY?: boolean; + /** Defaults to the real stdout. Piped means a script is reading us, not a person. */ + stdoutIsTTY?: boolean; + yes?: boolean; + json?: boolean; +} + +/** + * Whether a human is present to answer. Both streams must be a terminal — + * gating on stdout alone would still prompt a caller that piped only its input, + * and inquirer would then wait on a stream that never delivers a keystroke. + */ +function shouldPromptUninstallFeedback(gate: UninstallFeedbackGate): boolean { + if (gate.yes === true || gate.json === true) return false; + const stdin = gate.stdinIsTTY ?? process.stdin.isTTY; + const stdout = gate.stdoutIsTTY ?? process.stdout.isTTY; + return stdin === true && stdout === true; +} + +/** + * Where the survey talks to the user. Prompts render on stderr by default so + * the removal report keeps stdout to itself, matching `confirmDestructive`. + * Injectable so the prompt sequence itself is testable. + */ +export interface UninstallFeedbackIO { + input?: NodeJS.ReadableStream; + output?: NodeJS.WritableStream; +} + +async function askOptionalLine(io: Required, prompt: string): Promise { + const rl = createInterface({ input: io.input, output: io.output }); + try { + return (await rl.question(prompt)).trim(); + } finally { + rl.close(); + } +} + +/** + * The default prompt sequence. Skipping the reason ends the survey — someone + * who just said they'd rather not say should not then be asked twice more. + */ +export async function collectUninstallFeedbackAnswers( + io: UninstallFeedbackIO = {}, +): Promise { + const streams = { input: io.input ?? process.stdin, output: io.output ?? process.stderr }; + // Said before the first question, not beside the email field: the note is + // just as much of a send, and the answer to "where does this go?" should not + // arrive after someone has already typed it. + streams.output.write(dim('\nWhat you share is sent to the OpenKnowledge team.\n')); + const reason = await select( + { + message: 'Before you go, mind sharing why? (optional)', + // Start on the opt-out so a reflexive Enter can't invent a churn reason. + default: null, + // Every reason on screen at once. The default page size is one short of + // the taxonomy plus the skip row, which would scroll the first reason out + // of view precisely because the cursor starts at the bottom. + pageSize: UNINSTALL_FEEDBACK_REASONS.length + 1, + choices: [ + ...UNINSTALL_FEEDBACK_REASONS.map((option) => ({ + name: option.label, + value: option.value as ReasonChoice, + })), + { name: "Skip, I'd rather not say", value: null }, + ], + }, + streams, + ); + if (reason === null) return {}; + + const note = await askOptionalLine(streams, dim('Anything else we should know? (optional) ')); + const email = await askOptionalLine(streams, dim('Email, if we may follow up (optional) ')); + return { reason, note: note || undefined, email: email || undefined }; +} + +export interface UninstallFeedbackPromptDeps extends UninstallFeedbackGate, UninstallFeedbackIO { + appVersion: string; + platform: string; + /** Test hook for the prompt sequence. */ + collect?: () => Promise; + /** Test hook for the intake transport. */ + submit?: (submission: UninstallFeedbackSubmission) => Promise; +} + +export type UninstallFeedbackOutcome = + | 'not-prompted' + | 'skipped' + | 'submitted' + | 'undelivered' + | 'failed'; + +/** + * Ask, then file. The returned outcome is diagnostic only — the caller removes + * OpenKnowledge either way — but it still reports the send honestly rather than + * calling a dropped POST "submitted". + */ +export async function promptUninstallFeedback( + deps: UninstallFeedbackPromptDeps, +): Promise { + if (!shouldPromptUninstallFeedback(deps)) return 'not-prompted'; + let answers: UninstallFeedbackAnswers; + try { + answers = await (deps.collect ?? (() => collectUninstallFeedbackAnswers(deps)))(); + } catch { + // Ctrl-C at a prompt, or a stdin that closed underneath it. Both are + // external events, and neither may propagate into a removal that has + // already happened. + return 'failed'; + } + if (!hasUninstallFeedbackContent(answers)) return 'skipped'; + // Thank the person, not the transport: the send is best-effort and bounded, + // so waiting to report on it would either stall the exit or report a lie. + (deps.output ?? process.stderr).write(`\n${accent('Thank you. We read every response.')}\n\n`); + // The transport resolves on every failure it can have, so this needs no guard. + const result = await (deps.submit ?? postUninstallFeedback)({ + ...answers, + source: 'cli_uninstall', + appVersion: deps.appVersion, + platform: deps.platform, + }); + return result.ok ? 'submitted' : 'undelivered'; +} diff --git a/packages/cli/src/commands/uninstall.ts b/packages/cli/src/commands/uninstall.ts index 9f4ce0d78..1b18cbe16 100644 --- a/packages/cli/src/commands/uninstall.ts +++ b/packages/cli/src/commands/uninstall.ts @@ -20,6 +20,7 @@ import { join, resolve } from 'node:path'; import { findEnclosingProjectRoot, withHiddenWindowsConsole } from '@inkeep/open-knowledge-server'; import checkbox from '@inquirer/checkbox'; import { Command } from 'commander'; +import { PACKAGE_VERSION } from '../constants.ts'; import { desktopUserDataDir, readDesktopRecentProjects } from '../integrations/desktop-state.ts'; import { readPathInstallMarker } from '../integrations/path-shim.ts'; import { accent, dim, error as errorColor, info, success, warning } from '../ui/colors.ts'; @@ -32,6 +33,7 @@ import { removalOutcomeToJson, removalPlanToJson, } from './removal-render.ts'; +import { promptUninstallFeedback, type UninstallFeedbackPromptDeps } from './uninstall-feedback.ts'; // --------------------------------------------------------------------------- // Install-method detection (detect + instruct; never self-delete) @@ -268,6 +270,8 @@ interface UninstallDeps { * exercised end-to-end without touching the real OS keychain. */ runRemovalDeps?: RunRemovalDeps; + /** Prompt + transport seams for the post-confirm churn survey. */ + feedback?: Pick; } export interface UninstallOptions { @@ -281,7 +285,10 @@ export interface UninstallOptions { json?: boolean; purgeContent?: boolean; allProjects?: boolean; + /** Whether stdout is a terminal. */ isTTY?: boolean; + /** Whether stdin is a terminal — only the feedback prompt reads from it. */ + isStdinTTY?: boolean; argv1?: string; confirmStream?: NodeJS.ReadableStream; deps?: UninstallDeps; @@ -291,6 +298,13 @@ export interface UninstallResult { status: 'dry-run' | 'cancelled' | 'done' | 'failed'; message: string; exitCode: number; + /** + * The churn survey, deferred so the caller runs it AFTER printing `message` — + * a departing user sees the "files removed" confirmation before being asked + * why. Present only on a successful removal (the sole case that surveys), and + * the caller must await it so the POST flushes before the process exits. + */ + runFeedbackAfterReport?: () => Promise; } const URL_SCHEME_NOTE = dim( @@ -390,6 +404,26 @@ export async function runUninstall(opts: UninstallOptions = {}): Promise => { + await promptUninstallFeedback({ + stdinIsTTY: opts.isStdinTTY, + stdoutIsTTY: opts.isTTY, + yes: opts.yes, + json: opts.json, + appVersion: PACKAGE_VERSION, + platform, + ...opts.deps?.feedback, + }); + } + : undefined; + const parts = [ opts.json ? JSON.stringify(removalOutcomeToJson('uninstall', outcome), null, 2) @@ -409,6 +443,7 @@ export async function runUninstall(opts: UninstallOptions = {}): Promise 0 ? 'failed' : 'done', message: parts.join('\n'), exitCode: outcome.failed.length > 0 ? 1 : 0, + runFeedbackAfterReport, }; } @@ -444,6 +479,10 @@ export function uninstallCommand(): Command { allProjects: options.allProjects, }); process.stdout.write(`${result.message}\n`); + // After the report is on screen (so "files removed" lands before the + // survey), run the deferred churn prompt and await it so the POST + // flushes before the process exits. + await result.runFeedbackAfterReport?.(); if (result.exitCode !== 0) process.exitCode = result.exitCode; }, ); diff --git a/packages/core/src/constants/uninstall-feedback.test.ts b/packages/core/src/constants/uninstall-feedback.test.ts new file mode 100644 index 000000000..dd195de86 --- /dev/null +++ b/packages/core/src/constants/uninstall-feedback.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, test } from 'vitest'; +import { UNINSTALL_FEEDBACK_REASONS as BARREL_REASONS } from '../index.ts'; +import { isUninstallFeedbackReason, UNINSTALL_FEEDBACK_REASONS } from './uninstall-feedback.ts'; + +describe('uninstall feedback reasons taxonomy', () => { + // Slugs travel to `/api/feedback` and are how churn tickets get grouped, so + // editing or reordering one silently re-buckets every reason already filed + // under it. Labels are display-only and free to reword. + test('pins the slug set and order', () => { + expect(UNINSTALL_FEEDBACK_REASONS.map((reason) => reason.value)).toEqual([ + 'workflow-fit', + 'missing-feature', + 'hard-to-start', + 'unreliable', + 'switched-tool', + 'one-off', + 'other', + ]); + }); + + test('every reason carries a distinct non-empty label', () => { + const labels = UNINSTALL_FEEDBACK_REASONS.map((reason) => reason.label); + for (const label of labels) { + expect(label.trim()).not.toBe(''); + } + expect(new Set(labels).size).toBe(labels.length); + }); + + test('is exported from the package barrel both surfaces import', () => { + expect(BARREL_REASONS).toBe(UNINSTALL_FEEDBACK_REASONS); + }); + + test('admits exactly the taxonomy slugs and nothing else', () => { + for (const reason of UNINSTALL_FEEDBACK_REASONS) { + expect(isUninstallFeedbackReason(reason.value)).toBe(true); + } + for (const value of ['', 'other ', 'Other', 'too-expensive', undefined, null, 0, {}]) { + expect(isUninstallFeedbackReason(value)).toBe(false); + } + }); +}); diff --git a/packages/core/src/constants/uninstall-feedback.ts b/packages/core/src/constants/uninstall-feedback.ts new file mode 100644 index 000000000..2c465c891 --- /dev/null +++ b/packages/core/src/constants/uninstall-feedback.ts @@ -0,0 +1,41 @@ +/** + * The churn-survey reasons offered when someone uninstalls OpenKnowledge — + * shared verbatim by the desktop uninstall window and the `ok uninstall` CLI + * prompt so the two surfaces can never drift apart. + * + * `value` is the contract: it travels to `/api/feedback` inside the opaque + * `reasons` array and is what churn tickets are grouped by, so a slug edit or a + * reorder silently re-buckets everything already filed. `label` is display-only + * and free to reword. + * + * Plain English, not Lingui — neither consumer (Electron main, CLI) is + * localization-wired. + */ +export const UNINSTALL_FEEDBACK_REASONS = Object.freeze([ + { value: 'workflow-fit', label: "It didn't fit into my workflow" }, + { value: 'missing-feature', label: 'It was missing a feature I needed' }, + { value: 'hard-to-start', label: 'It was too hard to set up or get started' }, + { value: 'unreliable', label: 'Bugs, crashes, or it felt unreliable' }, + { value: 'switched-tool', label: "I'm switching to another tool" }, + { value: 'one-off', label: 'It was a trial or one-off project' }, + { value: 'other', label: 'Something else' }, +] as const satisfies readonly { readonly value: string; readonly label: string }[]); + +/** A single offered reason, as rendered by the desktop window and CLI prompt. */ +type UninstallFeedbackReasonOption = (typeof UNINSTALL_FEEDBACK_REASONS)[number]; + +/** The slug half of the taxonomy — what actually goes on the wire. */ +export type UninstallFeedbackReason = UninstallFeedbackReasonOption['value']; + +const UNINSTALL_FEEDBACK_REASON_VALUES: ReadonlySet = new Set( + UNINSTALL_FEEDBACK_REASONS.map((option) => option.value), +); + +/** + * Narrow an inbound slug to the taxonomy. The desktop window hands its answers + * back to the main process through a navigation URL, so the slug arrives as an + * arbitrary string and has to be re-checked before it can be filed. + */ +export function isUninstallFeedbackReason(value: unknown): value is UninstallFeedbackReason { + return UNINSTALL_FEEDBACK_REASON_VALUES.has(value); +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0eace3e83..1c7d5bbdb 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -263,6 +263,11 @@ export { } from './constants/preview-theme-tokens.ts'; export { PRODUCT_NAME } from './constants/product.ts'; export { DEFAULT_SERVER_HOST } from './constants/server.ts'; +export { + isUninstallFeedbackReason, + UNINSTALL_FEEDBACK_REASONS, + type UninstallFeedbackReason, +} from './constants/uninstall-feedback.ts'; export { ALLOWED_AUDIO_MIME_TYPES, ALLOWED_IMAGE_MIME_TYPES, @@ -1080,6 +1085,15 @@ export { parseLoomUrl, } from './utils/loom-embed.ts'; export { type PdfAnchorParts, parsePdfAnchor } from './utils/pdf-anchor.ts'; +export { + hasUninstallFeedbackContent, + type PostUninstallFeedbackOptions, + postUninstallFeedback, + type UninstallFeedbackAnswers, + type UninstallFeedbackResult, + type UninstallFeedbackSource, + type UninstallFeedbackSubmission, +} from './utils/uninstall-feedback-submit.ts'; export { isVimeoUrl } from './utils/vimeo-embed.ts'; export { type ParsedYouTubeUrl, diff --git a/packages/core/src/utils/uninstall-feedback-submit.test.ts b/packages/core/src/utils/uninstall-feedback-submit.test.ts new file mode 100644 index 000000000..04cfbae58 --- /dev/null +++ b/packages/core/src/utils/uninstall-feedback-submit.test.ts @@ -0,0 +1,358 @@ +import { afterEach, describe, expect, test } from 'vitest'; +import { postUninstallFeedback as barrelPostUninstallFeedback } from '../index.ts'; +import { hasUninstallFeedbackContent, postUninstallFeedback } from './uninstall-feedback-submit.ts'; + +/** The origin baked into shipped builds, which a GUI app has no env to override. */ +const SHIPPED_INTAKE_ORIGIN = 'https://openknowledge.ai'; + +const HOST_FACTS = { source: 'cli_uninstall', appVersion: '1.2.3', platform: 'darwin' } as const; + +interface SeenRequest { + url: string; + method: string | undefined; + contentType: string | undefined; + body: Record; +} + +const realFetch = globalThis.fetch; +const realOriginEnv = process.env.OK_FEEDBACK_INTAKE_ORIGIN; + +afterEach(() => { + globalThis.fetch = realFetch; + if (realOriginEnv === undefined) delete process.env.OK_FEEDBACK_INTAKE_ORIGIN; + else process.env.OK_FEEDBACK_INTAKE_ORIGIN = realOriginEnv; +}); + +/** Installs a fetch that records each request and answers with `respond()`. */ +function recordRequests(respond: () => Response | Promise): SeenRequest[] { + const seen: SeenRequest[] = []; + globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + seen.push({ + url: String(input), + method: init?.method, + contentType: new Headers(init?.headers).get('content-type') ?? undefined, + body: JSON.parse(String(init?.body)) as Record, + }); + return Promise.resolve(respond()); + }) as typeof globalThis.fetch; + return seen; +} + +function jsonResponse(status: number, body: unknown): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); +} + +/** A request that never answers on its own — only the caller's abort ends it. */ +function hangUntilAborted(): { wasAborted: () => boolean } { + let wasAborted = false; + globalThis.fetch = ((_input: RequestInfo | URL, init?: RequestInit) => { + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + wasAborted = true; + reject(new DOMException('The operation was aborted', 'TimeoutError')); + }); + }); + }) as typeof globalThis.fetch; + return { wasAborted: () => wasAborted }; +} + +describe('postUninstallFeedback', () => { + test('posts the single-select wire shape the intake schema accepts', async () => { + const seen = recordRequests(() => jsonResponse(200, { reference: 'OK-42' })); + + const result = await postUninstallFeedback({ + ...HOST_FACTS, + reason: 'missing-feature', + note: 'Needed nested tags.', + email: 'departing@example.com', + }); + + expect(result).toEqual({ ok: true, reference: 'OK-42' }); + expect(seen).toHaveLength(1); + expect(seen[0]?.url).toBe(`${SHIPPED_INTAKE_ORIGIN}/api/feedback`); + expect(seen[0]?.method).toBe('POST'); + expect(seen[0]?.contentType).toBe('application/json'); + expect(seen[0]?.body).toEqual({ + kind: 'uninstall', + reasons: ['missing-feature'], + message: 'Needed nested tags.', + email: 'departing@example.com', + appVersion: '1.2.3', + platform: 'darwin', + source: 'cli_uninstall', + }); + }); + + test('sends an empty reasons array when the user only left a note', async () => { + const seen = recordRequests(() => jsonResponse(200, { reference: 'OK-43' })); + + await postUninstallFeedback({ ...HOST_FACTS, note: 'no reason fit' }); + + expect(seen[0]?.body).toMatchObject({ reasons: [], message: 'no reason fit' }); + expect(seen[0]?.body).not.toHaveProperty('email'); + }); + + // The intake validates `email` as an email and would 400 on `''`, so a + // blanked-out field has to travel as absent rather than as an empty string. + test('omits blank note and email rather than sending empty strings', async () => { + const seen = recordRequests(() => jsonResponse(200, { reference: 'OK-44' })); + + await postUninstallFeedback({ + ...HOST_FACTS, + reason: 'one-off', + note: ' ', + email: ' ', + }); + + expect(seen[0]?.body).not.toHaveProperty('message'); + expect(seen[0]?.body).not.toHaveProperty('email'); + }); + + test('trims surrounding whitespace off the note and email', async () => { + const seen = recordRequests(() => jsonResponse(200, { reference: 'OK-45' })); + + await postUninstallFeedback({ + ...HOST_FACTS, + note: ' too slow ', + email: ' someone@example.com ', + }); + + expect(seen[0]?.body).toMatchObject({ + message: 'too slow', + email: 'someone@example.com', + }); + }); + + test('reports success even when the response body carries no reference', async () => { + recordRequests(() => new Response('', { status: 200 })); + + await expect(postUninstallFeedback({ ...HOST_FACTS, reason: 'other' })).resolves.toEqual({ + ok: true, + reference: '', + }); + }); + + test.each([ + { status: 400, reason: 'invalid' }, + { status: 413, reason: 'invalid' }, + { status: 503, reason: 'unavailable' }, + { status: 500, reason: 'error' }, + { status: 429, reason: 'error' }, + ])('maps HTTP $status to reason $reason', async ({ status, reason }) => { + recordRequests(() => new Response('', { status })); + + await expect(postUninstallFeedback({ ...HOST_FACTS, reason: 'other' })).resolves.toEqual({ + ok: false, + reason, + }); + }); + + test('resolves rather than throwing when the network fails', async () => { + globalThis.fetch = (() => + Promise.reject(new Error('getaddrinfo ENOTFOUND'))) as typeof globalThis.fetch; + + await expect(postUninstallFeedback({ ...HOST_FACTS, reason: 'unreliable' })).resolves.toEqual({ + ok: false, + reason: 'error', + }); + }); + + // A departing user must never be parked on a hung intake: the request is + // abandoned at the ceiling so the caller can get on with the uninstall. + test('abandons a hung request at the timeout instead of blocking the caller', async () => { + const hung = hangUntilAborted(); + + const result = await postUninstallFeedback( + { ...HOST_FACTS, reason: 'switched-tool' }, + { timeoutMs: 25 }, + ); + + expect(hung.wasAborted()).toBe(true); + expect(result).toEqual({ ok: false, reason: 'timeout' }); + }); + + test('targets the loopback intake origin the env names', async () => { + process.env.OK_FEEDBACK_INTAKE_ORIGIN = 'http://localhost:4321'; + const seen = recordRequests(() => jsonResponse(200, { reference: 'OK-47' })); + + await postUninstallFeedback({ ...HOST_FACTS, reason: 'other' }); + + expect(seen[0]?.url).toBe('http://localhost:4321/api/feedback'); + }); + + // A departing user's note and follow-up address must never go out in + // cleartext, and a bad origin must fail the send rather than silently + // reverting to the shipped one. + test.each([ + { origin: 'not a url', label: 'unparseable' }, + { origin: 'http://feedback.example.com', label: 'plaintext off-box' }, + { origin: 'ftp://localhost:4321', label: 'non-web scheme' }, + ])('refuses to send to an $label origin', async ({ origin }) => { + process.env.OK_FEEDBACK_INTAKE_ORIGIN = origin; + const seen = recordRequests(() => jsonResponse(200, { reference: 'OK-48' })); + + await expect(postUninstallFeedback({ ...HOST_FACTS, reason: 'other' })).resolves.toEqual({ + ok: false, + reason: 'error', + }); + expect(seen).toEqual([]); + }); + + test('sends to an off-box origin over https', async () => { + process.env.OK_FEEDBACK_INTAKE_ORIGIN = 'https://staging.example.com'; + const seen = recordRequests(() => jsonResponse(200, { reference: 'OK-49' })); + + await postUninstallFeedback({ ...HOST_FACTS, reason: 'other' }); + + expect(seen[0]?.url).toBe('https://staging.example.com/api/feedback'); + }); + + // The intake validates the whole body at once, so shipping a typo'd address + // would 400 the request and lose the reason and note along with it. + test.each([ + 'me@', + 'me.com', + 'not an address', + ])('never spends a round trip on the obviously-broken address %s', async (email) => { + const seen = recordRequests(() => jsonResponse(200, { reference: 'OK-50' })); + + const result = await postUninstallFeedback({ + ...HOST_FACTS, + reason: 'unreliable', + note: 'kept crashing', + email, + }); + + expect(result).toEqual({ ok: true, reference: 'OK-50' }); + expect(seen).toHaveLength(1); + expect(seen[0]?.body).toMatchObject({ reasons: ['unreliable'], message: 'kept crashing' }); + expect(seen[0]?.body).not.toHaveProperty('email'); + }); + + // These pass this side's cheap check but fail the intake's stricter + // `z.email()`, which would otherwise reject the whole body. Correctness must + // not depend on the two validators agreeing across the mirror boundary. + test.each([ + 'me@example.c', + 'josé@example.com', + 'a..b@example.com', + ])('refiles without the address when the intake rejects %s', async (email) => { + let attempts = 0; + const seen = recordRequests(() => { + attempts += 1; + return attempts === 1 + ? new Response('', { status: 400 }) + : jsonResponse(200, { reference: 'OK-51' }); + }); + + const result = await postUninstallFeedback({ + ...HOST_FACTS, + reason: 'unreliable', + note: 'kept crashing', + email, + }); + + expect(result).toEqual({ ok: true, reference: 'OK-51' }); + expect(seen).toHaveLength(2); + expect(seen[0]?.body).toMatchObject({ email }); + expect(seen[1]?.body).toMatchObject({ reasons: ['unreliable'], message: 'kept crashing' }); + expect(seen[1]?.body).not.toHaveProperty('email'); + }); + + test('retries at most once, so a body rejected for another reason still settles', async () => { + const seen = recordRequests(() => new Response('', { status: 400 })); + + const result = await postUninstallFeedback({ + ...HOST_FACTS, + reason: 'other', + email: 'me@example.c', + }); + + expect(result).toEqual({ ok: false, reason: 'invalid' }); + expect(seen).toHaveLength(2); + }); + + test('does not retry a rejection when there was no address to blame', async () => { + const seen = recordRequests(() => new Response('', { status: 400 })); + + await postUninstallFeedback({ ...HOST_FACTS, reason: 'other', note: 'no address given' }); + + expect(seen).toHaveLength(1); + }); + + // Only a rejected body is worth refiling. Feedback being switched off, a + // hung intake, or a dead network say nothing about the address, and a second + // attempt would just spend the caller's remaining budget. + test.each([503, 500])('does not retry a %s, which the address cannot explain', async (status) => { + const seen = recordRequests(() => new Response('', { status })); + + await postUninstallFeedback({ ...HOST_FACTS, reason: 'other', email: 'me@example.com' }); + + expect(seen).toHaveLength(1); + }); + + // The retry shares the caller's ceiling rather than starting a fresh one — + // the desktop flow holds the finish screen open for exactly this budget. + // + // Deliberately slow for a unit test. The only thing separating the two + // implementations is elapsed time, and `AbortSignal.timeout` runs on Node's + // internal timer, which vitest's fake timers do not drive — so this has to + // use the real clock, and the constants are sized to leave a margin that + // survives CPU contention across parallel workers rather than to run fast. + // The shared deadline is self-correcting (overshoot on the first attempt + // shrinks what the retry gets), so the margin only has to cover the final + // abort delivery: correct lands at ~BUDGET, per-attempt at ~BUDGET + FIRST. + test('spends one budget across both attempts, not one budget each', async () => { + const BUDGET = 600; + const FIRST_ATTEMPT_MS = 360; + let attempts = 0; + globalThis.fetch = ((_input: RequestInfo | URL, init?: RequestInit) => { + attempts += 1; + // The rejection arrives late, then the refile hangs: a per-attempt + // timeout would let the pair run to BUDGET + FIRST_ATTEMPT_MS. + if (attempts === 1) { + return new Promise((resolve) => + setTimeout(() => resolve(new Response('', { status: 400 })), FIRST_ATTEMPT_MS), + ); + } + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => + reject(new DOMException('The operation was aborted', 'TimeoutError')), + ); + }); + }) as typeof globalThis.fetch; + + const startedAt = Date.now(); + const result = await postUninstallFeedback( + { ...HOST_FACTS, reason: 'other', email: 'me@example.c' }, + { timeoutMs: BUDGET }, + ); + const elapsed = Date.now() - startedAt; + + expect(attempts).toBe(2); + expect(result).toEqual({ ok: false, reason: 'timeout' }); + expect(elapsed).toBeLessThan(BUDGET + FIRST_ATTEMPT_MS / 2); + }); + + test('is exported from the package barrel both surfaces import', () => { + expect(barrelPostUninstallFeedback).toBe(postUninstallFeedback); + }); +}); + +describe('hasUninstallFeedbackContent', () => { + // Both uninstall surfaces gate their POST on this so an untouched form can + // never file an empty churn ticket, and so the two can't drift on what + // "empty" means. + test.each([ + { answers: {}, expected: false }, + { answers: { note: ' ', email: '\t' }, expected: false }, + { answers: { reason: 'other' as const }, expected: true }, + { answers: { note: 'something' }, expected: true }, + { answers: { email: 'someone@example.com' }, expected: true }, + ])('is $expected for $answers', ({ answers, expected }) => { + expect(hasUninstallFeedbackContent(answers)).toBe(expected); + }); +}); diff --git a/packages/core/src/utils/uninstall-feedback-submit.ts b/packages/core/src/utils/uninstall-feedback-submit.ts new file mode 100644 index 000000000..6d79220c6 --- /dev/null +++ b/packages/core/src/utils/uninstall-feedback-submit.ts @@ -0,0 +1,199 @@ +import type { UninstallFeedbackReason } from '../constants/uninstall-feedback.ts'; + +/** + * Best-effort churn-survey transport for the two non-React uninstall surfaces — + * the desktop uninstall window (Electron main) and `ok uninstall`. Both are + * plain Node, so this deliberately avoids the browser-only machinery in the + * editor app's `lib/feedback.ts` (`FileReader`, `import.meta.env`). + * + * It resolves rather than throws on every failure: the caller is midway through + * removing OpenKnowledge, and a dropped feedback POST must never surface as an + * error or stall the removal past the timeout ceiling. + */ + +/** Which uninstall surface collected the answers — bounded, for analytics. */ +export type UninstallFeedbackSource = 'desktop_uninstall' | 'cli_uninstall'; + +/** The part a departing user fills in; every field is optional by design. */ +export interface UninstallFeedbackAnswers { + /** The single primary reason, when one was picked. */ + reason?: UninstallFeedbackReason; + /** Free-text elaboration, sent as the ticket `message`. */ + note?: string; + /** Follow-up address, sent only when the user opted in. */ + email?: string; +} + +export interface UninstallFeedbackSubmission extends UninstallFeedbackAnswers { + source: UninstallFeedbackSource; + appVersion: string; + platform: string; +} + +export interface PostUninstallFeedbackOptions { + /** + * Ceiling on the whole operation, including the retry a rejected address + * triggers; the caller proceeds once it elapses. + */ + timeoutMs?: number; +} + +// Diverges from the in-app `FeedbackResult` (app/src/lib/feedback.ts) by one +// member: `timeout`. That path is browser-driven with no `app.quit()` deadline +// pressing on it, so it never has to distinguish a bounded-wait abandonment from +// a plain error. The desktop/CLI path does, because the survey is flushed before +// the process exits, so the extra variant is deliberate, not drift. +export type UninstallFeedbackResult = + | { ok: true; reference: string } + // `invalid` — rejected as malformed (400) or oversized (413). + // `unavailable` — feedback is turned off server-side (503). + // `timeout` — abandoned at the ceiling; the request may still land. + // `error` — anything else (5xx, network, unusable origin). + | { ok: false; reason: 'invalid' | 'unavailable' | 'timeout' | 'error' }; + +/** + * Shipped default, matching the desktop bug-report intake: a GUI-launched app + * never receives a shell env var, so the production origin has to be baked in. + */ +const DEFAULT_INTAKE_ORIGIN = 'https://openknowledge.ai'; + +/** + * Short enough that a departing user never notices the wait, long enough for + * the POST to flush before the desktop flow reaches `app.quit()` — a + * fire-and-forget request would be torn down mid-flight in a packaged build. + */ +const DEFAULT_TIMEOUT_MS = 4_000; + +/** Collapses whitespace-only input to absent — the intake rejects `email: ''`. */ +function presentText(value: string | undefined): string | undefined { + const trimmed = value?.trim(); + return trimmed === undefined || trimmed === '' ? undefined : trimmed; +} + +/** + * Whether the user actually left anything worth filing. Both surfaces gate + * their POST on this so an untouched form can't create an empty churn ticket, + * and so neither can drift on what counts as empty. + */ +export function hasUninstallFeedbackContent(answers: UninstallFeedbackAnswers): boolean { + return ( + answers.reason !== undefined || + presentText(answers.note) !== undefined || + presentText(answers.email) !== undefined + ); +} + +/** + * Admit an intake origin only when transport-safe: `https:` anywhere, or plain + * `http:` strictly on loopback, so a dev run can point at a local marketing + * server. Anything else would carry a departing user's note — and the email + * address they just handed over — in cleartext to a MITM-able endpoint. Mirrors + * the bug-report upload's gate; transport encryption is all either one claims. + */ +function transportSafeOrigin(value: string): URL | null { + let url: URL; + try { + url = new URL(value); + } catch { + return null; + } + if (url.protocol === 'https:') return url; + const loopback = + url.hostname === 'localhost' || url.hostname === '127.0.0.1' || url.hostname === '[::1]'; + return url.protocol === 'http:' && loopback ? url : null; +} + +/** + * `null` when the configured origin is unusable — the send then fails rather + * than quietly reverting to the shipped origin, so a misconfigured env can + * never redirect the answers somewhere unintended. + */ +function resolveIntakeOrigin(): URL | null { + // Core also ships into the browser bundle, where `process` does not exist. + const fromEnv = + typeof process === 'undefined' + ? undefined + : presentText(process.env?.OK_FEEDBACK_INTAKE_ORIGIN); + return transportSafeOrigin(fromEnv ?? DEFAULT_INTAKE_ORIGIN); +} + +/** + * A cheap first pass at the address, so the ordinary `me@` / `me.com` typo + * never costs a round trip. Deliberately NOT an attempt to mirror the intake's + * validator: that one lives across the mirror boundary and is stricter in ways + * this cannot track, which is why a rejection is also recovered from below. + */ +function plausibleEmail(value: string | undefined): string | undefined { + const trimmed = presentText(value); + if (trimmed === undefined) return undefined; + return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(trimmed) ? trimmed : undefined; +} + +async function sendFeedback( + url: URL, + body: Record, + timeoutMs: number, +): Promise { + try { + const response = await fetch(url, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify(body), + signal: AbortSignal.timeout(timeoutMs), + }); + if (response.ok) { + // The ticket is filed; an unparseable body only costs us the reference, + // which no uninstall surface displays. + const data = (await response.json().catch(() => null)) as { reference?: unknown } | null; + return { ok: true, reference: typeof data?.reference === 'string' ? data.reference : '' }; + } + if (response.status === 400 || response.status === 413) return { ok: false, reason: 'invalid' }; + if (response.status === 503) return { ok: false, reason: 'unavailable' }; + return { ok: false, reason: 'error' }; + } catch (err) { + // The network is the trust boundary: offline, DNS failure, a hung intake + // hitting the ceiling, or an unusable configured origin all land here. + // Runtimes disagree on whether an `AbortSignal.timeout()` abort carries + // `TimeoutError` or `AbortError`, so both count as the ceiling firing. + const timedOut = + err instanceof Error && (err.name === 'TimeoutError' || err.name === 'AbortError'); + return { ok: false, reason: timedOut ? 'timeout' : 'error' }; + } +} + +/** + * File one uninstall-feedback ticket. Callers should first check + * {@link hasUninstallFeedbackContent} — this posts whatever it is handed. + */ +export async function postUninstallFeedback( + submission: UninstallFeedbackSubmission, + options: PostUninstallFeedbackOptions = {}, +): Promise { + const origin = resolveIntakeOrigin(); + if (origin === null) return { ok: false, reason: 'error' }; + const url = new URL('/api/feedback', origin); + const message = presentText(submission.note); + const email = plausibleEmail(submission.email); + const body = { + kind: 'uninstall', + reasons: submission.reason === undefined ? [] : [submission.reason], + ...(message === undefined ? {} : { message }), + appVersion: submission.appVersion, + platform: submission.platform, + source: submission.source, + }; + // One ceiling for the whole operation, retry included, so the departing user + // waits no longer than the caller budgeted for. + const deadline = Date.now() + (options.timeoutMs ?? DEFAULT_TIMEOUT_MS); + const remaining = (): number => deadline - Date.now(); + + if (email === undefined) return sendFeedback(url, body, remaining()); + const withEmail = await sendFeedback(url, { ...body, email }, remaining()); + // The intake validates the whole body at once, so an address this side let + // through but its stricter validator rejects would take the reason and the + // note down with it. Refile without the address: those are what the survey + // exists to collect, and a rejected body never reached the ticket tracker, + // so this cannot duplicate one. + if (withEmail.ok || withEmail.reason !== 'invalid' || remaining() <= 0) return withEmail; + return sendFeedback(url, body, remaining()); +} diff --git a/packages/desktop/src/main/desktop-uninstall.ts b/packages/desktop/src/main/desktop-uninstall.ts index e2bd430bb..d41b09b03 100644 --- a/packages/desktop/src/main/desktop-uninstall.ts +++ b/packages/desktop/src/main/desktop-uninstall.ts @@ -7,14 +7,24 @@ * `ok uninstall --yes` for the global footprint), and finally reveals * OpenKnowledge.app in Finder so the user can drag it to the Trash. * - * Electron-free + dependency-injected so the path predicates and generated - * helper script are unit-testable without an Electron runtime. + * Electron-free + dependency-injected so the path predicates, the flow + * decisions, and the generated helper script are unit-testable without an + * Electron runtime. */ import { spawn as spawnChild } from 'node:child_process'; import { existsSync, readFileSync } from 'node:fs'; import { homedir } from 'node:os'; import { basename, dirname, join, resolve } from 'node:path'; +import { + hasUninstallFeedbackContent, + isUninstallFeedbackReason, + postUninstallFeedback, + UNINSTALL_FEEDBACK_REASONS, + type UninstallFeedbackAnswers, + type UninstallFeedbackResult, + type UninstallFeedbackSubmission, +} from '@inkeep/open-knowledge-core'; const APP_BUNDLE_FROM_EXEC_RE = /^(.*\.app)\/Contents\/MacOS\/[^/]+$/; const SUPPORTED_APP_BUNDLE_NAME = 'OpenKnowledge.app'; @@ -56,6 +66,23 @@ export type RunDesktopUninstallCleanupResult = | { ok: true } | { ok: false; error: string; exitCode?: number | null }; +export type DesktopUninstallUiPreviewMode = 'success' | 'failure'; + +/** + * Resolve the dev-only uninstall UI preview mode from its env var. Returns null + * (preview off) in a packaged build regardless of the env value, so the + * non-destructive walkthrough can never fire in a shipped app. + */ +export function resolveDesktopUninstallUiPreviewMode( + raw: string | undefined, + isPackaged: boolean, +): DesktopUninstallUiPreviewMode | null { + if (isPackaged) return null; + if (raw === 'success' || raw === '1' || raw === 'true') return 'success'; + if (raw === 'failure' || raw === 'fail') return 'failure'; + return null; +} + /** Resolve `/Applications/OpenKnowledge.app` from Electron's main execPath. */ export function resolveAppBundleFromExecPath( execPath: string, @@ -193,7 +220,28 @@ function buildDesktopUninstallProjectRows( .join('\n'); } -const DESKTOP_UNINSTALL_PICKER_SCHEME = 'ok-desktop-uninstall:'; +// The custom-scheme protocol shared by every uninstall window (picker, feedback, +// notice) — each window's will-navigate parser matches inbound URLs against it. +const DESKTOP_UNINSTALL_SCHEME = 'ok-desktop-uninstall:'; + +/** + * One source of truth for the type scale + neutral chrome shared by every + * uninstall window (picker, feedback, progress, notices). Each screen is its + * own inline-HTML document with a self-contained ` + + +
+
+

Thanks for giving OpenKnowledge a try.

+

What you share is sent to the OpenKnowledge team.

+
+
+
+
+ Before you go, mind sharing why? +${buildDesktopUninstallFeedbackReasonRows()} +
+
+ + +
+
+ +
+ +
+
+ + +
+
+
+ + +`; +} + +/** + * Renderer text arriving at the main process: trim, drop blanks so an untouched + * field never counts as an answer, and clamp to the intake's field limits. + */ +function boundedFeedbackAnswer(raw: string | null, maxLength: number): string | undefined { + const trimmed = raw?.trim(); + if (trimmed === undefined || trimmed === '') return undefined; + return trimmed.slice(0, maxLength); +} + +/** + * Read the feedback window's answers off its private navigation URL. `null` + * means "not a feedback result" (the caller keeps waiting); an empty object + * means the user left without answering, which posts nothing but still + * proceeds with the uninstall. + */ +export function parseDesktopUninstallFeedbackUrl(url: string): UninstallFeedbackAnswers | null { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return null; + } + if (parsed.protocol !== DESKTOP_UNINSTALL_SCHEME) return null; + if (parsed.hostname === DESKTOP_UNINSTALL_FEEDBACK_SKIP_HOST) return {}; + if (parsed.hostname !== DESKTOP_UNINSTALL_FEEDBACK_SEND_HOST) return null; + + const reason = parsed.searchParams.get('reason'); + const note = boundedFeedbackAnswer( + parsed.searchParams.get('note'), + DESKTOP_UNINSTALL_FEEDBACK_NOTE_MAX_LEN, + ); + const email = boundedFeedbackAnswer( + parsed.searchParams.get('email'), + DESKTOP_UNINSTALL_FEEDBACK_EMAIL_MAX_LEN, + ); + return { + // A slug outside the taxonomy would file a ticket nothing can group by; + // dropping it keeps whatever the user actually wrote. + ...(isUninstallFeedbackReason(reason) ? { reason } : {}), + ...(note === undefined ? {} : { note }), + ...(email === undefined ? {} : { email }), + }; +} + +// --------------------------------------------------------------------------- +// Pre-cleanup confirm flow +// --------------------------------------------------------------------------- + +export interface DesktopUninstallConfirmStepDeps { + candidates: readonly DesktopUninstallProjectCandidate[]; + /** Resolves the projects to remove, or `null` when the user cancels. */ + showProjectPicker: ( + candidates: readonly DesktopUninstallProjectCandidate[], + ) => Promise; + /** Plain confirmation for installs with no known projects; `false` cancels. */ + showConfirmNotice: () => Promise; +} + +export type DesktopUninstallConfirmOutcome = + | { proceed: false } + | { proceed: true; projectPaths: string[] }; + +/** + * Everything between the menu click and the irreversible cleanup: get the + * uninstall confirmed on whichever surface fits the install. + * + * The confirm surfaces are the only place an uninstall can still be called off. + * Feedback is asked later — after a successful removal, see + * runDesktopUninstallOutcomeStep — so the survey only reaches people who left. + */ +export async function confirmDesktopUninstall( + deps: DesktopUninstallConfirmStepDeps, +): Promise { + let projectPaths: string[] = []; + if (deps.candidates.length > 0) { + const selected = await deps.showProjectPicker(deps.candidates); + if (selected === null) return { proceed: false }; + projectPaths = selected.map((candidate) => candidate.path); + } else if (!(await deps.showConfirmNotice())) { + return { proceed: false }; + } + return { proceed: true, projectPaths }; +} + +// --------------------------------------------------------------------------- +// Post-cleanup outcome flow +// --------------------------------------------------------------------------- + +export interface DesktopUninstallFeedbackStepDeps { + /** Show the feedback screen and resolve with whatever the user left. */ + collect: () => Promise; + appVersion: string; + platform?: string; + /** Injectable for tests; the real transport bounds its own wait. */ + submit?: (submission: UninstallFeedbackSubmission) => Promise; +} + +export type DesktopUninstallFeedbackStepOutcome = + | { status: 'skipped' } + | { status: 'submitted'; result: UninstallFeedbackResult } + | { status: 'failed'; error: unknown }; + +/** + * Ask the departing user why — the removal has already succeeded by now — and + * flush the answer before the flow reaches the finish screen and `app.quit()`: + * a fire-and-forget POST would be killed mid-flight in a packaged build. + * + * The window and the transport are both outside this module, so every failure + * comes back as an outcome instead of throwing: OpenKnowledge is already gone + * by this point and a courtesy question must never derail what follows. + */ +export async function runDesktopUninstallFeedbackStep( + deps: DesktopUninstallFeedbackStepDeps, +): Promise { + try { + const answers = await deps.collect(); + if (!hasUninstallFeedbackContent(answers)) return { status: 'skipped' }; + const submit = deps.submit ?? postUninstallFeedback; + const result = await submit({ + ...answers, + source: 'desktop_uninstall', + appVersion: deps.appVersion, + platform: deps.platform ?? process.platform, + }); + return { status: 'submitted', result }; + } catch (error) { + return { status: 'failed', error }; + } +} + +export interface DesktopUninstallOutcomeStepDeps { + /** How the cleanup script finished; the failure branch carries its own error. */ + cleanup: RunDesktopUninstallCleanupResult; + /** Asked only when cleanup succeeded, before the finish screen. */ + runFeedbackStep: () => Promise; + showCompletion: () => Promise; + /** Receives the narrowed failure so the notice can't be handed a blank error. */ + showFailure: (cleanup: { error: string }) => Promise; +} + +/** + * The screens after cleanup runs. Feedback is asked only on success — right + * after the uninstall the user came to do is done, and before the finish + * screen — so a failed (and possibly retried) uninstall is never surveyed. + */ +export async function runDesktopUninstallOutcomeStep( + deps: DesktopUninstallOutcomeStepDeps, +): Promise { + if (!deps.cleanup.ok) { + await deps.showFailure(deps.cleanup); + return; + } + await deps.runFeedbackStep(); + await deps.showCompletion(); +} + export function buildDesktopUninstallProgressHtml(): string { return ` @@ -470,17 +845,8 @@ export function buildDesktopUninstallProgressHtml(): string { Uninstalling OpenKnowledge @@ -547,11 +913,28 @@ export function readDesktopUninstallLogForDisplay( * `dialog.showMessageBox` renders `detail` at macOS's fixed small font, which * is what pushed these off NSAlert. */ +interface DesktopUninstallChecklistItem { + label: string; + detail?: string; + /** `true` = already done (✓); `false` = the one remaining action (○). */ + done: boolean; +} + export interface DesktopUninstallNoticeSpec { title: string; + /** One muted line under the title (e.g. "Almost done. Here's what's left."). */ + subtitle?: string; paragraphs: string[]; + /** A done/pending checklist rendered in the body, for the recap-plus-action screen. */ + checklist?: DesktopUninstallChecklistItem[]; /** Small muted line under the body (e.g. the cleanup log path). */ footnote?: string; + /** + * When set, renders a subtle link with this text that reveals the cleanup log + * in Finder. The path itself never enters the HTML — main holds it and reveals + * on the intercepted `reveal-log` navigation (see `onRevealLog`). + */ + logRevealLabel?: string; /** Monospace scrollable block (the cleanup log). */ log?: string; confirmLabel: string; @@ -577,23 +960,33 @@ export function desktopUninstallConfirmNotice(): DesktopUninstallNoticeSpec { export function desktopUninstallCompletionNotice(opts: { projectCount: number; - logPath: string; }): DesktopUninstallNoticeSpec { - const paragraphs = ['Your markdown content and authored skills were kept.']; - if (opts.projectCount > 0) { - paragraphs.push( - `OpenKnowledge was also removed from ${opts.projectCount} project${opts.projectCount === 1 ? '' : 's'}.`, - ); - } - paragraphs.push( - 'One step left: move OpenKnowledge.app to the Trash.', - 'Click Continue to show the app in Finder. OpenKnowledge will then quit.', - ); + const removedDetail = + opts.projectCount > 0 + ? `Cleaned up, including from ${opts.projectCount} project${opts.projectCount === 1 ? '' : 's'}.` + : 'Settings and integrations were cleaned up.'; + // A scannable checklist rather than prose: the two done items are glanceable + // reassurance, and the eye lands on the one pending item — the real action. return { title: 'OpenKnowledge files were removed', - paragraphs, - footnote: `Cleanup log: ${opts.logPath}`, - confirmLabel: 'Continue', + subtitle: "Almost done. Here's what happened and what's left.", + paragraphs: [], + checklist: [ + { + label: 'Kept your content', + detail: 'Markdown files and authored skills were left untouched.', + done: true, + }, + { label: 'Removed OpenKnowledge files', detail: removedDetail, done: true }, + { + label: 'Move OpenKnowledge.app to the Trash', + detail: + 'Reveal in Finder shows the app and quits OpenKnowledge, so you can drag it to the Trash.', + done: false, + }, + ], + logRevealLabel: 'Cleanup log', + confirmLabel: 'Reveal in Finder', }; } @@ -620,36 +1013,65 @@ export function desktopUninstallFailureNotice(opts: { }; } -/** Failure-path follow-up; the success notice folds this step in. */ +/** Failure-path follow-up; the success notice folds this step into its checklist. */ export function desktopUninstallFinalStepNotice(): DesktopUninstallNoticeSpec { + // Same last action as the success screen, so keep the copy + button aligned. return { title: 'One more step', paragraphs: [ - 'Move OpenKnowledge.app to the Trash to finish.', - 'Click Continue to show the app in Finder. OpenKnowledge will then quit.', + 'Reveal in Finder shows the app and quits OpenKnowledge, so you can drag it to the Trash.', ], - confirmLabel: 'Continue', + confirmLabel: 'Reveal in Finder', }; } -export function parseDesktopUninstallNoticeUrl(url: string): 'confirm' | 'cancel' | null { +export function parseDesktopUninstallNoticeUrl( + url: string, +): 'confirm' | 'cancel' | 'reveal-log' | null { let parsed: URL; try { parsed = new URL(url); } catch { return null; } - if (parsed.protocol !== DESKTOP_UNINSTALL_PICKER_SCHEME) return null; + if (parsed.protocol !== DESKTOP_UNINSTALL_SCHEME) return null; if (parsed.hostname === 'notice-confirm') return 'confirm'; if (parsed.hostname === 'notice-cancel') return 'cancel'; + // A non-terminal action: reveal the log in Finder without closing the notice. + if (parsed.hostname === 'notice-reveal-log') return 'reveal-log'; return null; } export function buildDesktopUninstallNoticeHtml(spec: DesktopUninstallNoticeSpec): string { const paragraphs = spec.paragraphs.map((text) => `

${htmlEscape(text)}

`).join('\n'); + const subtitle = + spec.subtitle === undefined ? '' : `

${htmlEscape(spec.subtitle)}

`; + const checklist = + spec.checklist === undefined + ? '' + : `
    +${spec.checklist + .map((item) => { + // The ✓ glyph is a shape channel (not colour alone); the visually-hidden + // status word carries the same state to a screen reader. + const marker = item.done + ? 'Done. ' + : 'To do. '; + const detail = + item.detail === undefined ? '' : `${htmlEscape(item.detail)}`; + return `
  1. ${marker}${htmlEscape(item.label)}${detail}
    2. `; + }) + .join('\n')} +
`; const logBlock = spec.log === undefined ? '' : `
${htmlEscape(spec.log)}
`; const footnote = spec.footnote === undefined ? '' : `

${htmlEscape(spec.footnote)}

`; + // The path never enters the HTML — the link only carries the intercepted + // action; main reveals the log it already knows the path to. + const logReveal = + spec.logRevealLabel === undefined + ? '' + : `

${htmlEscape(spec.logRevealLabel)}

`; const cancelButton = spec.cancelLabel === undefined ? '' @@ -669,15 +1091,7 @@ export function buildDesktopUninstallNoticeHtml(spec: DesktopUninstallNoticeSpec ${htmlEscape(spec.title)}