fix: enforce absolute log paths, and name the real cause when one cannot be opened

CHANGELOG.md

@@ -36,6 +36,11 @@ _If you are upgrading from 1.x: please see [Migrating from 1.x](README.md#migrat
 - Declare `execute_anonymous` destructive, so clients stop treating it as safe to run unprompted ([#52])
 - Stop `analyze_apex_log_performance` reporting that performance looks good on a log that exhausted the CPU limit ([#86])
 - Report the same `totalMethods` from all three analysis tools on an unfiltered call; `get_apex_log_summary` did not count entry points, so it reported fewer methods than the other two ([#88])
+- Warn when a caller-given `execute_anonymous` `outputDir` resolves outside every root the client declared. The log is still written, and the response names where it went ([#109])
+- Close cleanly on `SIGTERM`, so a supervised restart or a container stop no longer kills the server mid-shutdown ([#109])
+- Return an absolute `filePath` from `execute_anonymous`, so the path it hands back is one the analysis tools accept. A relative `outputDir` now anchors to the project root, the same base the default uses ([#109])
+- Refuse a relative `logFilePath` instead of resolving it against the server's working directory, which is where the client spawned the server and not where the caller is ([#109])
+- Name the real cause when a log file cannot be opened. A permission error, a directory in place of a file, or an exhausted descriptor table were all reported as "Log file not found", sending the caller to look for a file that was there ([#109])
 
 ## [1.0.0] - 2026-03-20
 
@@ -55,3 +60,4 @@ _If you are upgrading from 1.x: please see [Migrating from 1.x](README.md#migrat
 [#86]: https://github.com/certinia/debug-log-analyzer-mcp/issues/86
 [#87]: https://github.com/certinia/debug-log-analyzer-mcp/issues/87
 [#88]: https://github.com/certinia/debug-log-analyzer-mcp/issues/88
+[#109]: https://github.com/certinia/debug-log-analyzer-mcp/issues/109

CLAUDE.md

@@ -88,9 +88,13 @@ The server provides four main capabilities:
 
 Log analysis tools (1-3) accept absolute file paths to `.log` files and return structured JSON for AI processing.
 
+## Naming
+
+Every tool, parameter and response field follows the rules in [DEVELOPING.md](DEVELOPING.md#-naming-tools-and-fields) — read them before you add or rename one. In short: prefix every tool `apexlog_`; the verb states the shape of the result (`get_` one, `list_` many, `search_` many matched to a caller query, `create`/`update`/`delete`/`write_` one resource written, `execute`/`run_` an effect outside the server) and the noun states what the result is; `analyze`, `process`, `handle`, `manage`, `find`, `detect`, `check` and `fetch` are banned; fields name the fact and not the calculation, carry their unit (`durationSelfMs`, `fileSizeBytes`), use `total` only for "including children" and `self` only for "excluding them" (so `durationTotalMs` names a log's duration and a row's alike), keep one name per fact across all tools, count as `<noun>Count`, fold acronyms in lowerCamel, and state booleans as bare adjectives (`truncated`).
+
 ## Response Shaping
 
-Responses are TOON-encoded and deliberately lean, but the saving comes from shape, never from dropping a fact — see the conventions in [DEVELOPING.md](DEVELOPING.md#️-shaping-tool-responses) and the helpers in `src/tools/responseShaping.ts`. In short: restructure before you delete (flatten nested objects into TOON tables — `toLimitRows` is the worked example, 45% cheaper than the nested form and still complete); always report a fixed-schema field even at zero, because an absent count cannot be told apart from one that was never parsed; use `omitEmpty` **only** for occurrence lists, where absence unambiguously means nothing happened; never report the same figure twice; never echo the caller's input back; round durations and percentages (`roundMs`/`roundPercent`); and keep every row of a table on the same key set so TOON keeps its one-header-plus-one-line-per-row form.
+Responses are TOON-encoded and deliberately lean, but the saving comes from shape, never from dropping a fact — see the conventions in [DEVELOPING.md](DEVELOPING.md#️-shaping-tool-responses) and the helpers in `src/tools/responseShaping.ts`. In short: restructure before you delete (flatten nested objects into TOON tables — `toLimitRows` is the worked example, 45% cheaper than the nested form and still complete); always report a fixed-schema field even at zero, because an absent count cannot be told apart from one that was never parsed; use `omitEmpty` **only** for occurrence lists, where absence unambiguously means nothing happened; never report the same figure twice; never state what the caller can derive from the numbers beside it; never echo the caller's input back; round durations and percentages (`roundMs`/`roundPercent`); and keep every row of a table on the same key set so TOON keeps its one-header-plus-one-line-per-row form.
 
 Concretely: `analyze_apex_log_performance` returns no prose `summary` — its one unique fact is the scalar `topMethodsSelfPercentage` — and omits `recommendations` only when nothing stands out; `get_apex_log_summary` returns no `file`, all thirteen governor limits as `{name, used, limit}` rows, the full `debugLevels` list, and omits only `logIssues`; `find_performance_bottlenecks` excludes from `governorLimitWarnings` any limit already detailed by a dedicated section; `execute_anonymous` emits the `.gitignore` tip only when it created the output directory.
 

DEVELOPING.md

@@ -12,9 +12,10 @@ Welcome to the development guide for the **Apex Log MCP Server**. This document
 2. [Setting Up the Development Environment](#-setting-up-the-development-environment)
 3. [Building](#-building)
 4. [Running the Server Locally](#-running-the-server-locally)
-5. [Shaping Tool Responses](#️-shaping-tool-responses)
-6. [Shaping Tool Definitions](#️-shaping-tool-definitions)
-7. [Testing Your Changes](#-testing-your-changes)
+5. [Naming Tools and Fields](#-naming-tools-and-fields)
+6. [Shaping Tool Responses](#️-shaping-tool-responses)
+7. [Shaping Tool Definitions](#️-shaping-tool-definitions)
+8. [Testing Your Changes](#-testing-your-changes)
 
 ## 🔧 Prerequisites
 
@@ -108,6 +109,60 @@ Once you’ve built the server or run the watcher, you can run the MCP server fo
 
    To disable Apex execution altogether, use `--no-apex-execution`. See the [README](README.md#production-safety) for the full policy.
 
+## 🔤 Naming Tools and Fields
+
+A name has to be decidable: for any new tool or field, exactly one name follows from these rules. The
+reader is an agent that cannot ask what a name means.
+
+### Prefix every tool with `apexlog_`
+
+Unprefixed names collide across servers. `get_issue` and `list_issues` ship in both the GitHub and the
+Sentry server; `search_files` and `read_file` in both Filesystem and Google Drive. A client with two of
+those loaded cannot tell them apart.
+
+One unbroken unit — `apexlog_`, not `apex_log_`, as `slack_` is — so the boundary between the namespace
+and the verb is visible.
+
+### The verb states the shape of the result
+
+The result is what the caller plans around. The work is invisible to it.
+
+| Verb | The caller gets back |
+| --- | --- |
+| `get_` | exactly one thing, identified by the input |
+| `list_` | a collection; filters, thresholds and ranking are allowed |
+| `search_` | a collection matched to a query the caller supplies |
+| `create_` / `update_` / `delete_` / `write_` | one resource, written |
+| `execute_` / `run_` | an effect outside this server |
+
+A filter does not make it a `search_` — Sentry's `list_issues` and GitHub's `list_pull_requests` both
+take filters. `search_` is for a caller's query string.
+
+Banned: `analyze`, `process`, `handle` and `manage`, because they name work, so two tools can both
+claim them; `find`, `detect`, `check` and `fetch`, because they are synonyms of the verbs above.
+
+The noun states what the result is, in the caller's words: `slow_operations`, not `timed_nodes`.
+
+A bare noun (`apexlog_summary`) is shorter, and `git_status` shows it can work — but only because git's
+subcommands *are* its vocabulary, so `status` reads as a verb there. Ours is not, and a bare noun
+cannot say whether one thing or many come back.
+
+### Fields
+
+1. **Name the fact, not the calculation**: `returnedSelfPercentage`, not `coveredSelfPercentage`.
+2. **Carry the unit** when the type cannot: `durationSelfMs`, `fileSizeBytes`.
+3. **`total` always means "including children", `self` always means "excluding them".** Never "summed
+   across rows". A log's duration *is* its root frame's, so `durationTotalMs` names it at every scope,
+   and the parser's own `duration.total` / `duration.self` reach the wire unrenamed.
+4. **One name per fact, in every tool.** A word may still serve two unrelated facts where position
+   prevents confusion: `limit` is both the input row count and the column naming which governor limit
+   a row is about.
+5. **Counts are `<singularNoun>Count`**: `soqlCount`, `dmlRowCount`. Not `totalX`, not a plural alone.
+6. **lowerCamel, acronyms folded**: `soqlCount`.
+7. **Booleans are bare adjectives that read true**: `truncated`, `succeeded`. No `isX`, no `hasX`.
+8. **An input names what it limits, on the axis it acts on**: `limit`, `minSelfMs`. `minDuration`
+   filtered on total time while the ranking used self time, and the name hid it.
+
 ## ✂️ Shaping Tool Responses
 
 Every token a tool returns is a token the agent cannot spend on reasoning, so responses are kept as
@@ -144,6 +199,11 @@ one-liners.
   value in two sections. Recommendations say what to *do*; the numbers stay in the data. Where prose
   carried a fact the table could not, replace it with a scalar rather than deleting it —
   `topMethodsSelfPercentage` is ~8 tokens where the paragraph it replaced was ~55.
+- **Don't state what the caller can derive.** A sentence earns its tokens only if it carries a fact the
+  numbers do not. "No bottlenecks found" follows from an always-present empty list, so the fix is a
+  complete shape, not a sentence; "High CPU usage — consider optimizing algorithms" follows from the
+  percentage beside it. Advice built from one column and a hardcoded threshold is a worse copy of what
+  the agent does anyway, because the agent reads every column.
 - **Don't echo the input back.** If the caller supplied it (a file path, a flag), it does not belong
   in the response.
 - **Round to the precision someone acts on.** `roundMs` for durations (3dp, keeps microsecond

src/server.ts

@@ -67,7 +67,11 @@ class ApexLogServer {
       this.server.close();
       process.exit(0);
     };
+    // SIGTERM as well as SIGINT. A supervised restart, a container stop, and a
+    // client that ends a stdio server all send SIGTERM, and Node's default for
+    // it is to exit without running any of this.
     process.once("SIGINT", shutdown);
+    process.once("SIGTERM", shutdown);
   }
 
   private registerTools(): void {

src/tools/analyzeLogPerformance.ts

@@ -5,7 +5,12 @@
 import { z } from "zod";
 import { ApexLog } from "../ApexLogParser.js";
 import { encode } from "@toon-format/toon";
-import { loadApexLog, isMethodNode, walkLog } from "./apexLogSource.js";
+import {
+  loadApexLog,
+  isMethodNode,
+  walkLog,
+  logFilePathSchema,
+} from "./apexLogSource.js";
 import {
   NS_TO_MS,
   omitEmpty,
@@ -14,9 +19,7 @@ import {
 } from "./responseShaping.js";
 
 export const analyzeLogPerformanceInputSchema = {
-  logFilePath: z
-    .string()
-    .describe("Absolute path to the Apex debug log file (.log)"),
+  logFilePath: logFilePathSchema,
   topMethods: z
     .number()
     .optional()

src/tools/apexLogSource.ts

@@ -3,13 +3,30 @@
  */
 
 import { promises as fs, type BigIntStats } from "fs";
+import { isAbsolute } from "path";
+import { z } from "zod";
 import {
   parse,
   ApexLog,
   LogLine,
   type LogSubCategory,
 } from "../ApexLogParser.js";
 
+/**
+ * The one declaration of the log path, shared by every tool that takes one, so
+ * all three enforce it the same way.
+ *
+ * A relative path is refused rather than resolved: it would resolve against the
+ * server's working directory, which is where the client happened to spawn us
+ * and not where the caller is. Resolving would read a different file, or none,
+ * and report neither. Refinements do not reach the JSON schema, so this costs
+ * no tokens in the tool definition — `pnpm run eval` holds that to its budget.
+ */
+export const logFilePathSchema = z
+  .string()
+  .refine(isAbsolute, "must be an absolute path")
+  .describe("Absolute path to the Apex debug log file (.log)");
+
 type CachedLog = {
   path: string;
   fingerprint: string;
@@ -79,9 +96,18 @@ export async function loadApexLog(logFilePath: string): Promise<ApexLog> {
   try {
     handle = await fs.open(logFilePath, "r");
     fingerprint = fingerprintOf(await handle.stat({ bigint: true }));
-  } catch {
+  } catch (error) {
     await handle?.close();
-    throw new Error(`Log file not found: ${logFilePath}`);
+    // A missing file is one of several ways this fails. Reporting all of them
+    // as "not found" sends the caller to look for a file that is there, when
+    // the real cause was a permission, a directory in place of a file, or a
+    // full descriptor table. Name the cause, and keep the original as `cause`.
+    const code = (error as NodeJS.ErrnoException).code ?? String(error);
+    const message =
+      code === "ENOENT"
+        ? `Log file not found: ${logFilePath}`
+        : `Cannot read log file ${logFilePath}: ${code}`;
+    throw new Error(message, { cause: error });
   }
 
   try {

src/tools/executeAnonymous.ts

@@ -109,14 +109,51 @@ export function executeAnonymousToolConfig(apexExecutionDisabled = false) {
   };
 }
 
-async function getProjectPath(server: McpServer): Promise<string | undefined> {
+async function getRootPaths(server: McpServer): Promise<string[]> {
   try {
     const { roots } = await server.server.listRoots();
-    const rootUri = roots[0]?.uri;
-    return rootUri ? new URL(rootUri).pathname : undefined;
+    return roots.map((root) => new URL(root.uri).pathname);
   } catch {
+    return [];
+  }
+}
+
+/** The resolved path, or the path itself when it does not resolve. */
+async function realPathOrSelf(target: string): Promise<string> {
+  return fs.realpath(target).catch(() => target);
+}
+
+/**
+ * The MCP spec expects a server to work inside the roots the client declares,
+ * and `outputDir` is agent-supplied, so it is the path an injected instruction
+ * takes. Refusing would break a caller who means to write elsewhere, so say so
+ * instead: the response names where the log went, and the same line goes to
+ * stderr for the person watching the server.
+ *
+ * Symlinks are followed on both sides, so a link inside a root that points out
+ * of one is still outside. A client that declares no roots gives nothing to
+ * compare against, so it stays silent.
+ */
+async function warnIfOutsideRoots(
+  outputDir: string,
+  rootPaths: string[],
+): Promise<string | undefined> {
+  if (rootPaths.length === 0) {
     return undefined;
   }
+
+  const target = await realPathOrSelf(outputDir);
+  const roots = await Promise.all(rootPaths.map(realPathOrSelf));
+  const inside = roots.some(
+    (root) => target === root || target.startsWith(root + path.sep),
+  );
+  if (inside) {
+    return undefined;
+  }
+
+  const warning = `Debug log written to ${target}, which is outside every root this client declared.`;
+  console.error(`[apex-log-mcp] ${warning}`);
+  return warning;
 }
 
 async function getAliasForUsername(
@@ -146,7 +183,8 @@ export async function executeAnonymous(
     return toolError(APEX_EXECUTION_DISABLED_MESSAGE);
   }
 
-  const projectPath = await getProjectPath(server);
+  const rootPaths = await getRootPaths(server);
+  const projectPath = rootPaths[0];
 
   const org = await resolveOrg(projectPath, targetOrg);
   const connection = org.getConnection();
@@ -206,8 +244,14 @@ export async function executeAnonymous(
   const logId = logRecord.Id;
   const logBody = await connection.request(`/sobjects/ApexLog/${logId}/Body/`);
 
-  const outputDir =
-    args.outputDir ?? path.join(projectPath ?? process.cwd(), ".apex-log-mcp");
+  // Absolute, because `filePath` below goes straight back to the analysis
+  // tools, which refuse a relative path. A relative `outputDir` anchors to the
+  // project root, the same base the default uses, rather than to wherever the
+  // client happened to spawn this server.
+  const outputDir = path.resolve(
+    projectPath ?? process.cwd(),
+    args.outputDir ?? ".apex-log-mcp",
+  );
   // Resolves to the first directory created, or undefined when it already existed,
   // which is exactly when the .gitignore tip below is worth its tokens.
   const createdDir = await fs.mkdir(outputDir, { recursive: true });
@@ -216,12 +260,19 @@ export async function executeAnonymous(
   await fs.writeFile(filePath, logBody as string, "utf-8");
   const stats = await fs.stat(filePath);
 
+  // Only for a caller-given directory: the default is inside the project root
+  // by construction, so checking it could only ever say the obvious.
+  const warning = args.outputDir
+    ? await warnIfOutsideRoots(outputDir, rootPaths)
+    : undefined;
+
   return {
     content: [
       {
         type: "text" as const,
         text: encode({
           filePath,
+          ...(warning && { warning }),
           fileSizeBytes: stats.size,
           org: orgLabel,
           orgType: classification,

src/tools/findPerformanceBottlenecks.ts

@@ -6,13 +6,11 @@ import { z } from "zod";
 import { ApexLog } from "../ApexLogParser.js";
 import { type SlowMethod, extractMethods } from "./analyzeLogPerformance.js";
 import { encode } from "@toon-format/toon";
-import { loadApexLog } from "./apexLogSource.js";
+import { loadApexLog, logFilePathSchema } from "./apexLogSource.js";
 import { NS_TO_MS, roundMs, roundPercent } from "./responseShaping.js";
 
 export const findPerformanceBottlenecksInputSchema = {
-  logFilePath: z
-    .string()
-    .describe("Absolute path to the Apex debug log file (.log)"),
+  logFilePath: logFilePathSchema,
   analysisType: z
     .enum(["cpu", "database", "methods", "all"])
     .optional()

src/tools/getLogSummary.ts

@@ -5,7 +5,12 @@
 import { z } from "zod";
 import { ApexLog } from "../ApexLogParser.js";
 import { encode } from "@toon-format/toon";
-import { loadApexLog, isMethodNode, walkLog } from "./apexLogSource.js";
+import {
+  loadApexLog,
+  isMethodNode,
+  walkLog,
+  logFilePathSchema,
+} from "./apexLogSource.js";
 import {
   NS_TO_MS,
   omitEmpty,
@@ -14,9 +19,7 @@ import {
 } from "./responseShaping.js";
 
 export const getLogSummaryInputSchema = {
-  logFilePath: z
-    .string()
-    .describe("Absolute path to the Apex debug log file (.log)"),
+  logFilePath: logFilePathSchema,
 };
 
 export type LogSummaryArgs = z.infer<

tests/analyzeLogPerformance.test.ts

@@ -77,7 +77,9 @@ describe("analyzeLogPerformance", () => {
   describe("File Validation", () => {
     it("should throw error when file does not exist", async () => {
       const args: AnalyzeLogArgs = { logFilePath: "/nonexistent/file.log" };
-      mockedFs.stat.mockRejectedValue(new Error("File not found"));
+      mockedFs.stat.mockRejectedValue(
+        Object.assign(new Error("ENOENT"), { code: "ENOENT" }),
+      );
 
       await expect(analyzeLogPerformance(args)).rejects.toThrow(
         "Log file not found: /nonexistent/file.log",