fix: address remaining code review issues from PR #39

docs/docs/configure/config.md

@@ -57,6 +57,7 @@ Configuration is loaded from multiple sources, with later sources overriding ear
 | `skills` | `object` | Skill paths and URLs |
 | `plugin` | `string[]` | Plugin specifiers |
 | `instructions` | `string[]` | Glob patterns for instruction files |
+| `telemetry` | `object` | Telemetry settings (see [Telemetry](telemetry.md)) |
 | `compaction` | `object` | Context compaction settings (see [Context Management](context-management.md)) |
 | `experimental` | `object` | Experimental feature flags |
 

docs/docs/configure/telemetry.md

@@ -47,7 +47,7 @@ Telemetry helps us:
 
 ## Disabling Telemetry
 
-To disable all telemetry collection, add this to your configuration file (`~/.config/altimate/config.json`):
+To disable all telemetry collection, add this to your configuration file (`~/.config/altimate-code/altimate-code.json`):
 
 ```json
 {

packages/altimate-code/src/command/index.ts

@@ -118,21 +118,17 @@ export namespace Command {
           source: "mcp",
           description: prompt.description,
           get template() {
-            // since a getter can't be async we need to manually return a promise here
-            return new Promise<string>(async (resolve, reject) => {
-              const template = await MCP.getPrompt(
-                prompt.client,
-                prompt.name,
-                prompt.arguments
-                  ? // substitute each argument with $1, $2, etc.
-                    Object.fromEntries(prompt.arguments?.map((argument, i) => [argument.name, `$${i + 1}`]))
-                  : {},
-              ).catch(reject)
-              resolve(
-                template?.messages
-                  .map((message) => (message.content.type === "text" ? message.content.text : ""))
-                  .join("\n") || "",
-              )
+            return MCP.getPrompt(
+              prompt.client,
+              prompt.name,
+              prompt.arguments
+                ? Object.fromEntries(prompt.arguments.map((argument, i) => [argument.name, `$${i + 1}`]))
+                : {},
+            ).then((template) => {
+              if (!template) throw new Error(`Failed to load MCP prompt: ${prompt.name}`)
+              return template.messages
+                .map((message) => (message.content.type === "text" ? message.content.text : ""))
+                .join("\n")
             })
           },
           hints: prompt.arguments?.map((_, i) => `$${i + 1}`) ?? [],

packages/altimate-code/src/mcp/index.ts

@@ -29,6 +29,12 @@ export namespace MCP {
   const log = Log.create({ service: "mcp" })
   const DEFAULT_TIMEOUT = 30_000
 
+  const registeredMcpTools = new Set<string>()
+
+  export function isMcpTool(name: string): boolean {
+    return registeredMcpTools.has(name)
+  }
+
   export const Resource = z
     .object({
       name: z.string(),
@@ -215,7 +221,7 @@ export namespace MCP {
 
   // Helper function to fetch prompts for a specific client
   async function fetchPromptsForClient(clientName: string, client: Client) {
-    const prompts = await client.listPrompts().catch((e) => {
+    const prompts = await withTimeout(client.listPrompts(), DEFAULT_TIMEOUT).catch((e) => {
       log.error("failed to get prompts", { clientName, error: e.message })
       return undefined
     })
@@ -237,7 +243,7 @@ export namespace MCP {
   }
 
   async function fetchResourcesForClient(clientName: string, client: Client) {
-    const resources = await client.listResources().catch((e) => {
+    const resources = await withTimeout(client.listResources(), DEFAULT_TIMEOUT).catch((e) => {
       log.error("failed to get prompts", { clientName, error: e.message })
       return undefined
     })
@@ -683,6 +689,7 @@ export namespace MCP {
       }),
     )
 
+    registeredMcpTools.clear()
     for (const { clientName, client, toolsResult } of toolsResults) {
       if (!toolsResult) continue
       const mcpConfig = config[clientName]
@@ -691,7 +698,9 @@ export namespace MCP {
       for (const mcpTool of toolsResult.tools) {
         const sanitizedClientName = clientName.replace(/[^a-zA-Z0-9_-]/g, "_")
         const sanitizedToolName = mcpTool.name.replace(/[^a-zA-Z0-9_-]/g, "_")
-        result[sanitizedClientName + "_" + sanitizedToolName] = await convertMcpTool(mcpTool, client, timeout)
+        const toolName = sanitizedClientName + "_" + sanitizedToolName
+        registeredMcpTools.add(toolName)
+        result[toolName] = await convertMcpTool(mcpTool, client, timeout)
       }
     }
     return result

packages/altimate-code/src/session/compaction.ts

@@ -298,8 +298,12 @@ When constructing the summary, try to stick to this template:
         },
       })
     }
-    if (processor.message.error) return "stop"
+    if (processor.message.error) {
+      compactionAttempts.delete(input.sessionID)
+      return "stop"
+    }
     Bus.publish(Event.Compacted, { sessionID: input.sessionID })
+    compactionAttempts.delete(input.sessionID)
     return "continue"
   }
 

packages/altimate-code/src/session/processor.ts

@@ -16,6 +16,7 @@ import { SessionCompaction } from "./compaction"
 import { PermissionNext } from "@/permission/next"
 import { Question } from "@/question"
 import { Telemetry } from "@/telemetry"
+import { MCP } from "@/mcp"
 
 export namespace SessionProcessor {
   const DOOM_LOOP_THRESHOLD = 3
@@ -212,7 +213,7 @@ export namespace SessionProcessor {
                         attachments: value.output.attachments,
                       },
                     })
-                    const toolType = match.tool.startsWith("mcp__") ? "mcp" as const : "standard" as const
+                    const toolType = MCP.isMcpTool(match.tool) ? "mcp" as const : "standard" as const
                     Telemetry.track({
                       type: "tool_call",
                       timestamp: Date.now(),
@@ -241,14 +242,14 @@ export namespace SessionProcessor {
                       state: {
                         status: "error",
                         input: value.input ?? match.state.input,
-                        error: (value.error as any).toString(),
+                        error: (value.error instanceof Error ? value.error.message : String(value.error)).slice(0, 1000),
                         time: {
                           start: match.state.time.start,
                           end: Date.now(),
                         },
                       },
                     })
-                    const errToolType = match.tool.startsWith("mcp__") ? "mcp" as const : "standard" as const
+                    const errToolType = MCP.isMcpTool(match.tool) ? "mcp" as const : "standard" as const
                     Telemetry.track({
                       type: "tool_call",
                       timestamp: Date.now(),
@@ -261,7 +262,7 @@ export namespace SessionProcessor {
                       duration_ms: Date.now() - match.state.time.start,
                       sequence_index: toolCallCounter,
                       previous_tool: previousTool,
-                      error: (value.error as any).toString().slice(0, 500),
+                      error: (value.error instanceof Error ? value.error.message : String(value.error)).slice(0, 500),
                     })
                     toolCallCounter++
                     previousTool = match.tool
@@ -345,7 +346,7 @@ export namespace SessionProcessor {
                     duration_ms: Date.now() - stepStartTime,
                   })
                   // Context utilization tracking
-                  const totalTokens = usage.tokens.input + usage.tokens.output + usage.tokens.cache.read + usage.tokens.cache.write
+                  const totalTokens = usage.tokens.input + usage.tokens.output + usage.tokens.cache.read
                   const contextLimit = input.model.limit?.context ?? 0
                   if (contextLimit > 0) {
                     const cacheRead = usage.tokens.cache.read

packages/altimate-code/src/session/prompt.ts

@@ -303,6 +303,18 @@ export namespace SessionPrompt {
     const session = await Session.get(sessionID)
     await Telemetry.init()
     Telemetry.setContext({ sessionId: sessionID, projectId: Instance.project?.id ?? "" })
+    const beforeExitHandler = () => {
+      Telemetry.track({
+        type: "session_end",
+        timestamp: Date.now(),
+        session_id: sessionID,
+        total_cost: sessionTotalCost,
+        total_tokens: sessionTotalTokens,
+        tool_call_count: toolCallCount,
+        duration_ms: Date.now() - sessionStartTime,
+      })
+    }
+    process.once("beforeExit", beforeExitHandler)
     try {
     while (true) {
       SessionStatus.set(sessionID, { type: "busy" })
@@ -796,6 +808,7 @@ export namespace SessionPrompt {
     }
     SessionCompaction.prune({ sessionID })
     } finally {
+      process.removeListener("beforeExit", beforeExitHandler)
       const outcome: "completed" | "abandoned" | "error" = abort.aborted
         ? "abandoned"
         : sessionHadError

packages/altimate-code/src/telemetry/index.ts

@@ -298,6 +298,7 @@ export namespace Telemetry {
   let sessionId = ""
   let projectId = ""
   let appInsights: AppInsightsConfig | undefined
+  let droppedEvents = 0
   let initPromise: Promise<void> | undefined
   let initDone = false
 
@@ -345,9 +346,9 @@ export namespace Telemetry {
         time: new Date(timestamp).toISOString(),
         iKey: cfg.iKey,
         tags: {
-          "ai.session.id": sid,
+          "ai.session.id": sid || "startup",
           "ai.user.id": userEmail,
-          "ai.cloud.role": "altimate-code",
+          "ai.cloud.role": "altimate",
           "ai.application.ver": Installation.VERSION,
         },
         data: {
@@ -439,6 +440,7 @@ export namespace Telemetry {
     buffer.push(event)
     if (buffer.length > MAX_BUFFER_SIZE) {
       buffer.shift()
+      droppedEvents++
     }
   }
 
@@ -447,6 +449,18 @@ export namespace Telemetry {
 
     const events = buffer.splice(0, buffer.length)
 
+    if (droppedEvents > 0) {
+      events.push({
+        type: "error",
+        timestamp: Date.now(),
+        session_id: sessionId,
+        error_name: "TelemetryBufferOverflow",
+        error_message: `${droppedEvents} events dropped due to buffer overflow`,
+        context: "telemetry",
+      } as Event)
+      droppedEvents = 0
+    }
+
     const controller = new AbortController()
     const timeout = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS)
     try {
@@ -475,6 +489,7 @@ export namespace Telemetry {
     enabled = false
     appInsights = undefined
     buffer = []
+    droppedEvents = 0
     sessionId = ""
     projectId = ""
     initPromise = undefined

packages/altimate-code/src/tool/project-scan.ts

@@ -6,6 +6,7 @@ import path from "path"
 import { Telemetry } from "@/telemetry"
 import { Config } from "@/config/config"
 import { Flag } from "@/flag/flag"
+import { Skill } from "../skill"
 
 // --- Types ---
 
@@ -572,6 +573,8 @@ export const ProjectScanTool = Tool.define("project_scan", {
     if (Flag.ALTIMATE_CLI_ENABLE_EXA) enabledFlags.push("exa")
     if (Flag.ALTIMATE_CLI_ENABLE_QUESTION_TOOL) enabledFlags.push("question_tool")
 
+    const skillCount = await Skill.all().then(s => s.length).catch(() => 0)
+
     Telemetry.track({
       type: "environment_census",
       timestamp: Date.now(),
@@ -585,7 +588,7 @@ export const ProjectScanTool = Tool.define("project_scan", {
       dbt_test_count_bucket: dbtManifest ? Telemetry.bucketCount(dbtManifest.test_count) : "0",
       connection_sources: connectionSources,
       mcp_server_count: mcpServerCount,
-      skill_count: 0,
+      skill_count: skillCount,
       os: process.platform,
       feature_flags: enabledFlags,
     })

packages/altimate-code/src/tool/truncation.ts

@@ -37,7 +37,12 @@ export namespace Truncate {
     const entries = await fs.readdir(DIR).catch(() => [] as string[])
     for (const entry of entries) {
       if (!entry.startsWith("tool_")) continue
-      if (Identifier.timestamp(entry) >= cutoff) continue
+      try {
+        if (Identifier.timestamp(entry) >= cutoff) continue
+      } catch {
+        // Skip malformed IDs (e.g. legacy format or descending IDs)
+        continue
+      }
       await fs.unlink(path.join(DIR, entry)).catch(() => {})
     }
   }