Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/controller/src/app/env.ts`:
- Line 136: The current amplitudeApiKey assignment uses nullish coalescing
(parsed.AMPLITUDE_API_KEY ?? parsed.VITE_AMPLITUDE_API_KEY) which treats an
empty string as set; change it so an empty/whitespace AMPLITUDE_API_KEY is
considered unset and falls back to VITE_AMPLITUDE_API_KEY. Locate the
amplitudeApiKey assignment and replace the nullish-coalescing logic with a check
that treats parsed.AMPLITUDE_API_KEY as unset when it's undefined, null, or only
whitespace (e.g., use a trimmed truthiness check or explicit empty-string test)
so parsed.VITE_AMPLITUDE_API_KEY is used as the fallback.

In `@apps/controller/src/services/analytics-service.ts`:
- Around line 143-144: The dedupe sets sentUserMessageIds and sentSkillUseIds
are global but analyzeSession() constructs fallback ids like
assistant:${tool}:${index} or uses bare entry.id that can collide across
sessions; update analyzeSession() to prefix all generated/stored event ids with
the session.id (e.g., use `${session.id}:${entry.id}` or
`${session.id}:assistant:${tool}:${index}`) before checking/inserting into
sentUserMessageIds and sentSkillUseIds so deduping is session-scoped; apply the
same session-prefixed id logic wherever those sets are accessed/updated
(including the code paths around lines noted: 165-181, 442-447, 493-496).
- Around line 13-17: The AnalyticsState type uses a single boolean
sessionStartSent which prevents emitting session_start more than once globally;
change AnalyticsState to track session-scoped sent flags (e.g. replace
sessionStartSent with sentSessionStartIds: Record<string, boolean> or
Set<string>) and update all code paths that check/set sessionStartSent (and any
logic that deduplicates by that flag near the handlers referenced) to use a
session identifier (session id or first user-message id) as the key when
deciding to emit session_start; also update any persisted state reads/writes and
the related arrays sentUserMessageIds and sentSkillUseIds to follow the same
session-scoped approach so each new session can send its own session_start
without being blocked by a global flag.
- Around line 377-405: readSkillLedgerSources currently only returns installed
skills from the live ledger which causes resolveSkillSource to fall back to
"builtin" for skills that existed in a session snapshot; update the logic so the
per-session snapshot is authoritative by checking the resolved snapshot first
(use readResolvedSkills() data/its returned map) when resolving a skill source
(in resolveSkillSource or wherever skill_source is determined), and only fall
back to the ledger produced by readSkillLedgerSources() or to "builtin" if
neither snapshot nor ledger contains the slug; ensure you reference the existing
functions readResolvedSkills(), readSkillLedgerSources(), and resolveSkillSource
when implementing this fallback ordering.
- Around line 551-567: The Amplitude POST fetch in analytics-service.ts
currently has no explicit deadline; wrap the call with
AbortSignal.timeout(30000) (or 60000 for 60s) and pass the resulting signal in
the fetch options (add signal) so the poll loop can't be blocked by a slow
Amplitude response; update the fetch invocation that constructs the body with
this.env.amplitudeApiKey to include the signal and handle the abort/timeout
error (e.g., catch the AbortError and treat it as a non-fatal timeout) so the
poll iteration can continue.

In `@apps/web/src/components/channel-connect-modal.tsx`:
- Around line 155-162: The cancel tracking fires while a submit is still in
flight; update handleClose to only call track("workspace_channel_config_cancel",
...) when not submittedSuccessfully AND not loading (e.g., if
(!submittedSuccessfully && !loading) { track(...) }), and add loading to the
useCallback dependency array so the callback uses the current in-flight state;
keep the existing onClose() call unchanged.

In `@apps/web/src/pages/auth.tsx`:
- Around line 209-218: The legacy dual-sent tracking calls are using the raw
authSourceParam instead of the normalized value; update the track calls that
currently pass authSourceParam (e.g., the first track call that sends
"login_email_success"/"signup_email_success" and the provider-based
"*_${provider}_success" calls) to pass normalizedSource ?? "Landing" (or similar
default) so both the legacy and new events use
normalizeAuthSource(authSourceParam); locate usages around the blocks
referencing normalizeAuthSource, normalizedSource, isLogin, and track (also
apply the same change in the other duplicated blocks noted) and replace the raw
authSourceParam with normalizedSource for consistency.

In `@apps/web/src/pages/home.tsx`:
- Around line 867-870: Create a shared connect handler (e.g.,
handleChannelConnect) that receives the channel id/object, calls
normalizeChannel and emits track("workspace_channel_connect_click", { channel })
when normalizeChannel returns a value, and then performs the existing setup/open
flow; replace the two inline usages (the secondary grid block that currently
calls normalizeChannel + track and the primary onboarding cards that open the
same setup flow) to both call handleChannelConnect so the primary first-run
cards are tracked through the same funnel.

In `@apps/web/src/pages/models.tsx`:
- Around line 85-95: getProviderIdFromModelId currently falls back to using the
first path segment (or whole modelId) as a provider, causing bogus provider
values; change the fallback so that if no matching entry is found in models you
only extract and return a provider when modelId contains an explicit separator
(e.g., contains "/"), otherwise return null. Update the function
getProviderIdFromModelId (parameters models and modelId) to check for a slash
before splitting and returning the provider_name, and ensure it returns null for
bare model IDs that aren't present in models.

---

Outside diff comments:
In `@apps/web/src/components/channel-setup/discord-setup-view.tsx`:
- Around line 64-95: The catch block in handleConnect (which calls
postApiV1ChannelsDiscordConnect) fails to emit the submit-failure analytics; add
a track(...) call there mirroring the error path (e.g.,
track("workspace_channel_config_submit", { channel: "discord", success: false
})) before showing the toast so failures from thrown exceptions are tracked;
place it at the start of the catch block in discord-setup-view.tsx so existing
toast.error and finally setConnecting(false) behavior remain unchanged.

In `@apps/web/src/components/channel-setup/feishu-setup-view.tsx`:
- Around line 137-166: The catch block in handleConnect doesn't report a failed
submission when postApiV1ChannelsFeishuConnect throws; update the catch to call
track("workspace_channel_config_submit", { channel: "feishu", success: false })
before showing the toast (and optionally include any error message), so failures
thrown by postApiV1ChannelsFeishuConnect are recorded the same way as error
responses.

In `@apps/web/src/components/channel-setup/slack-oauth-view.tsx`:
- Around line 206-237: In handleManualConnect, the catch block for
postApiV1ChannelsSlackConnect does not emit the failure telemetry, so add a
track("workspace_channel_config_submit", { channel: "slack", success: false })
inside the catch before showing the toast (and optionally include error info if
available); reference the existing track calls in the success and error branches
to ensure symmetry with the success path and keep setConnecting(false) in the
finally as-is.

---

Nitpick comments:
In `@apps/controller/src/runtime/loops.ts`:
- Around line 111-118: The loop currently sets stopped = true in the returned
cleanup but leaves a pending sleep(params.env.runtimeSyncIntervalMs) unresolved,
so cleanup can wait the full interval; change the loop to use a cancellable wait
(e.g., race sleep(...) with a stop promise or use an AbortController) inside run
so the sleep resolves immediately when stopping. Concretely: inside the scope
that defines run and stopped, create a stopResolve function or AbortController
and use Promise.race([sleep(...), stopPromise]) (or an abortableSleep that
listens to the controller) instead of awaiting sleep() directly; then call
stopResolve() (or controller.abort()) in the returned cleanup closure so the
pending wait is cancelled and run exits without waiting for the interval. Ensure
references to run, stopped, and sleep are updated accordingly.

In `@apps/controller/src/services/analytics-service.ts`:
- Around line 160-175: The loop in analytics-service (using
sessionsRuntime.listSessions(), readTranscript(), readResolvedSkills(), and
readSkillLedgerSources()) re-reads full transcript and sessions.json for every
session on each poll; fix by persisting and checking per-session metadata (file
offset and mtime) and caching directory-level sessions.json to avoid re-parsing
unchanged files: update the session processing to load a small per-session state
store (lastReadOffset, lastMtime, lastResolvedSkillsHash) and only call
readTranscript/readResolvedSkills for files that changed since last poll, and
add a cache keyed by session directory for the sessions.json content read by
readSkillLedgerSources/readResolvedSkills so repeated lookups in the same dir
reuse the parsed value.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web/src/components/skills/import-skill-modal.tsx`:
- Around line 86-88: The analytics call inside import-skill-modal.tsx is sending
a raw local filename via selectedFile.name as a fallback; update the
track("workspace_skill_enable", ...) invocation to avoid any client filenames by
removing the selectedFile.name fallback and instead send only the
server-provided result.slug (or a fixed safe constant like "unknown_skill" or
"local_skill" when result.slug is falsy). Locate the track call and replace the
name payload to use result.slug ?? "unknown_skill" (or just result.slug) so no
local filenames are included in analytics.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/controller/src/app/env.ts`:
- Around line 136-137: The amplitudeApiKey assignment currently only trims
parsed.AMPLITUDE_API_KEY but leaves parsed.VITE_AMPLITUDE_API_KEY untrimmed;
update the amplitudeApiKey expression in env.ts (the amplitudeApiKey
export/field) to apply .trim() and emptiness handling to both sources (e.g., use
parsed.AMPLITUDE_API_KEY?.trim() || parsed.VITE_AMPLITUDE_API_KEY?.trim()) so a
whitespace-only VITE_AMPLITUDE_API_KEY doesn't produce a truthy invalid key.

In `@apps/controller/src/services/analytics-service.ts`:
- Around line 203-213: The poll() flow is adding dedupe state (calls like
this.sentUserMessageIds.add(...), this.sentSkillUseIds.add(...), and setting
this.state.sessionStartSent = true) immediately after calling sendEvent(), but
sendEvent() can fail and returns/logs on errors; only mark events as sent when
sendEvent actually succeeded. Update sendEvent callers in analytics-service (the
spots around the user_message_sent, skill_use, and session_start flows) to check
the boolean/result returned by sendEvent() and only execute
this.sentUserMessageIds.add(...), this.sentSkillUseIds.add(...), or set
this.state.sessionStartSent = true when sendEvent() indicates success; keep the
rest of the logic unchanged so transient Amplitude failures do not permanently
drop events.
- Around line 324-375: readResolvedSkills currently iterates
Object.values(parsed) and merges resolvedSkills from all sessions; change its
signature (readResolvedSkills) to accept the current sessionId and only read the
snapshot for that key from the parsed sessions JSON (use parsed[sessionId]
instead of Object.values(parsed)), then validate that entry's skillsSnapshot and
resolvedSkills (resolvedSkills, name, filePath, source) exactly as before;
update callers to pass the session.id; reference sessionsJsonPath, parsed,
snapshot, and resolvedSkills when making this selective lookup and preserving
existing type/validation checks.

In `@apps/web/src/components/channel-connect-modal.tsx`:
- Around line 215-229: The catch/rejection path of the Channel connect flow
isn't reporting failures to analytics — update the error-handling so that
whenever a postApiV1Channels*Connect call rejects you call
track("workspace_channel_config_submit", { channel: channelType, success: false
}) before showing the toast (i.e., add the same track call used in the 409/error
branch into the catch/rejection branch where the toast is currently shown),
ensuring the channelType value used matches the existing variable and keep the
existing toast behavior.

In `@apps/web/src/components/skills/import-skill-modal.tsx`:
- Around line 85-89: After successfully calling
importMutation.mutateAsync(selectedFile) (the const result = await
importMutation.mutateAsync(selectedFile) block), guard the analytics call
track(...) so it cannot throw and block the success UX: wrap the
track("workspace_skill_enable", { name: result.slug ?? "unknown_skill",
skill_source: "custom" }) call in its own try/catch (or Promise.catch if async)
and swallow/log analytics errors, ensuring the subsequent success handling
(closing the modal/updating state) always runs regardless of track failures.

---

Outside diff comments:
In `@apps/web/src/components/channel-setup/feishu-setup-view.tsx`:
- Around line 151-166: The try/catch currently wraps both the API call and the
post-success UI/analytics, so if onConnected() or UI code throws after a
successful API call it incorrectly records a failed submit; refactor by limiting
the try/catch to the API call (postApiV1ChannelsFeishuConnect) and any analytics
tracking (track calls), then run toast.success, identify, and onConnected()
outside that try block (or add a separate try around analytics only) so that a
post-success UI error cannot trigger the failure branch that emits
workspace_channel_config_submit with success: false.

---

Duplicate comments:
In `@apps/controller/src/services/analytics-service.ts`:
- Around line 13-17: AnalyticsState currently uses a single boolean
sessionStartSent which causes a single global session_start to block all later
sessions; change AnalyticsState to track session-scoped state (e.g.,
sessionStartSentBySession: Record<string, boolean> and
sentUserMessageIdsBySession: Record<string, string[]> and
sentSkillUseIdsBySession: Record<string, string[]>), key entries by session.id
(or first user-message id) and update the logic that checks/sets
sessionStartSent, sentUserMessageIds, and sentSkillUseIds to read/write the
per-session maps (ensure removal/cleanup when sessions end or expire); update
any functions referencing sessionStartSent, sentUserMessageIds, or
sentSkillUseIds to use the session-keyed maps instead.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/.vitepress/theme/ThemeToggle.vue`:
- Around line 7-9: The injected toggle handler is named _toggleAppearance but
the template calls toggleAppearance, causing an undefined click handler; fix by
making the names consistent—either rename the injected constant
_toggleAppearance to toggleAppearance (update the inject call that currently
uses _toggleAppearance) or change the template to call _toggleAppearance—ensure
the inject("toggle-appearance", ...) result and the template click handler
reference the same identifier.