-
Notifications
You must be signed in to change notification settings - Fork 452
feat: [ENG-2621] serve analytics disclosure markdown via daemon + canonicalise privacy URL #752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: proj/analytics-system-tool-mode
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import type {ITransportServer} from '../../../core/interfaces/transport/i-transport-server.js' | ||
|
|
||
| import { | ||
| type AnalyticsDisclosureResponse, | ||
| AnalyticsEvents, | ||
| } from '../../../../shared/transport/events/analytics-events.js' | ||
| import {loadAnalyticsDisclosureText} from '../../../../shared/utils/load-analytics-disclosure.js' | ||
|
|
||
| export interface AnalyticsDisclosureHandlerDeps { | ||
| readonly loadDisclosure?: () => Promise<string> | ||
| readonly transport: ITransportServer | ||
| } | ||
|
|
||
| /** | ||
| * Serves `analytics:getDisclosure` so the local web UI can render the same | ||
| * canonical disclosure markdown shown by the CLI consent prompt | ||
| * (`brv settings set analytics.share true`). Single source of truth is | ||
| * `src/shared/assets/analytics-disclosure.md`, read via | ||
| * `loadAnalyticsDisclosureText()`. | ||
| */ | ||
| export class AnalyticsDisclosureHandler { | ||
| private readonly loadDisclosure: () => Promise<string> | ||
| private readonly transport: ITransportServer | ||
|
|
||
| public constructor(deps: AnalyticsDisclosureHandlerDeps) { | ||
| this.loadDisclosure = deps.loadDisclosure ?? loadAnalyticsDisclosureText | ||
| this.transport = deps.transport | ||
| } | ||
|
|
||
| public setup(): void { | ||
| this.transport.onRequest<void, AnalyticsDisclosureResponse>(AnalyticsEvents.GET_DISCLOSURE, async () => { | ||
| const markdown = await this.loadDisclosure() | ||
| return {markdown} | ||
| }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1 @@ | ||
| /** | ||
| * Public privacy policy URL for ByteRover CLI analytics. | ||
| * Placeholder until M1.5 lands the canonical docs page; reviewers should | ||
| * update this constant when the byterover-docs URL is finalized. | ||
| */ | ||
| export const PRIVACY_POLICY_URL = 'https://byterover.dev/privacy' | ||
| export const PRIVACY_POLICY_URL = 'https://www.byterover.dev/services/privacy' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import {queryOptions, useQuery} from '@tanstack/react-query' | ||
|
|
||
| import type {QueryConfig} from '../../../lib/react-query' | ||
|
|
||
| import { | ||
| type AnalyticsDisclosureResponse, | ||
| AnalyticsEvents, | ||
| } from '../../../../shared/transport/events/analytics-events.js' | ||
| import {useTransportStore} from '../../../stores/transport-store' | ||
|
|
||
| export const getAnalyticsDisclosure = (): Promise<AnalyticsDisclosureResponse> => { | ||
| const {apiClient} = useTransportStore.getState() | ||
| if (!apiClient) return Promise.reject(new Error('Not connected')) | ||
| return apiClient.request<AnalyticsDisclosureResponse, void>(AnalyticsEvents.GET_DISCLOSURE) | ||
| } | ||
|
|
||
| export const getAnalyticsDisclosureQueryOptions = () => | ||
| queryOptions({ | ||
| queryFn: getAnalyticsDisclosure, | ||
| queryKey: ['analyticsDisclosure'], | ||
| }) | ||
|
Comment on lines
+17
to
+21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick (non-blocking): the PR description claims the webui will refetch the disclosure on tab refocus ("TanStack Query default + override on the globalConfig query"), but the global
|
||
|
|
||
| type UseGetAnalyticsDisclosureOptions = { | ||
| queryConfig?: QueryConfig<typeof getAnalyticsDisclosureQueryOptions> | ||
| } | ||
|
|
||
| export const useGetAnalyticsDisclosure = ({queryConfig}: UseGetAnalyticsDisclosureOptions = {}) => | ||
| useQuery({ | ||
| ...getAnalyticsDisclosureQueryOptions(), | ||
| ...queryConfig, | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,11 +5,11 @@ import {ChevronDown, ExternalLink, ShieldCheck} from 'lucide-react' | |
| import {useState} from 'react' | ||
| import {toast} from 'sonner' | ||
|
|
||
| import {PRIVACY_POLICY_URL} from '../../../../shared/constants/privacy.js' | ||
| import {formatError} from '../../../lib/error-messages' | ||
| import {noop} from '../../../lib/noop' | ||
| import {useGetGlobalConfig} from '../api/get-global-config' | ||
| import {useSetAnalytics} from '../api/set-analytics' | ||
| import {ANALYTICS_PRIVACY_URL} from '../constants' | ||
| import {DisableConfirmDialog} from './disable-confirm-dialog' | ||
| import {DisclosureDetails} from './disclosure-details' | ||
| import {EnableConfirmDialog} from './enable-confirm-dialog' | ||
|
|
@@ -103,12 +103,12 @@ export function AnalyticsPanel() { | |
|
|
||
| <a | ||
| className="text-foreground/80 hover:text-foreground inline-flex items-center gap-2 border-t px-5 py-3 text-sm transition-colors" | ||
| href={ANALYTICS_PRIVACY_URL} | ||
| href={PRIVACY_POLICY_URL} | ||
| rel="noopener noreferrer" | ||
| target="_blank" | ||
| > | ||
| <ExternalLink className="size-3.5 text-primary" /> | ||
| <span className="text-primary">docs.byterover.dev/privacy</span> | ||
| <span className="text-primary">byterover.dev/services/privacy</span> | ||
| </a> | ||
|
Comment on lines
104
to
112
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (non-blocking): the visible label |
||
| </div> | ||
| )} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,38 @@ | ||
| import {ANALYTICS_DISCLOSURE_SECTIONS} from '../constants' | ||
| import {Skeleton} from '@campfirein/byterover-packages/components/skeleton' | ||
|
|
||
| import {formatError} from '../../../lib/error-messages' | ||
| import {noop} from '../../../lib/noop' | ||
| import {MarkdownView} from '../../context/components/markdown-view' | ||
| import {useGetAnalyticsDisclosure} from '../api/get-analytics-disclosure' | ||
|
|
||
| export function DisclosureDetails() { | ||
| return ( | ||
| <div className="grid grid-cols-1 gap-x-6 gap-y-5 md:grid-cols-2"> | ||
| {ANALYTICS_DISCLOSURE_SECTIONS.map((section) => { | ||
| const Icon = section.icon | ||
| return ( | ||
| <div className="flex flex-col gap-2" key={section.label}> | ||
| <Icon className="size-4 text-muted-foreground" strokeWidth={1.75} /> | ||
| <div className="flex flex-col gap-1"> | ||
| <span className="text-foreground text-[0.6875rem] font-semibold tracking-wider"> | ||
| {section.label} | ||
| </span> | ||
| <p className="text-muted-foreground text-[0.8125rem] leading-relaxed">{section.body}</p> | ||
| </div> | ||
| </div> | ||
| ) | ||
| })} | ||
| </div> | ||
| ) | ||
| const {data, error, isError, isLoading, refetch} = useGetAnalyticsDisclosure() | ||
|
|
||
| if (isLoading) { | ||
| return ( | ||
| <div className="flex flex-col gap-2"> | ||
| <Skeleton className="h-4 w-32" /> | ||
| <Skeleton className="h-4 w-full" /> | ||
| <Skeleton className="h-4 w-3/4" /> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| if (isError) { | ||
| return ( | ||
| <p className="text-destructive text-sm"> | ||
| ✗ {formatError(error, 'Failed to load disclosure')} | ||
| {' · '} | ||
| <button | ||
| className="underline underline-offset-2" | ||
| onClick={() => refetch().catch(noop)} | ||
| type="button" | ||
| > | ||
| retry | ||
| </button> | ||
| </p> | ||
| ) | ||
| } | ||
|
|
||
| return <MarkdownView content={data?.markdown ?? ''} /> | ||
| } |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import {expect} from 'chai' | ||
|
|
||
| import {AnalyticsDisclosureHandler} from '../../../../../../src/server/infra/transport/handlers/analytics-disclosure-handler.js' | ||
| import {AnalyticsEvents} from '../../../../../../src/shared/transport/events/analytics-events.js' | ||
| import {createMockTransportServer} from '../../../../../helpers/mock-factories.js' | ||
|
|
||
| type DisclosureHandler = (data: unknown, clientId: string) => Promise<{markdown: string}> | ||
|
|
||
| describe('AnalyticsDisclosureHandler', () => { | ||
| it('registers a handler for analytics:getDisclosure on setup()', () => { | ||
| const transport = createMockTransportServer() | ||
| new AnalyticsDisclosureHandler({loadDisclosure: async () => 'noop', transport}).setup() | ||
| expect(transport._handlers.has(AnalyticsEvents.GET_DISCLOSURE)).to.equal(true) | ||
| }) | ||
|
|
||
| it('returns the markdown loaded from the injected loader', async () => { | ||
| const transport = createMockTransportServer() | ||
| const markdown = '# Disclosure\n\nLorem.' | ||
| new AnalyticsDisclosureHandler({loadDisclosure: async () => markdown, transport}).setup() | ||
|
|
||
| const handler = transport._handlers.get(AnalyticsEvents.GET_DISCLOSURE) as DisclosureHandler | ||
| const result = await handler(undefined, 'client-1') | ||
|
|
||
| expect(result).to.deep.equal({markdown}) | ||
| }) | ||
|
|
||
| it('propagates loader errors so the daemon does not silently serve empty disclosure', async () => { | ||
| const transport = createMockTransportServer() | ||
| const boom = new Error('ENOENT') | ||
| new AnalyticsDisclosureHandler({ | ||
| async loadDisclosure() { | ||
| throw boom | ||
| }, | ||
| transport, | ||
| }).setup() | ||
|
|
||
| const handler = transport._handlers.get(AnalyticsEvents.GET_DISCLOSURE) as DisclosureHandler | ||
| await handler(undefined, 'client-1').then( | ||
| () => expect.fail('expected promise to reject'), | ||
| (error: Error) => expect(error).to.equal(boom), | ||
| ) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| import {expect} from 'chai' | ||
| import {createSandbox, type SinonSandbox, type SinonStub} from 'sinon' | ||
|
|
||
| import type {BrvApiClient} from '../../../../../../src/webui/lib/api-client.js' | ||
|
|
||
| import {AnalyticsEvents} from '../../../../../../src/shared/transport/events/analytics-events.js' | ||
| import {getAnalyticsDisclosure} from '../../../../../../src/webui/features/analytics/api/get-analytics-disclosure.js' | ||
| import {useTransportStore} from '../../../../../../src/webui/stores/transport-store.js' | ||
|
|
||
| describe('getAnalyticsDisclosure', () => { | ||
| let sandbox: SinonSandbox | ||
| let request: SinonStub | ||
|
|
||
| beforeEach(() => { | ||
| sandbox = createSandbox() | ||
| request = sandbox.stub() | ||
| useTransportStore.setState({ | ||
| apiClient: {on: sandbox.stub(), request} as unknown as BrvApiClient, | ||
| }) | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| sandbox.restore() | ||
| useTransportStore.setState({apiClient: null}) | ||
| }) | ||
|
|
||
| it('emits analytics:getDisclosure with no payload', async () => { | ||
| request.resolves({markdown: '# Disclosure'}) | ||
| await getAnalyticsDisclosure() | ||
| expect(request.firstCall.args[0]).to.equal(AnalyticsEvents.GET_DISCLOSURE) | ||
| }) | ||
|
|
||
| it('returns the markdown body from the daemon response', async () => { | ||
| request.resolves({markdown: '# Title\n\nBody.'}) | ||
| const result = await getAnalyticsDisclosure() | ||
| expect(result).to.deep.equal({markdown: '# Title\n\nBody.'}) | ||
| }) | ||
|
|
||
| it('rejects when the transport is not connected', async () => { | ||
| useTransportStore.setState({apiClient: null}) | ||
| await getAnalyticsDisclosure().then( | ||
| () => expect.fail('expected promise to reject'), | ||
| (error: Error) => expect(error.message).to.equal('Not connected'), | ||
| ) | ||
| }) | ||
| }) |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick (non-blocking):
AnalyticsDisclosureResponseSchemais declared inanalytics-events.tswithmarkdown: z.string().min(1)but is only used here for type inference, neversafeParse'd on the response. Sister handlerAnalyticsListHandlervalidates inbound payloads withAnalyticsListRequestSchema.safeParse; this one has no inbound payload to validate. IfloadDisclosure()ever returns""(truncated read, bad bundle), the wire response will technically violate the.min(1)schema and the webui will silently render an empty<MarkdownView/>. Cheap defense:if (!markdown) throw new Error('disclosure missing')— keeps the failure mode visible (the existing error-state UI inDisclosureDetails) instead of "blank panel, no toast." Not blocking since the bundled asset is checked into git; flagging because the test on line 27-42 of the handler test already establishes the propagate-errors contract you'd be leaning on.