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,27 @@ export namespace SessionPrompt {
  const session = await Session.get(sessionID)
  await Telemetry.init()
  Telemetry.setContext({ sessionId: sessionID, projectId: Instance.project?.id ?? "" })
+ let emergencySessionEndFired = false
+ const emergencySessionEnd = () => {
+ if (emergencySessionEndFired) return
+ emergencySessionEndFired = true
+ 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,
+ })
+ }
+ // beforeExit covers event-loop drain without entering the session loop.
+ // exit covers process.exit() calls (sync only — track() buffers, flush is best-effort).
+ // SIGINT/SIGTERM are NOT handled here: the abort controller already triggers
+ // loop exit → finally block → normal session_end. Adding signal handlers
+ // would interfere with the default termination behavior.
+ process.once("beforeExit", emergencySessionEnd)
+ process.once("exit", emergencySessionEnd)
  try {
  while (true) {
  SessionStatus.set(sessionID, { type: "busy" })
@@ -796,6 +817,8 @@ export namespace SessionPrompt {
  }
  SessionCompaction.prune({ sessionID })
  } finally {
+ process.removeListener("beforeExit", emergencySessionEnd)
+ process.removeListener("exit", emergencySessionEnd)
  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(() => {})
  }
  }