feat(onboarding): first-run funnel telemetry - #1049
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds first-run onboarding telemetry across the TUI, gateway authorization, worker lifecycle, activation tools, prompt handling, and telemetry transport. It adds event contracts, session tracking, launch correlation, abandonment reporting, privacy-safe fields, and validation coverage. ChangesFirst-run onboarding telemetry
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
full receipts (1 session)
orchestrator ·
|
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
8 similar comments
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
…ness The Part 2 gate was triggered by useReady(), which is `connected() || setupComplete()`. `connected()` flips as soon as a provider lands in sync data — and the BYOK confirm handlers do exactly that inside `await sync.bootstrap()`, before going on to open the model picker (dialog-provider ApiMethod and CodeMethod). So for every BYOK provider the gate mounted mid-handler, the handler's own `dialog.replace(<DialogModel/>)` destroyed it a moment later, and the one-shot latch meant it never came back. `/onboard-connect` was never submitted, which made activation_menu_shown, activation_job_selected, first_job_completed, sample_setup_completed and first_prompt_sent unreachable on the majority onboarding path — while onboarding_completed and scan_gate_shown were still reported for a gate the user never saw. The gateway path was unaffected because its success branch does not replace the dialog, which is why this survived manual testing and the Big Pickle E2E. Now driven by setup completion alone, which is only set once a model is genuinely chosen — the model picker, the Big Pickle accept path, and the gateway auto-select. That is also what the spec means by "a model is ready". Reported in consensus review of #1049 (CRITICAL), flagged independently by two reviewers, with the ordering verified against sync.tsx: the store write is inside `batch()` within an awaited promise chain, two `.then()` hops before bootstrap() resolves, so the effect flushes before the dialog.replace under either effect-scheduling model. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
ecd94c8 to
447f8b6
Compare
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
2 similar comments
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Addresses the findings from the review that fall inside this PR. MAJOR — markSetupComplete() ran before the model-availability check (dialog-provider). The branch below it deliberately refuses to claim success when the gateway connects but offers nothing usable, and said so in a comment — but completion had already been marked, so telemetry reported a finished onboarding while the user was told to go pick a model. Now marked where the model is actually set. This matters more since the scan gate moved onto the same signal. MAJOR — the flushTimer leak. doInit() assigned a new interval without clearing the previous handle, and shutdown() only ever clears the current one, so a second doInit() stranded a timer for the life of the process. doShutdown also nulled initPromise unconditionally, discarding a doInit() that init() had chained onto the in-flight shutdown. Both fixed; see the note in doShutdown on why only the first is covered by a test. MAJOR — instance_connected and onboarding_abandoned could both be reported for one launch. The gateway success events are emitted on the worker thread while abandonment state is main-thread-owned, so a user who finished in the browser and quit before the TUI observed the new provider was reported as abandoning at gateway_auth. The exit path now checks whether credentials landed. MAJOR — environment_scan_completed fired on every project_scan, including /discover and any model-initiated call, so a funnel query could exceed 100% conversion. Now guarded on isOnboardingSession. MINOR — command.execute.before created a tracking record for every slash command in every session, churning the capped map and evicting genuine onboarding sessions, after which their remaining activation events were silently dropped. noteCommandSubmission now only touches sessions already tracked, and /onboard-connect marks the session before flagging its own submission. MINOR — the cross-package event parity test that two comments claimed but which did not exist. Now a compile-time assertion pinning the packages/tui event union to the Telemetry variants; verified by renaming a property and watching the build fail. Required exporting the context subpath from packages/tui. MINOR — a raw HOME path could reach LLM-visible tool output on a sample-setup failure. Paths are masked in `output`; the full message stays in metadata, which the model never sees. MINOR — documented why sample_setup_completed uses a success boolean instead of a _failed sibling event. NIT — launchId() no longer writes process.env. The worker receives the id explicitly through WorkerOptions.env, so the write only leaked it into every subprocess the CLI spawns. Also removes the stray `// scratch` line at the end of cli/cmd/tui.ts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
…ness The Part 2 gate was triggered by useReady(), which is `connected() || setupComplete()`. `connected()` flips as soon as a provider lands in sync data — and the BYOK confirm handlers do exactly that inside `await sync.bootstrap()`, before going on to open the model picker (dialog-provider ApiMethod and CodeMethod). So for every BYOK provider the gate mounted mid-handler, the handler's own `dialog.replace(<DialogModel/>)` destroyed it a moment later, and the one-shot latch meant it never came back. `/onboard-connect` was never submitted, which made activation_menu_shown, activation_job_selected, first_job_completed, sample_setup_completed and first_prompt_sent unreachable on the majority onboarding path — while onboarding_completed and scan_gate_shown were still reported for a gate the user never saw. The gateway path was unaffected because its success branch does not replace the dialog, which is why this survived manual testing and the Big Pickle E2E. Now driven by setup completion alone, which is only set once a model is genuinely chosen — the model picker, the Big Pickle accept path, and the gateway auto-select. That is also what the spec means by "a model is ready". Reported in consensus review of #1049 (CRITICAL), flagged independently by two reviewers, with the ordering verified against sync.tsx: the store write is inside `batch()` within an awaited promise chain, two `.then()` hops before bootstrap() resolves, so the effect flushes before the dialog.replace under either effect-scheduling model. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
Addresses the findings from the review that fall inside this PR. MAJOR — markSetupComplete() ran before the model-availability check (dialog-provider). The branch below it deliberately refuses to claim success when the gateway connects but offers nothing usable, and said so in a comment — but completion had already been marked, so telemetry reported a finished onboarding while the user was told to go pick a model. Now marked where the model is actually set. This matters more since the scan gate moved onto the same signal. MAJOR — the flushTimer leak. doInit() assigned a new interval without clearing the previous handle, and shutdown() only ever clears the current one, so a second doInit() stranded a timer for the life of the process. doShutdown also nulled initPromise unconditionally, discarding a doInit() that init() had chained onto the in-flight shutdown. Both fixed; see the note in doShutdown on why only the first is covered by a test. MAJOR — instance_connected and onboarding_abandoned could both be reported for one launch. The gateway success events are emitted on the worker thread while abandonment state is main-thread-owned, so a user who finished in the browser and quit before the TUI observed the new provider was reported as abandoning at gateway_auth. The exit path now checks whether credentials landed. MAJOR — environment_scan_completed fired on every project_scan, including /discover and any model-initiated call, so a funnel query could exceed 100% conversion. Now guarded on isOnboardingSession. MINOR — command.execute.before created a tracking record for every slash command in every session, churning the capped map and evicting genuine onboarding sessions, after which their remaining activation events were silently dropped. noteCommandSubmission now only touches sessions already tracked, and /onboard-connect marks the session before flagging its own submission. MINOR — the cross-package event parity test that two comments claimed but which did not exist. Now a compile-time assertion pinning the packages/tui event union to the Telemetry variants; verified by renaming a property and watching the build fail. Required exporting the context subpath from packages/tui. MINOR — a raw HOME path could reach LLM-visible tool output on a sample-setup failure. Paths are masked in `output`; the full message stays in metadata, which the model never sees. MINOR — documented why sample_setup_completed uses a success boolean instead of a _failed sibling event. NIT — launchId() no longer writes process.env. The worker receives the id explicitly through WorkerOptions.env, so the write only leaked it into every subprocess the CLI spawns. Also removes the stray `// scratch` line at the end of cli/cmd/tui.ts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
8e369a8 to
6843e2d
Compare
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Majors: 1. The "never re-init across an in-flight shutdown" guard was unreachable. It sat inside `if (!initPromise)`, but doShutdown() leaves initPromise set until after `await flush()` — so during the exact window it protected against, init() took the other branch, returned the stale resolved promise, and the caller tracked into a buffer doShutdown() then emptied. The guard now runs first and chains onto shutdownPromise, keeping generations separate. The event loss was live. 2. flush() itself was not serialized. A timer flush could already have spliced the buffer and be awaiting fetch when shutdown began; shutdown then saw an empty buffer, reset state and returned, and the worker terminated mid-request — or the timer's retry re-inserted events into a buffer that had just been cleared. All flushes now chain through one promise, and shutdown drains it first. 3. The picker's double-submit latch could brick the first-run dialog. It was claimed before dispatching, and connectProvider silently no-ops for a provider the server filtered out via enabled_providers/disabled_providers — so every later key returned early, on the first-run gate, before the user had a model. The latch is now claimed only once an action actually dispatched. 4. Onboarding-taxonomy events fired from ordinary returning-user paths: /connect emitted provider_selected, /model emitted the Big Pickle pair, and any later sample_setup emitted sample_setup_completed. All are now gated on an active first run (TUI) or an onboarding session (sample_setup), matching what was already done for environment_scan_completed. model_picker_shown stays ungated because it carries a trigger and is already distinguishable. 5. The previous commit instrumented only READY rows. READY means "already holds valid credentials", which on a genuine first run is empty — so the long-tail providers the commit was written for were all missed. NEEDS-SETUP rows and the catalogue's Big Pickle row now emit too. 6. via_search was hardcoded true, but the catalogue opens from four places and only one is the search row. The BYOK flow — the most common non-gateway first run — was being recorded as having gone through search, which is precisely the distinction the field exists to make. Now a prop, set only by openFullCatalog. 7. first_job_completed could name a different job than activation_job_selected, since the record tracked booleans but never which job was chosen. It now stores the selected job and completes only that one. isJobCompletion also required `!== false`, counting missing metadata as success; now `=== true`. 8. redactPaths both over- and under-redacted. A space in the character class made matches run past the path and eat the rest of the sentence; single-segment paths like /root — which rejectUnsafeHome emits verbatim — were never matched; and names with apostrophes leaked fragments. Replaced with known-value substitution first, then a conservative pattern that stops at whitespace. Verified against every case in the review. 9. The worker's telemetry flush sat ahead of core cleanup and took a fixed 2s of a 5s budget shared with the trace drain, risking terminate() cutting off instance disposal. Moved after disposal and server stop, on the residual budget. Minors: docs/comment contradictions about the scan guard (10); provider_selected docs row updated for the new shape, including that search emits it twice for one user (11); parity assertion made two-way — the one-way version let a removed TUI event compile (12); prototype-chain lookup in the provider map, where a custom provider named `constructor` bypassed the allowlist (13); once-per-session claim on environment_scan_completed (14); ambient session resolved before the await rather than after (15); scan-gate dismissal now reports `dismissed` instead of emitting nothing (17); session eviction prefers non-onboarding records (18); cachedLaunchId cleared on shutdown (21). Test fixture: the picker tests served an empty provider list, which is exactly the degenerate state finding 3 describes — they passed while asserting nothing about whether the row worked. They now serve real providers, and two cases were added: a filtered-out row must not brick the dialog, and outside a first run the picker records an impression but no choice. Not addressed: 16 (stale credential file suppressing abandonment) and 20 (abandonedEmitted set before emit) both need emit() to report success, which changes its documented never-fails contract; 19 (allowlist drift) needs a source of truth for built-in providers that does not currently exist; 22 is a comment inaccuracy about env inheritance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| // errors and silently under-reports the "CLI shipped without its assets" case. | ||
| if (OnboardingTelemetry.isOnboardingSession(ctx.sessionID)) | ||
| void OnboardingTelemetry.emit({ | ||
| type: "sample_setup_completed", |
There was a problem hiding this comment.
SUGGESTION: Indentation drift inside the emit object literal
When the three emit(...) calls were wrapped in if (OnboardingTelemetry.isOnboardingSession(ctx.sessionID)) (this block and the matching ones at lines 179 and 210), the type: line kept its deeper indent while its siblings settled at 10 spaces, so it sits 2 spaces further out than success / models / tables / reused directly beneath it.
| type: "sample_setup_completed", | |
| type: "sample_setup_completed", |
The same drift appears at lines 179 and 210.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Fixed in fe6ac08 — real artifact of the script that rewrapped these three emit(...) calls inside the isOnboardingSession gate. All three sites re-indented.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge
Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous Review Summaries (6 snapshots, latest commit 8e6d52f)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 8e6d52f)Status: 1 Issue Found | Recommendation: Address before merge
Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit b66bbe9)Status: 1 Issue Found | Recommendation: Address before merge
Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous review (commit 6828ef1)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (8 files)
Fix these issues in Kilo Cloud Previous review (commit 3875d00)Status: No Issues Found | Recommendation: Merge Incremental pass over commit What was re-checked (sound — no issues)
Files Reviewed (5 files)
Previous review (commit a949fdc)Status: No Issues Found | Recommendation: Merge Incremental pass over the two follow-up commits ( The earlier SUGGESTION (indentation drift in the What was re-checked (sound — no issues)
Files Reviewed (15 files)
Previous review (commit 1c21544)Status: 1 Suggestion Found | Recommendation: Non-blocking — merge Overview
Issue Details (click to expand)SUGGESTION
What was checked (sound — no issues)Reviewed against the repo's known landmines:
Files Reviewed (22 files)
Reviewed by glm-5.2 · Input: 50.5K · Output: 7.8K · Cached: 389.3K Review guidance: REVIEW.md from base branch |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/altimate/plugin/onboarding-telemetry.ts">
<violation number="1" location="packages/opencode/src/altimate/plugin/onboarding-telemetry.ts:94">
P2: `first_prompt_sent` can be lost after a failed slash command because this pending flag is set before execution and has no failure cleanup. Tie the marker to the synthetic prompt or clear it when command execution aborts so a later actual user message is not suppressed.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/opencode/src/altimate/tools/sample-setup.ts (1)
99-136: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRedact filesystem paths in the "sample source missing" branch too.
This branch embeds
messagedirectly intoguidance, andguidancegoes straight intooutput(the model-facing text) at line 134, with noredactPaths()call.readSampleVersionAt(line 315-323, unchanged) throws an error that embedsmanifestPath, a real filesystem path that can contain the OS username (for example an ENOENT message fromfs.readFileSync).resolveSampleSourcecan plausibly throw similar path-bearing errors.The materialize-failure branch at lines 206-230 already applies
redactPaths(message)for exactly this reason, with an explicit comment thatoutput"is sent to the provider on every later turn." This branch sends the same class of data unredacted.Apply the same treatment here.
🔒 Proposed fix to redact the underlying error message
const message = err instanceof Error ? err.message : String(err) const guidance = `Could not locate the shipped starter sample source. This usually means the CLI ` + `was installed without its wrapper package assets. Reinstall with: ` + `\`npm i -g `@altimateai/altimate-code`@latest\`\n\n` + - `Underlying error: ${message}` + `Underlying error: ${redactPaths(message)}`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/altimate/tools/sample-setup.ts` around lines 99 - 136, Redact filesystem paths in the sample-source resolution failure before exposing the error through model-facing output. In the catch block surrounding resolveSampleSource/readSampleVersionAt, pass the underlying error text through the existing redactPaths utility before assigning it to message or interpolating it into guidance, while preserving the metadata error handling and status output behavior.
🧹 Nitpick comments (4)
packages/opencode/src/altimate/telemetry/onboarding.ts (1)
308-316: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the bottom-of-file self-reexport.
The module uses flat top-level exports, which is correct. It does not end with the self-reexport the repository convention requires.
♻️ Proposed change
export function resetForTest() { furthestStage = undefined completed = false abandonedEmitted = false funnelStarted = false sessions.clear() } + +export * as OnboardingTelemetry from "./onboarding" // altimate_change endAs per coding guidelines: "Do not use
export namespace Foo { ... }for module organization. Use flat top-level exports and a bottom-of-file self-reexport such asexport * as Foo from "./foo"."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/altimate/telemetry/onboarding.ts` around lines 308 - 316, Append the repository-standard bottom-of-file self-reexport to the onboarding module, preserving its existing flat top-level exports and resetForTest implementation. Use the module’s expected namespace alias and self-reference pattern without introducing an export namespace block.Source: Coding guidelines
packages/opencode/src/altimate/telemetry/index.ts (1)
939-949: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow the
classifyProviderreturn type to the event enum.
provider_selected.provideris a closed enum, butclassifyProviderreturns{ provider: string }. A wrong value inCURATED_PROVIDER_ENUMwould still type-check at the call sites. Derive the type from the event union so drift fails the build.♻️ Proposed typing change
+ type ProviderEnum = Extract<Event, { type: "provider_selected" }>["provider"] + /** The curated picker's own rows map to named enum values; everything else is `other`. */ - const CURATED_PROVIDER_ENUM: Record<string, string> = Object.assign(Object.create(null), { + const CURATED_PROVIDER_ENUM: Record<string, ProviderEnum> = Object.assign(Object.create(null), { "altimate-backend": "altimate_gateway", anthropic: "anthropic", openai: "openai", google: "google", }) export function classifyProvider( providerID: string, modelID?: string, - ): { provider: string; provider_id?: string } { + ): { provider: ProviderEnum; provider_id?: string } {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/altimate/telemetry/index.ts` around lines 939 - 949, Update classifyProvider to return the provider_selected event’s closed provider enum type instead of a generic string, deriving the type from the existing event union or its provider field. Ensure CURATED_PROVIDER_ENUM values and all returned literals are checked against that enum so type drift fails at compile time.packages/tui/test/cli/tui/dialog-model-welcome.test.tsx (1)
144-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReset the shared onboarding state in
cleanup.
mountPickermutates module-global signals inpackages/tui/src/component/altimate-onboarding.tsxthroughresetSetupComplete()andmarkFirstRunActive().cleanup()only destroys the renderer, sofirstRunActivestays set after the suite finishes. Any other suite that shares the module registry then starts with an active first run.🧪 Proposed teardown
async cleanup() { app.renderer.destroy() + onboarding.resetSetupComplete() },Based on coding guidelines: "Tests using global
mock.module, dispatchers, or similar shared state must provide teardown and isolation safe for parallelbun testexecution."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tui/test/cli/tui/dialog-model-welcome.test.tsx` around lines 144 - 147, Update cleanup() in mountPicker to reset the shared onboarding state after destroying the renderer by invoking the existing resetSetupComplete() and markFirstRunActive() helpers as appropriate. Ensure firstRunActive and related module-global signals return to their initial values so subsequent or parallel suites start isolated.Source: Coding guidelines
packages/tui/test/cli/tui/dialog-scan-gate.test.tsx (1)
143-169: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the
onDismisspath.
mountGatedoes not passonDismisstoDialogScanGate, and no test in this file exercises the dismiss path added alongside these two duplicate-choice tests. Add a test that triggers dismissal (mouse click on the "esc" label, and/or an Escape keypress once wired) and assertsonDismissfires exactly once.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tui/test/cli/tui/dialog-scan-gate.test.tsx` around lines 143 - 169, The scan-gate tests cover duplicate choices but not dismissal. Update mountGate to provide an onDismiss spy to DialogScanGate, then add a test that triggers dismissal through the visible “esc” mouse target (or wired Escape key) and asserts the callback fires exactly once, preserving the existing choice assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/opencode/src/altimate/plugin/altimate.ts`:
- Around line 412-422: Update the catch block around the gateway sign-in flow to
avoid logging raw error messages that may contain the tenant instance name.
Change the console.error call to log the classified reason from reasonOf(err)
instead, while preserving the existing telemetry emission and failure handling.
In `@packages/opencode/src/altimate/telemetry/index.ts`:
- Around line 1809-1818: Update doShutdown’s telemetry state reset to also clear
userEmail alongside machineId, sessionId, and projectId. Ensure doInit starts
with no stale email when Account.active() returns no account, while preserving
the existing identity initialization behavior.
- Around line 1804-1808: Update the shutdown sequence around inFlightFlush and
flush so the in-flight drain is raced against the caller-provided timeout
budget, rather than waiting unboundedly. Track elapsed time or remaining budget
after draining, then pass the remaining timeout to the final flush while
preserving the default behavior when timeoutMs is undefined.
- Around line 1546-1568: Update the shutdown-handling branch in the
initialization method to assign the chained reinitialization promise from
shutdownPromise.catch(...).then(doInit) to both reinitPromise and initPromise.
Preserve the existing reinitPromise cleanup, and ensure the assignment allows
doShutdown() to await the in-flight reinitialization while keeping the
documented initPromise guard accurate.
In `@packages/opencode/test/altimate/telemetry/onboarding.test.ts`:
- Around line 325-361: Fix teardown in the shown telemetry test by restoring
ALTIMATE_TELEMETRY_DISABLED with the same undefined-aware conditional pattern
already used for APPLICATIONINSIGHTS_CONNECTION_STRING, deleting it when
origDisabled was unset. Update the body parsing around Telemetry.flush so it
searches all collected bodies for the envelopes containing onboarding_started
and scan_gate_choice instead of assuming bodies[0], preserving the existing
launch_id assertions.
In `@packages/tui/src/component/dialog-scan-gate.tsx`:
- Around line 16-40: The physical Escape key must trigger the same dismissal
path as the on-screen escape action. In
packages/tui/src/component/dialog-scan-gate.tsx, update the useKeyboard handler
around lines 104-104 to route evt.name === "escape" to dismiss(), preserving the
chosen guard and onDismiss reporting; the dialog logic at lines 16-40 requires
no direct change. Add or update coverage in
packages/tui/test/cli/tui/dialog-scan-gate.test.tsx lines 143-169 to verify
physical Escape invokes dismissal reporting and clears the dialog.
---
Outside diff comments:
In `@packages/opencode/src/altimate/tools/sample-setup.ts`:
- Around line 99-136: Redact filesystem paths in the sample-source resolution
failure before exposing the error through model-facing output. In the catch
block surrounding resolveSampleSource/readSampleVersionAt, pass the underlying
error text through the existing redactPaths utility before assigning it to
message or interpolating it into guidance, while preserving the metadata error
handling and status output behavior.
---
Nitpick comments:
In `@packages/opencode/src/altimate/telemetry/index.ts`:
- Around line 939-949: Update classifyProvider to return the provider_selected
event’s closed provider enum type instead of a generic string, deriving the type
from the existing event union or its provider field. Ensure
CURATED_PROVIDER_ENUM values and all returned literals are checked against that
enum so type drift fails at compile time.
In `@packages/opencode/src/altimate/telemetry/onboarding.ts`:
- Around line 308-316: Append the repository-standard bottom-of-file
self-reexport to the onboarding module, preserving its existing flat top-level
exports and resetForTest implementation. Use the module’s expected namespace
alias and self-reference pattern without introducing an export namespace block.
In `@packages/tui/test/cli/tui/dialog-model-welcome.test.tsx`:
- Around line 144-147: Update cleanup() in mountPicker to reset the shared
onboarding state after destroying the renderer by invoking the existing
resetSetupComplete() and markFirstRunActive() helpers as appropriate. Ensure
firstRunActive and related module-global signals return to their initial values
so subsequent or parallel suites start isolated.
In `@packages/tui/test/cli/tui/dialog-scan-gate.test.tsx`:
- Around line 143-169: The scan-gate tests cover duplicate choices but not
dismissal. Update mountGate to provide an onDismiss spy to DialogScanGate, then
add a test that triggers dismissal through the visible “esc” mouse target (or
wired Escape key) and asserts the callback fires exactly once, preserving the
existing choice assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 11a42e9b-6229-4219-8e10-1a571479b3b2
📒 Files selected for processing (22)
docs/docs/reference/telemetry.mdpackages/opencode/src/altimate/plugin/altimate.tspackages/opencode/src/altimate/plugin/onboarding-telemetry.tspackages/opencode/src/altimate/telemetry/index.tspackages/opencode/src/altimate/telemetry/onboarding.tspackages/opencode/src/altimate/tools/project-scan.tspackages/opencode/src/altimate/tools/sample-setup.tspackages/opencode/src/cli/cmd/tui.tspackages/opencode/src/cli/tui/worker.tspackages/opencode/src/plugin/index.tspackages/opencode/src/session/prompt.tspackages/opencode/test/altimate/telemetry/onboarding.test.tspackages/tui/package.jsonpackages/tui/src/app.tsxpackages/tui/src/component/altimate-onboarding.tsxpackages/tui/src/component/dialog-model.tsxpackages/tui/src/component/dialog-provider.tsxpackages/tui/src/component/dialog-scan-gate.tsxpackages/tui/src/component/prompt/index.tsxpackages/tui/src/context/onboarding-telemetry.tsxpackages/tui/test/cli/tui/dialog-model-welcome.test.tsxpackages/tui/test/cli/tui/dialog-scan-gate.test.tsx
| export function DialogScanGate(props: { onChoose: (arg: "scan" | "skip") => void; onDismiss?: () => void }) { | ||
| const { theme } = useTheme() | ||
| const dialog = useDialog() | ||
| const [selected, setSelected] = createSignal(0) // 0 = Yes (default, per spec ❯) | ||
|
|
||
| onMount(() => dialog.setSize("large")) | ||
|
|
||
| // altimate_change — keyboard (return / y / n) and mouse handlers all call run() directly, and | ||
| // nothing stops two firing before the dialog unmounts. Without this guard a fast double-press | ||
| // submits `/onboard-connect` twice and double-counts the funnel choice. | ||
| let chosen = false | ||
|
|
||
| // altimate_change — esc/click-away previously called dialog.clear() directly, leaving a | ||
| // scan_gate_shown with no matching choice and no abandonment either (completed is already true | ||
| // from the same readiness transition). Report it as its own outcome. | ||
| function dismiss() { | ||
| if (chosen) return | ||
| chosen = true | ||
| props.onDismiss?.() | ||
| dialog.clear() | ||
| } | ||
|
|
||
| function run(arg: "scan" | "skip") { | ||
| if (chosen) return | ||
| chosen = true |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate target files:"
git ls-files | rg 'packages/tui/src/component/dialog-scan-gate\.tsx|packages/tui/test/cli/tui/dialog-scan-gate\.test\.tsx' || true
echo
echo "Search scan gate related symbols:"
rg -n "scan_gate_choice|scan_gate_shown|onDismiss|DialogScanGate|useKeyboard|dialog\.clear\(|onKey|escape|Escape" packages/tui -S || true
echo
echo "Component outline:"
ast-grep outline packages/tui/src/component/dialog-scan-gate.tsx --view expanded || true
echo
echo "Relevant component lines:"
wc -l packages/tui/src/component/dialog-scan-gate.tsx
sed -n '1,150p' packages/tui/src/component/dialog-scan-gate.tsx | cat -n
echo
echo "Relevant test lines:"
wc -l packages/tui/test/cli/tui/dialog-scan-gate.test.tsx
sed -n '1,240p' packages/tui/test/cli/tui/dialog-scan-gate.test.tsx | cat -nRepository: AltimateAI/altimate-code
Length of output: 32793
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "List nearby component files with Dialog or Keyboard patterns:"
fd -e tsx -e ts . packages/tui/src | sed 's#^\./##' | rg 'Dialog|Keyboard|dialog|keyboard' || true
echo
echo "UseKeyboard implementations/usages:"
rg -n "function useKeyboard|const useKeyboard|useKeyboard\(" packages/tui/src packages/tui -S || trueRepository: AltimateAI/altimate-code
Length of output: 2583
Wire physical Escape to dismiss(), or remove the “esc” claim. dismiss() is only called from the on-screen esc label, while useKeyboard handles Up/Down/Return/y/n but not Escape. Pressing the physical Escape key now does not route through onDismiss reporting, so add evt.name === "escape" → dismiss() here or update the esc/click-away comment.
📍 Affects 2 files
packages/tui/src/component/dialog-scan-gate.tsx#L16-L40(this comment)packages/tui/src/component/dialog-scan-gate.tsx#L104-L104packages/tui/test/cli/tui/dialog-scan-gate.test.tsx#L143-L169
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tui/src/component/dialog-scan-gate.tsx` around lines 16 - 40, The
physical Escape key must trigger the same dismissal path as the on-screen escape
action. In packages/tui/src/component/dialog-scan-gate.tsx, update the
useKeyboard handler around lines 104-104 to route evt.name === "escape" to
dismiss(), preserving the chosen guard and onDismiss reporting; the dialog logic
at lines 16-40 requires no direct change. Add or update coverage in
packages/tui/test/cli/tui/dialog-scan-gate.test.tsx lines 143-169 to verify
physical Escape invokes dismissal reporting and clears the dialog.
There was a problem hiding this comment.
Fixed in fe6ac08, at the registration site rather than in the component. dialog.replace() takes an onClose second argument that DialogProvider invokes on the physical Escape and on click-away, so app.tsx now registers a latched recordScanChoice("dismissed") there and shares it with onDismiss. Every close path records exactly one choice, and a real choice wins.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Relevant dialog registration and scan-choice code:"
rg -n -C 8 'DialogScanGate|dialog\.replace\(|recordScanChoice|onDismiss|scan_gate_choice' \
packages/tui/src/app.tsx
echo
echo "Dialog API and close callback implementation:"
rg -n -C 8 'replace\s*\(|onClose|clear\s*\(|escape|Escape|click' \
packages/tui/src/ui packages/tui/src -g '*dialog*.ts' -g '*dialog*.tsx'
echo
echo "Focused component behavior:"
sed -n '16,85p' packages/tui/src/component/dialog-scan-gate.tsx | cat -nLength of output: 156972
@sahrizvi, the dialog.replace() callback correctly covers physical Escape and click-away.
However, the real choice does not win with the current order. In DialogScanGate.run(), dialog.clear() runs before props.onChoose(arg). dialog.clear() synchronously invokes the registered onClose, which records "dismissed" first. The scanChoiceRecorded latch then suppresses "scan" or "skip".
Call props.onChoose(arg) before dialog.clear(), or otherwise record the real choice before the close callback runs. The finding remains unresolved.
🧬 Code Graph Analysis Results
packages/tui/src/app.tsx — lines 205–364
run creates the renderer and mounts the TUI provider hierarchy. OnboardingTelemetryProvider is mounted above DialogProvider; App is rendered inside DialogProvider and PromptRefProvider. Telemetry defaults to a no-op when input.onTelemetry is absent.
packages/tui/src/ui/dialog.tsx — lines 222–228
useDialog() retrieves the dialog context and throws if called outside DialogProvider.
You are interacting with an AI system.
`cubic` (18) and `kilo-code-bot` (1). Four of these are regressions from the consensus-review fixes in `8e369a8`. Correctness: - `app.tsx`: completing setup from the prompt gate before sync hydration settled made the first-run effect see `onboardingReady()` and return, skipping the funnel and the scan gate entirely. `setupComplete()` discriminates that case from a genuine returning user. - `telemetry/index.ts`: a caller arriving after a shutdown settled but before the chained `doInit()` finished started a second one, replacing the flush timer. The chained promise is now published as `initPromise` too. - `telemetry/index.ts`: `doShutdown()` cleared `cachedLaunchId`, so a long-lived `serve` process minted a new `launch_id` per prompt. Replaced with a `resetLaunchIdForTest()` seam. - `plugin/onboarding-telemetry.ts`: `activation_job_selected` is claimed in `tool.execute.before`, so a job that throws still records the selection the docs promise. - `session/prompt.ts`: an attachment-only message counted as `first_prompt_sent`. - `app.tsx` / `altimate-onboarding.tsx`: `Escape` and click-away closes recorded no `scan_gate_choice` / `big_pickle_choice` — only the inline `esc` control did. - `dialog-model.tsx`: `via_search` was dropped when returning from Big Pickle. - `context/onboarding-telemetry.tsx`: `try`/`catch` cannot catch an async tracker's rejection. Bounds and privacy: - `cmd/tui.ts`: the exit budget covered only the flush, not the credential read or `shutdown()`'s await of initialization. - `telemetry/index.ts`: `shutdown()` awaited an in-flight flush unbounded (10s). - `worker.ts` / `cmd/tui.ts`: the two halves of the exit budget now share `Telemetry.TUI_SHUTDOWN_BUDGET_MS`. - `sample-setup.ts`: the `sample_source_missing` output embedded absolute paths. Tests: activation-event tests drive both tool hooks rather than `.after` alone, plus coverage for a tool that throws. The launch-correlation test redirects `HOME` so real `init()` no longer writes `~/.altimate/machine-id`. Declined: the stale `commandSubmission` flag after a failed slash command. There is no `command.execute.after` hook to clear it from, it suppresses at most one event in a session where the command already failed, and dropping the guard would mis-record the scan gate's synthetic `/onboard-connect` as a typed prompt in every fresh onboarding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
- `doShutdown()` cleared `machineId` but kept `userEmail`, so a logout followed by a re-init in the same process kept hashing the previous account into `ai.user.id`. `doInit()` only assigns it when `Account.active()` returns one. - The launch-correlation test restored `ALTIMATE_TELEMETRY_DISABLED` unconditionally, coercing an unset original to the string `"undefined"` and leaking a disabled-telemetry flag into every sibling suite. It also read `bodies[0]`, which the 5s interval can make the wrong batch. `coderabbit` independently reported the reinit-chain and unbounded-drain bugs in `telemetry/index.ts` that `cubic` also found; both were fixed in `fe6ac08`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/opencode/src/altimate/telemetry/index.ts (1)
1848-1858: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winReset
userEmailalong with the other identity state.
doShutdownclearssessionId,projectId, andmachineIdbut leavesuserEmailuntouched.doInitonly assignsuserEmailwhenAccount.active()returns an account, so after a logout and re-init in the same process, events keep shipping the previously hashed email inai.user.id. This is the same finding raised on a prior commit of this file and has not been addressed yet.🛡️ Proposed fix
sessionId = "" projectId = "" machineId = "" + userEmail = ""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/altimate/telemetry/index.ts` around lines 1848 - 1858, Update doShutdown to clear userEmail alongside sessionId, projectId, and machineId, ensuring re-initialization after logout cannot reuse the previous hashed email.
♻️ Duplicate comments (1)
packages/opencode/src/altimate/telemetry/index.ts (1)
1831-1846: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the total shutdown wait to a single budget, not two sequential ones.
The drain step at Line 1838-1843 waits up to
timeoutMs ?? REQUEST_TIMEOUT_MS. Line 1846 then callsflush(timeoutMs), which internally aborts its own fetch after another fulltimeoutMs. The two waits are not deducted from each other. WheninFlightFlushis active at shutdown time, total wall-clock time can reach2 × timeoutMsinstead of the caller's intended ceiling.This directly undermines the budget contract documented at Line 53-61:
TUI_SHUTDOWN_BUDGET_MSis described as the value the worker and main thread "must agree" on so the worker's flush is not truncated byworker.terminate(). If the worker passes a bounded remaining budget intoshutdown({ timeoutMs })expecting a single ceiling, this doubling can push the real duration past that ceiling, and an outer terminate/timeout can then kill the process mid-flush()— reintroducing the exact data loss this drain step exists to prevent.Track a deadline and pass the remaining time to
flush().🐛 Proposed fix
if (inFlightFlush) { - let drainTimer: ReturnType<typeof setTimeout> | undefined - await Promise.race([ - inFlightFlush.catch(() => {}), - new Promise<void>((resolve) => { - drainTimer = setTimeout(resolve, timeoutMs ?? REQUEST_TIMEOUT_MS) - }), - ]) - if (drainTimer) clearTimeout(drainTimer) + const deadline = Date.now() + (timeoutMs ?? REQUEST_TIMEOUT_MS) + let drainTimer: ReturnType<typeof setTimeout> | undefined + await Promise.race([ + inFlightFlush.catch(() => {}), + new Promise<void>((resolve) => { + drainTimer = setTimeout(resolve, Math.max(0, deadline - Date.now())) + }), + ]) + if (drainTimer) clearTimeout(drainTimer) + if (timeoutMs !== undefined) timeoutMs = Math.max(0, deadline - Date.now()) } await flush(timeoutMs)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/altimate/telemetry/index.ts` around lines 1831 - 1846, Update the shutdown flow around inFlightFlush and flush to track one deadline based on timeoutMs ?? REQUEST_TIMEOUT_MS, use only the remaining time for the drain race, and pass the remaining budget to flush(timeout). Ensure the total shutdown wait never exceeds the original timeout budget while preserving the existing drain-before-final-flush ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/tui/src/app.tsx`:
- Around line 671-708: Update DialogScanGate.run() so props.onChoose(arg)
executes before dialog.clear(). Preserve the existing close handling, but ensure
clearing the dialog cannot invoke the dismissal callback first and mask the real
"scan" or "skip" choice.
---
Outside diff comments:
In `@packages/opencode/src/altimate/telemetry/index.ts`:
- Around line 1848-1858: Update doShutdown to clear userEmail alongside
sessionId, projectId, and machineId, ensuring re-initialization after logout
cannot reuse the previous hashed email.
---
Duplicate comments:
In `@packages/opencode/src/altimate/telemetry/index.ts`:
- Around line 1831-1846: Update the shutdown flow around inFlightFlush and flush
to track one deadline based on timeoutMs ?? REQUEST_TIMEOUT_MS, use only the
remaining time for the drain race, and pass the remaining budget to
flush(timeout). Ensure the total shutdown wait never exceeds the original
timeout budget while preserving the existing drain-before-final-flush ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 09f11133-d3d8-43ae-ad5a-e5c2270a6c89
📒 Files selected for processing (15)
docs/docs/reference/telemetry.mdpackages/opencode/src/altimate/plugin/onboarding-telemetry.tspackages/opencode/src/altimate/telemetry/index.tspackages/opencode/src/altimate/telemetry/onboarding.tspackages/opencode/src/altimate/tools/project-scan.tspackages/opencode/src/altimate/tools/sample-setup.tspackages/opencode/src/cli/cmd/tui.tspackages/opencode/src/cli/tui/worker.tspackages/opencode/src/session/prompt.tspackages/opencode/test/altimate/telemetry/onboarding.test.tspackages/tui/src/app.tsxpackages/tui/src/component/altimate-onboarding.tsxpackages/tui/src/component/dialog-model.tsxpackages/tui/src/context/onboarding-telemetry.tsxpackages/tui/test/cli/tui/dialog-model-welcome.test.tsx
💤 Files with no reviewable changes (1)
- packages/opencode/src/altimate/tools/project-scan.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/opencode/src/session/prompt.ts
- docs/docs/reference/telemetry.md
- packages/tui/src/context/onboarding-telemetry.tsx
- packages/opencode/test/altimate/telemetry/onboarding.test.ts
- packages/opencode/src/altimate/tools/sample-setup.ts
- packages/opencode/src/cli/cmd/tui.ts
- packages/opencode/src/altimate/plugin/onboarding-telemetry.ts
- packages/opencode/src/altimate/telemetry/onboarding.ts
- packages/tui/src/component/dialog-model.tsx
- packages/tui/src/component/altimate-onboarding.tsx
There was a problem hiding this comment.
5 issues found across 15 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/tui/src/app.tsx">
<violation number="1" location="packages/tui/src/app.tsx:608">
P2: For the impatient-user path this new branch calls markFirstRunActive() after setup has already completed. Because markFirstRunActive() only ever sets the signal and the only clears (markSetupComplete/resetSetupComplete) will not run again once setup is already done, the firstRunActive flag is latched true for the entire rest of the session. That makes any later /model switch or /connect emit provider_selected and big_pickle_* funnel events, contaminating the onboarding funnel with routine activity — exactly the misattribution the firstRunActive gate was designed to prevent. It also means the impatient user's actual provider choice in the prompt-gate picker was never recorded, since that picker ran while firstRunActive was still false. Consider not re-marking firstRunActive here (the funnel events on this branch don't need it) or explicitly clearing it once the scan gate/onboarding flow completes rather than leaving it stuck true.</violation>
<violation number="2" location="packages/tui/src/app.tsx:706">
P2: Every Yes/No scan-gate selection is currently recorded as `dismissed`: `dialog.clear()` invokes this close callback before `onChoose(arg)` records the actual choice, and `scanChoiceRecorded` then suppresses `scan`/`skip`. Deferring the close callback or otherwise recording the choice before clearing the dialog preserves the correct outcome.</violation>
</file>
<file name="packages/opencode/src/altimate/telemetry/onboarding.ts">
<violation number="1" location="packages/opencode/src/altimate/telemetry/onboarding.ts:35">
P3: The enum note's stated mechanism doesn't match the code: it says reaching "connected" means the run completed and emitAbandonedIfIncomplete() returns early on `completed`, but `connected` is also reached via `instance_connected` (STAGE_FOR_EVENT), which sets the stage without setting `completed`, and `onboarding_completed` (the only setter of `completed`) is never emitted anywhere in src. The actual reason "connected" is never a `last_stage` on abandonment is that both `instance_connected` and `onboarding_completed` are worker-thread events, so the main thread that owns abandonment never reaches "connected" at all. Consider documenting the thread split as the real rationale to avoid misleading a future maintainer who might rely on the `completed` guard.</violation>
</file>
<file name="packages/opencode/src/altimate/telemetry/index.ts">
<violation number="1" location="packages/opencode/src/altimate/telemetry/index.ts:1590">
P2: Onboarding telemetry can now silently drop early events for a new session when an init() lands while the previous session's shutdown is still in flight (the exact scenario this reinit path was built for). Because `initPromise = reinitPromise` makes doShutdown()'s guard fail to match, `initDone` stays true from the old session while `enabled` has already been set false, so `track()` hits the `if (initDone && !enabled) return` guard and drops the event before the reinit's doInit() re-enables telemetry. Previously this window had initDone=false, so those early events were buffered and flushed. Consider resetting `initDone = false` alongside `initPromise = reinitPromise` so events emitted before the reinit completes are buffered rather than dropped, or awaiting init() before emitting in the same window.</violation>
<violation number="2" location="packages/opencode/src/altimate/telemetry/index.ts:1841">
P2: A slow in-flight telemetry request can still make TUI shutdown exceed its deadline and lose the worker's buffered onboarding events: the drain spends the full `timeoutMs`, then the serialized final flush waits behind that request and gets the same timeout again. Carry one absolute deadline/remaining budget through both operations and handle an expired in-flight request without queueing another flush past the deadline.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| "provider_setup", | ||
| "big_pickle_confirm", | ||
| "gateway_auth", | ||
| // NOTE: reaching this stage means the run completed, and emitAbandonedIfIncomplete() returns |
There was a problem hiding this comment.
P3: The enum note's stated mechanism doesn't match the code: it says reaching "connected" means the run completed and emitAbandonedIfIncomplete() returns early on completed, but connected is also reached via instance_connected (STAGE_FOR_EVENT), which sets the stage without setting completed, and onboarding_completed (the only setter of completed) is never emitted anywhere in src. The actual reason "connected" is never a last_stage on abandonment is that both instance_connected and onboarding_completed are worker-thread events, so the main thread that owns abandonment never reaches "connected" at all. Consider documenting the thread split as the real rationale to avoid misleading a future maintainer who might rely on the completed guard.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/telemetry/onboarding.ts, line 35:
<comment>The enum note's stated mechanism doesn't match the code: it says reaching "connected" means the run completed and emitAbandonedIfIncomplete() returns early on `completed`, but `connected` is also reached via `instance_connected` (STAGE_FOR_EVENT), which sets the stage without setting `completed`, and `onboarding_completed` (the only setter of `completed`) is never emitted anywhere in src. The actual reason "connected" is never a `last_stage` on abandonment is that both `instance_connected` and `onboarding_completed` are worker-thread events, so the main thread that owns abandonment never reaches "connected" at all. Consider documenting the thread split as the real rationale to avoid misleading a future maintainer who might rely on the `completed` guard.</comment>
<file context>
@@ -32,6 +32,9 @@ export const ONBOARDING_STAGES = [
"provider_setup",
"big_pickle_confirm",
"gateway_auth",
+ // NOTE: reaching this stage means the run completed, and emitAbandonedIfIncomplete() returns
+ // early on `completed`. So "connected" is a valid funnel position but never a `last_stage` on
+ // an abandonment — see the enum note in docs/docs/reference/telemetry.md.
</file context>
…oduced The close-handler fix in `fe6ac08` inverted the metric it was meant to repair, and two others had side effects. Found by `cubic` on the pushed head and by a class-scoped `codex` sweep. - `dialog-scan-gate.tsx` / `app.tsx`: `run()` calls `dialog.clear()` before `props.onChoose(arg)`, and `dialog.clear()` invokes the close handler synchronously — so the latched `dismissed` recorder consumed EVERY Yes/No and the funnel reported a dismissal for every real choice. New `onOutcome` seam, invoked before the clear on both the choice and dismiss paths; `onChoose` keeps its position after the clear so the prompt-submit ordering is unchanged. - `app.tsx`: dropped `markFirstRunActive()` from the pre-completed-setup branch. Its only clear is `markSetupComplete()`, which has already run there, so the flag latched true for the session and every later `/model` pick would have emitted funnel events. - `telemetry/index.ts`: publishing `initPromise = reinitPromise` made `doShutdown()`'s reset guard miss, leaving `initDone` true while `enabled` was already false — `track()` then silently DROPPED events in exactly the window the reinit path exists to protect. Reset `initDone` when publishing. - `telemetry/index.ts`: the shutdown budget was still not hard. One absolute deadline now spans the drain and the final flush, and a drain that expires aborts the in-flight request rather than letting the final flush chain behind the very request it gave up on — which is what made the timeout buy nothing. Retry write-back is suppressed during shutdown, since re-inserting into a buffer about to be cleared only ships those events under the next launch id. Tests. The existing scan-gate tests all passed against wiring that does not exist: `mountGate()` renders the gate as a plain child of `DialogProvider`, while production mounts it through `dialog.replace(fn, onClose)`. Added `mountGateAsApp()`, which reproduces the real wiring, plus three tests over it (scan, skip, escape). Added a shutdown-budget test with a hanging request. All four new tests were mutation-checked: they fail with their fix reverted, and the budget test hangs for the full 10s default without the abort. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/altimate/telemetry/index.ts">
<violation number="1" location="packages/opencode/src/altimate/telemetry/index.ts:1885">
P2: The final telemetry flush can overrun the caller’s shutdown budget by up to 250 ms, so the worker may be terminated before buffered exit events such as `onboarding_abandoned` are sent. Passing the actual positive remaining time, and skipping the flush when no time remains, would preserve the hard deadline.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| } | ||
| // Whatever the drain left of the shared deadline, never below a floor that lets the request | ||
| // actually be made. | ||
| await flush(Math.max(250, deadline - Date.now())) |
There was a problem hiding this comment.
P2: The final telemetry flush can overrun the caller’s shutdown budget by up to 250 ms, so the worker may be terminated before buffered exit events such as onboarding_abandoned are sent. Passing the actual positive remaining time, and skipping the flush when no time remains, would preserve the hard deadline.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/telemetry/index.ts, line 1885:
<comment>The final telemetry flush can overrun the caller’s shutdown budget by up to 250 ms, so the worker may be terminated before buffered exit events such as `onboarding_abandoned` are sent. Passing the actual positive remaining time, and skipping the flush when no time remains, would preserve the hard deadline.</comment>
<file context>
@@ -1830,21 +1850,41 @@ export namespace Telemetry {
- await flush(timeoutMs)
+ // Whatever the drain left of the shared deadline, never below a floor that lets the request
+ // actually be made.
+ await flush(Math.max(250, deadline - Date.now()))
inFlightFlush = undefined
+ shuttingDown = false
</file context>
| await flush(Math.max(250, deadline - Date.now())) | |
| const remaining = deadline - Date.now() | |
| if (remaining > 0) await flush(remaining) |
…r Guard Marker Guard was red on `3875d00`, and the root cause was in the comment that warns about this exact trap: the prose at `cli/cmd/tui.ts` spelled the closing marker out literally, and the parser matches that token in comment text — so the enclosing region closed on that line and everything after it was reported as unmarked. Reworded, plus start/end pairs added around six multi-line additions that only carried a single-line marker. `analyze.ts --markers --strict` is clean. The four remaining findings from the class-scoped `codex` sweep, all of which this PR introduced (none of these symbols exist at the merge-base): - `plugin/altimate.ts`: gateway events fired for every `authorize()`, so routine `/auth`, `/connect` and reauthentication all emitted onboarding-taxonomy events. They could not be gated on `funnelStarted` because the flow runs in the worker and the funnel opens on the main thread. The TUI now tells the worker through a new `onboardingStarted` RPC, and the four emits are gated on it. In `serve` / `run` / `github` there is no first-run funnel, so they correctly stay silent. - `plugin/altimate.ts`: the per-attempt outcome latch was a boolean, so two callbacks exchanging the same one-time token let the one that failed fast claim it and report `gateway_auth_failed` while the other saved valid credentials — connected user, failed funnel. Tri-state now; success is authoritative. - `dialog-model.tsx`: the catalogue had no submit latch (`DialogSelect.submit()` has none of its own) and recorded the choice before awaiting an async provider action, so a second Enter emitted `provider_selected` twice and started a second authorization flow. The curated picker has this latch; the catalogue did not. - `dialog-model.tsx`: the "Other" row recorded a selection before `promptCustomProviderID()` resolved, so cancelling that prompt still produced a `provider_selected` classified as `other` with no provider behind it. - `dialog-provider.tsx`: `firstRunActive` had no clear on the one path where the user is genuinely onboarded but `markSetupComplete()` never runs — gateway connected with no usable model. Added `clearFirstRunActive()` as that catalogue's close handler. Ordinary dismissals deliberately keep the flag: a user who has set nothing up is still mid-first-run. Not addressed, and not ours: per-session `Telemetry.shutdown()` is unsafe with concurrent sessions (session A's shutdown disables telemetry under session B). `Telemetry.shutdown` in `session/prompt.ts` predates this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
There was a problem hiding this comment.
3 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/altimate/plugin/altimate.ts">
<violation number="1" location="packages/opencode/src/altimate/plugin/altimate.ts:366">
P2: Fast first-run gateway sign-ins can be missing from the funnel because this worker-side guard runs before the unawaited `onboardingStarted` RPC has set `funnelStarted`; the subsequent callback can also complete before activation and lose both outcome events. Synchronizing the worker activation (or buffering gateway events until its acknowledgement) would retain first-run telemetry without re-enabling telemetry for routine `/auth`.</violation>
</file>
<file name="packages/tui/src/component/dialog-model.tsx">
<violation number="1" location="packages/tui/src/component/dialog-model.tsx:158">
P2: A valid provider named `__opencode_custom_provider__` is classified as the synthetic “Other” row, so choosing it omits `provider_selected` and undercounts that provider in the funnel. Distinguishing the option kind from its string value or reserving this sentinel in provider-ID validation would avoid the collision.</violation>
</file>
<file name="packages/opencode/src/altimate/telemetry/onboarding.ts">
<violation number="1" location="packages/opencode/src/altimate/telemetry/onboarding.ts:296">
P2: After the first-run flow completes, any later `/auth` or gateway `/connect` in the same TUI process is still treated as onboarding because this worker-side flag is one-way. Add a worker-side clear or terminal state transition when onboarding completes/logs out so reauthentication does not emit onboarding funnel events.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // attempted". open() failures are swallowed above (the URL is also printed for the | ||
| // user to paste), so this fires even when no browser actually launched. | ||
| // The URL is never sent — it carries the CSRF `state`. | ||
| if (OnboardingTelemetry.isFunnelActive()) void OnboardingTelemetry.emit({ type: "gateway_device_code_issued" }) |
There was a problem hiding this comment.
P2: Fast first-run gateway sign-ins can be missing from the funnel because this worker-side guard runs before the unawaited onboardingStarted RPC has set funnelStarted; the subsequent callback can also complete before activation and lose both outcome events. Synchronizing the worker activation (or buffering gateway events until its acknowledgement) would retain first-run telemetry without re-enabling telemetry for routine /auth.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/plugin/altimate.ts, line 366:
<comment>Fast first-run gateway sign-ins can be missing from the funnel because this worker-side guard runs before the unawaited `onboardingStarted` RPC has set `funnelStarted`; the subsequent callback can also complete before activation and lose both outcome events. Synchronizing the worker activation (or buffering gateway events until its acknowledgement) would retain first-run telemetry without re-enabling telemetry for routine `/auth`.</comment>
<file context>
@@ -362,12 +363,19 @@ export async function AltimateAuthPlugin(_input: PluginInput): Promise<Hooks> {
// user to paste), so this fires even when no browser actually launched.
// The URL is never sent — it carries the CSRF `state`.
- void OnboardingTelemetry.emit({ type: "gateway_device_code_issued" })
+ if (OnboardingTelemetry.isFunnelActive()) void OnboardingTelemetry.emit({ type: "gateway_device_code_issued" })
// One outcome per attempt. callback() closes over `result` and re-runs its whole body
</file context>
| // real `other` selection. Every other row dispatches a concrete provider, where | ||
| // recording before the auth flow is deliberate (a sign-in later cancelled still | ||
| // counts as a selection). | ||
| if (firstRunActive() && o.value !== CUSTOM_PROVIDER_OPTION_VALUE) { |
There was a problem hiding this comment.
P2: A valid provider named __opencode_custom_provider__ is classified as the synthetic “Other” row, so choosing it omits provider_selected and undercounts that provider in the funnel. Distinguishing the option kind from its string value or reserving this sentinel in provider-ID validation would avoid the collision.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/tui/src/component/dialog-model.tsx, line 158:
<comment>A valid provider named `__opencode_custom_provider__` is classified as the synthetic “Other” row, so choosing it omits `provider_selected` and undercounts that provider in the funnel. Distinguishing the option kind from its string value or reserving this sentinel in provider-ID validation would avoid the collision.</comment>
<file context>
@@ -130,7 +148,14 @@ export function DialogModel(props: {
+ // real `other` selection. Every other row dispatches a concrete provider, where
+ // recording before the auth flow is deliberate (a sign-in later cancelled still
+ // counts as a selection).
+ if (firstRunActive() && o.value !== CUSTOM_PROVIDER_OPTION_VALUE) {
trackOnboarding({
name: "provider_selected",
</file context>
| * here. In non-TUI hosts (`serve`, `run`, `github`) there is no first-run funnel, so this stays | ||
| * false and the gateway events correctly do not fire. | ||
| */ | ||
| export function markFunnelActive() { |
There was a problem hiding this comment.
P2: After the first-run flow completes, any later /auth or gateway /connect in the same TUI process is still treated as onboarding because this worker-side flag is one-way. Add a worker-side clear or terminal state transition when onboarding completes/logs out so reauthentication does not emit onboarding funnel events.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/telemetry/onboarding.ts, line 296:
<comment>After the first-run flow completes, any later `/auth` or gateway `/connect` in the same TUI process is still treated as onboarding because this worker-side flag is one-way. Add a worker-side clear or terminal state transition when onboarding completes/logs out so reauthentication does not emit onboarding funnel events.</comment>
<file context>
@@ -279,6 +279,29 @@ export function consumeCommandSubmission(sessionID: string): boolean {
+ * here. In non-TUI hosts (`serve`, `run`, `github`) there is no first-run funnel, so this stays
+ * false and the gateway events correctly do not fire.
+ */
+export function markFunnelActive() {
+ funnelStarted = true
+}
</file context>
| // documented promise that a sign-in later cancelled still counts as a selection. | ||
| onSelect: () => { | ||
| if (activated) return | ||
| activated = true |
There was a problem hiding this comment.
SUGGESTION: Cancelling the "Other" id prompt dead-ends the catalogue
activated is set to true here before o.onSelect?.() runs (line 165). For the custom "Other" row, o.onSelect() awaits promptCustomProviderID() and returns without navigating when the user cancels it (value === null -> early return, no dialog.replace). DialogSelect.submit() does not close the dialog on submit, and activated is a single shared component-scoped latch, so every later row is then blocked by the if (activated) return guard -- the catalogue becomes non-functional until the user Escape-closes and reopens it. The neighbouring comment already acknowledges the "Other" row can be cancelled, but the latch does not account for that path. Consider latching only after the custom flow confirms a selection, or resetting activated when o.onSelect() returns without navigating.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
…ct name CI's Marker Guard was failing at the branding leak audit, not the marker check I had been re-running. Same nested-marker trap as the last commit, one file over: the `provider_selected` block in `dialog-model.tsx` was written as a start/end pair, but it sits INSIDE the file-level `DialogModel` block. Its closing marker ended that outer block early, so everything below it — including an upstream "OpenCode Zen" reference in a comment at line 190 — fell outside any `altimate_change` region, where the branding scanner then flagged it. Single-line marker instead, matching the other in-block annotations in that component. The comment itself predates this branch (#1001); only its exposure is ours. All four Marker Guard steps now exit 0 locally: `--markers --strict`, `--branding`, `--require-markers --strict`, plus the parser and release-preflight test suites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
…nch_id test The three test failures on this PR predate the last two commits — the same three were red on `a949fdc`. - `telemetry/index.ts` had 35 `altimate_change start` against 34 ends: the "serialize flushes" block opened inside `flush()` and never closed. That is what both `altimate_change marker integrity` and `bridge merge: ... marker integrity` were reporting. - The launch-correlation test needs the REAL `Telemetry.init()`, since the launch id is minted there and nowhere else, but every other describe in that file spies `init` to a no-op to keep the funnel tests off the filesystem. The test was relying on a sibling's `afterEach` to have undone that spy, so its result depended on suite ordering: green when the file ran alone, red in the shared process. It now restores explicitly — before installing its own fetch spy, not after, which would remove it — clears `initPromise` via `shutdown()`, and asserts `isEnabled()` so any future sabotage fails with a cause instead of a mysteriously empty batch. Verified with the full CI command: 11,833 tests across 574 files, 0 fail — matching CI's own counts, so the shared-process condition the failure depends on was actually reproduced. The subprocess pass was already green on CI and touches none of this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
|
Reviewed from the onboarding/telemetry angle — solid, careful instrumentation (PII-free enums/counts, allowlisted 1. First-run telemetry fires before the user can opt out. These events are the very first thing that happens on a fresh install, and telemetry is on-by-default ( 2. Abandonment depends on the bounded exit flush. 3. Two counting traps analysts will hit. Both are documented in Everything else looks good — nice work on the two-thread |
…abandonment From @saravmajestic's review on #1049. The exit flush is bounded at 2s (main) / 5s (worker) so quitting never hangs the shell, and on expiry the in-flight request is aborted with no retry write-back. The docs covered SIGKILL loss but not this bounded-budget loss, which is the case that actually biases a metric: `onboarding_abandoned` is emitted only on the exit path, so a slow network removes exactly the signal being measured. Documented rather than papered over, including the direction — a dropped event can only remove an abandonment, never add one, so the measured rate is a lower bound and never an over-report. The event row now points at that limitation too. No behaviour change: the budgets exist to keep exit responsive, and raising them without production latency data would be guessing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
|
Thanks — all three checked against the code rather than taken at face value. One was a real gap and is fixed; one is already covered; one is accurate but predates this PR in a way that changes who needs to sign off. 2. Abandonment vs. the bounded exit flush — valid, fixed in You're right, and the gap was sharper than what the docs said. Now documented, including the direction, because that part is load-bearing for anyone reading a dashboard: a dropped event can only remove an abandonment, never add one, so the measured rate is a lower bound and never an over-report. Treat drop-off changes as meaningful only across comparable network conditions. I did not change the budgets. They exist to keep exit responsive, and picking new numbers without production flush-latency data would be guessing. If we start emitting a client-side flush-duration measure we can revisit with evidence. 3. The two counting traps — already covered, no change. Confirmed both are in 1. First-run telemetry before opt-out — accurate, but it isn't new here. Confirmed the factual claim: there is no telemetry notice or opt-out surface anywhere in the CLI or TUI; The nuance worth correcting before this becomes a merge condition: this PR doesn't introduce that pattern. So the sign-off you're asking for is real, but it's a product/privacy decision about the CLI's telemetry stance generally — not a gate this PR created or can resolve. I've deliberately changed no behaviour there. Happy to file it as a separate issue against the CLI's telemetry posture if that's the right venue. |
3 participants
Issue for this PR
Closes #1066
Type of change
What does this PR do?
Adds the first-run onboarding funnel taxonomy — 19 events covering the setup gate, provider choice, gateway auth, the scan gate, activation and drop-off.
They're emitted from three places, each being where the thing measured actually happens: the TUI (dialogs and startup effects), the gateway auth plugin and the scan/sample tools, and a fork plugin for activation via the existing
command.execute.before/tool.execute.afterhooks.Three things are worth knowing before reading the diff:
packages/tuican't import the telemetry module, so the host injects a callback throughTuiInputand a context carries it down — the same seamcontext/exit.tsxalready uses for the exit function. No HTTP route, no new API surface. The provider is mounted aboveDialogProvider, which matters:ui/dialog.tsxrenders dialog contents as a sibling of its children, so a provider around<App>is invisible to every dialog.The TUI and the server are two threads with separate telemetry buffers, and neither exit path flushed. Both now do, bounded, and every event carries a
launch_idso the two halves of a run can be joined — the funnel spans events emitted before any chat session exists and events emitted with a real one.Three activation events are inferred, not observed. The activation menu isn't UI; it's text the model writes from a prompt template and the user answers in free text. They're derived from the closest deterministic signals and documented as lower bounds, with two gaps named explicitly: the "something else" branch has no tool signature and is never counted, and skill-driven jobs can't be confirmed complete so they're absent from
first_job_completedrather than wrong in it.Counts, booleans and closed enums only — no tenant names, file paths, raw error text, or authorize URLs (which carry the CSRF state).
How did you verify your code works?
launch_idfails the correlation test, and removing the scan-gate guard fails both double-submit tests.launch_iddiffered between the two threads, making the correlation id useless for the exact join it exists for. Two plausible fixes were wrong before the third worked — Bun workers don't observe runtimeprocess.envmutations, andprocess.uptime()is per-thread. Fixed by handing the id to the worker explicitly at construction.tsgo --noEmitclean on both packages; 226 tests passing on the touched suites.Known and documented rather than hidden: under
OPENCODE_FAST_BOOT, sync reports ready before credentials load, so a returning user can transiently look un-onboarded. That's pre-existing product behaviour and these events faithfully report what the UI did.Screenshots / recordings
N/A — the UI is unchanged; this adds instrumentation only.
Checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_018SLUQF3xgZHsGZHSjxe7vb
Summary by CodeRabbit