Repository navigation
Conversation
…/setup-tracking-thr_gt5eeahuym
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5605b1ff6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <Suspense fallback={null}> | ||
| <OnboardingFlow | ||
| initialStep="agent" | ||
| entry={seenBefore ? "replay" : "first_run"} |
There was a problem hiding this comment.
When a browser starts without the local marker, seenBefore is captured as false for the lifetime of this gate; observing a completion only writes to localStorage and never updates that state. If the user completes onboarding and then selects Replay setup guide without reloading, this line labels the replay as first_run and emits another onboarding_started, corrupting the first-run funnel. Update the in-memory marker, or reread the stored marker, when completion is observed.
Useful? React with 👍 / 👎.
| const navigation = useSidebarNavigation({ enabled: onboarded }).data; | ||
| return ( | ||
| onboarded && | ||
| navigation !== undefined && |
There was a problem hiding this comment.
When the user's only thread is archived, useSetupComplete() changes back to false even though onboarding and the first thread have already happened. listSidebarNavigationThreads() reads the sidebar bootstrap, whose server query explicitly requests only archived: false threads in apps/server/src/routes/projects.ts:230-233; downstream prompts using this supposedly historical signal will therefore reappear after the user clears their active sidebar. Store a persistent completion fact or use a targeted existence query that includes archived threads.
AGENTS.md reference: AGENTS.md:L32-L36
Useful? React with 👍 / 👎.
…/setup-tracking-thr_gt5eeahuym
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec507045c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| experimental_recordTelemetryEvent( | ||
| args: SystemRecordTelemetryEventArgs, | ||
| ): Promise<SystemRecordTelemetryEventResult>; |
There was a problem hiding this comment.
Because PluginBbSdk inherits BbSdk.system, this addition publishes experimental_recordTelemetryEvent to plugin authors. The audit document and backend reference were updated, but plugins/plugin-api-docs/src/surfaces.ts contains no corresponding SDK symbol, so the designated Plugin Guide omits the new public surface; add it to the bb-sdk surface's apiSymbols.
AGENTS.md reference: AGENTS.md:L30-L30
Useful? React with 👍 / 👎.
| }, [agentStates, agentStatesQuery.isError, entry]); | ||
| const startReportedRef = useRef(entry !== "first_run"); | ||
| useEffect(() => { | ||
| if (startReportedRef.current || agentStates === undefined) return; |
There was a problem hiding this comment.
On a first run with no primary host, this provider query is disabled and agentStates remains undefined; the same is true after a terminal query failure. If the user then skips or completes the guide, the step and finished events are emitted but onboarding_started never is, producing sessions that finish without starting in the new funnel. Emit a start with an unknown state or otherwise handle the unavailable/error path rather than waiting indefinitely for provider data.
Useful? React with 👍 / 👎.
|
Superseded by #5412, independently based on main. The complete feature diff is byte-for-byte identical; review, QA results and screenshots are retained. This branch is preserved for the existing notification-card layer. |
## Human comments ## What was wrong Setup progress, agent readiness, sign-in results and rejected new-thread submissions were not observable in anonymous telemetry. ## What changed - Record anonymous setup progress, readiness, sign-in outcomes and new-thread submission outcomes through the opt-out-controlled telemetry route. - Accept fixed enums, booleans and explicit unknown values, without identifying fields or free-text error details. - Expose `useSetupComplete` and the experimental SDK method, and update configuration documentation. - Describe collection by usage category in Settings and the privacy page, while retaining purpose, identifiers, system details and opt-out instructions. Supersedes #5394. Independently based on main; no shared-component dependency. ## How you verified - [PR CI](https://github.com/get-bb/bb/actions/runs/38328259479) passed on `3bd09c3262585b764d77cca6761fc2328a21c3bd`. [Full CI](https://github.com/get-bb/bb/actions/runs/38328255967) also passed after retrying the Windows provider-replay job. - Scoped review has no P0/P1 findings; the subsequent copy-only change preserves the reviewed telemetry implementation. - Focused exact-head QA PASS: Chrome for Testing 154.0.8037.92, Settings and privacy, desktop and phone widths; real Safari 26.5.2, privacy. - Initial rendering, settled layout, hard reload and client navigation passed. Chrome reported no page or console errors during disclosure checks. - Exact-head telemetry delivery passed through the production-mode sender to an isolated local sink; the HogQL query parsed and ran. <details> <summary>Exact-head telemetry delivery QA</summary> Chrome for Testing 154.0.8037.92 exercised the branch web app at `3bd09c3262585b764d77cca6761fc2328a21c3bd` with fresh app data, telemetry enabled and the real production-mode server sender. - The sink received each action once: all four steps reached, Agent/Devices completed, Projects/Plugins skipped, and setup finished. - Readiness was observed from real installed agents. Checking again emitted no duplicate readiness event. - A real Codex sign-in command opened its sign-in guide; cancelling emitted one `started` and one `cancelled` outcome. - Temporarily disabling the selected provider before forwarding an unchanged submit request produced a real `409 provider_disabled`; the draft remained intact. - Re-enabling the provider produced `201`, one accepted outcome and a real Claude Code reply of “OK”. Each submission emitted one started outcome. - Captured payloads contained only the random install ID, standard system properties and the expected event fields; no identifying fields or free text. The default `0.0.0-dev` version intentionally disables the sender. This pass used `0.0.0-qa-onboarding-3bd09c3` and a transport-only preload redirecting the unchanged capture body to a loopback sink. That version is not excluded by the dashboard's exact dev-version filter, so no test events were sent to PostHog. Product source and event handlers were unchanged. Growth analytics independently executed the reached/ready/signed-in/submitted HogQL query against PostHog with no errors or warnings. This establishes query/schema compatibility; new-event ingestion after release remains unverified. Completing a new provider authentication was outside this pass: the real attempt was cancelled, and submission used an already-ready provider. </details> <details> <summary>Retained boundary checks and follow-up</summary> The unchanged implementation retains earlier API checks accepting allowed enum/null values and rejecting unknown enums, identifying extra fields and free text. Earlier sign-in startup-failure coverage used fixtures and was not a delivery check. Known P2 follow-up: abandonment during pending sign-in startup may be counted as failed when terminal creation later rejects. </details> Fresh full-viewport screenshots compare merge base `d34a3d219193d37bf4bb31b6b345dd113800c3f1` with exact head `3bd09c3262585b764d77cca6761fc2328a21c3bd`, using the same isolated fixture, routes and scroll anchors. Desktop: 1440 × 900 CSS pixels; phone: 390 × 844; both at 2×. | Surface | Before | After | | --- | --- | --- | | Usage-data setting / desktop |  |  | | Privacy policy / desktop |  |  | | Usage-data setting / phone |  |  | | Privacy policy / phone |  |  | The user approved the simplified disclosure screenshots at `3bd09c3` via Growth. Merge approval remains separate. BB-Thread-ID: thr_gt5eeahuym BB-Thread: [Onboarding](https://brsbl.getbb.app/projects/proj_khiw2za95v/threads/thr_gt5eeahuym) > AGENT GENERATED
Human comments
What was wrong
Setup progress, agent readiness, sign-in results and rejected new-thread submissions were not observable in anonymous telemetry.
What changed
Record strict anonymous setup events through the existing telemetry route and experimental SDK method. Distinguish first run from replay, observe aggregate installed/ready health and guide sign-in outcomes, and record started/accepted/failed new-thread submissions. Add
useSetupCompleteand update the existing Settings and privacy disclosures.Properties are fixed enums and booleans, with null for unknown health. No provider IDs, account details, prompts, paths, terminal output or error text are added. Existing opt-out applies. Command exit does not establish sign-in readiness; accepted submission does not establish a completed provider turn. Extracted from #5222 without rewriting its history.
How you verified
ec507045c34fb79066c774cecd09a21b05a3987c. Scoped security, correctness, architecture and performance review found no P0/P1. The main merge preserves the reviewed functional implementation.Before: parent
7af427b8e457c69069631adf27feebbbe2bfa0cd. After:ec507045c34fb79066c774cecd09a21b05a3987c. Same isolated fixture, route and scroll anchor, full viewport at 1440×900 or 390×844 CSS pixels, 2× density.User screenshot sign-off is pending.
BB-Thread-ID: thr_gt5eeahuym
BB-Thread: Onboarding