# MikroMCP v1.4 — System Administration Depth Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** Add 11 new tools covering user groups, firmware upgrades, config backup/export, log rule management, and NTP write — completing the "System Administration Depth" milestone.

**Architecture:** Each subsystem gets its own `src/domain/tools/<name>-tools.ts` file and a matching test file under `test/unit/domain/`. `manage_ntp_client` is added to the existing `network-services-tools.ts`. All new files follow the established pattern: Zod `.strict()` schema, idempotency check before write, dry-run on every write action, `enrichError` in every catch.

**Tech Stack:** TypeScript (ESM), Zod, Vitest, RouterOS REST API via `RouterOSRestClient`, `MikroMCPError` / `enrichError`.

**New tools (88 → 99):**
- `list_user_groups`, `manage_user_group`
- `get_upgrade_status`, `manage_upgrade`
- `create_backup`, `export_config`
- `list_log_rules`, `manage_log_rule`, `list_log_actions`, `manage_log_action`
- `manage_ntp_client`

---

## File Map

| Action | Path |
|---|---|
| Create | `src/domain/tools/user-group-tools.ts` |
| Create | `src/domain/tools/upgrade-tools.ts` |
| Create | `src/domain/tools/backup-tools.ts` |
| Create | `src/domain/tools/log-tools.ts` |
| Modify | `src/domain/tools/network-services-tools.ts` (add `manage_ntp_client`) |
| Modify | `src/domain/tools/index.ts` (register 4 new arrays) |
| Create | `test/unit/domain/user-group-tools.test.ts` |
| Create | `test/unit/domain/upgrade-tools.test.ts` |
| Create | `test/unit/domain/backup-tools.test.ts` |
| Create | `test/unit/domain/log-tools.test.ts` |
| Modify | `test/unit/domain/network-services-tools.test.ts` (add manage_ntp_client tests) |
| Modify | `docs/wiki/Available-Tools.md` |
| Modify | `README.md` |
| Modify | `CHANGELOG.md` |

---

## Task 1: User Group Tools

**Files:**
- Create: `src/domain/tools/user-group-tools.ts`
- Create: `test/unit/domain/user-group-tools.test.ts`

RouterOS path: `user/group`. Fields: `name`, `policy` (comma-separated list, e.g. `"read,write,ftp"`), `skin`. Idempotency key for `add`: name. `update` action changes the policy list.

- [ ] **Step 1: Write the failing test**

```typescript
// test/unit/domain/user-group-tools.test.ts
import { describe, it, expect, vi } from "vitest";
import { userGroupTools } from "../../../src/domain/tools/user-group-tools.js";
import type { ToolContext } from "../../../src/domain/tools/tool-definition.js";
import type { RouterOSRestClient } from "../../../src/adapter/rest-client.js";
import type { RouterConfig, Identity } from "../../../src/types.js";
import type { SshClient } from "../../../src/adapter/ssh-client.js";
import type { FtpClient } from "../../../src/adapter/ftp-client.js";
import { ErrorCategory } from "../../../src/domain/errors/error-types.js";

function makeContext(groups: Record<string, unknown>[] = []): ToolContext {
  return {
    routerId: "test-router",
    correlationId: "test-corr",
    routerConfig: {} as RouterConfig,
    sshClient: {} as SshClient,
    ftpClient: {} as FtpClient,
    identity: {
      id: "superadmin-builtin",
      name: "admin",
      role: "superadmin",
      allowedRouters: ["*"],
      allowedToolPatterns: ["*"],
    } as unknown as Identity,
    routerClient: {
      get: vi.fn().mockResolvedValue(groups),
      create: vi.fn().mockResolvedValue({ ".id": "*1", name: "ops" }),
      update: vi.fn().mockResolvedValue(undefined),
      remove: vi.fn().mockResolvedValue(undefined),
    } as unknown as RouterOSRestClient,
  };
}

const [listTool, manageTool] = userGroupTools;

describe("userGroupTools", () => {
  describe("metadata", () => {
    it("exports 2 tools", () => expect(userGroupTools).toHaveLength(2));
    it("has correct names", () => {
      expect(listTool.name).toBe("list_user_groups");
      expect(manageTool.name).toBe("manage_user_group");
    });
    it("list_user_groups is readOnly", () => expect(listTool.annotations.readOnlyHint).toBe(true));
    it("manage_user_group is not readOnly", () => expect(manageTool.annotations.readOnlyHint).toBe(false));
    it("manage_user_group is not destructive", () => expect(manageTool.annotations.destructiveHint).toBe(false));
  });

  describe("input schema — list_user_groups", () => {
    it("parses valid input", () => {
      expect(listTool.inputSchema.safeParse({ routerId: "r1" }).success).toBe(true);
    });
    it("applies default limit 100", () => {
      expect(listTool.inputSchema.parse({ routerId: "r1" }).limit).toBe(100);
    });
    it("rejects extra fields", () => {
      expect(listTool.inputSchema.safeParse({ routerId: "r1", extra: true }).success).toBe(false);
    });
    it("rejects limit out of range", () => {
      expect(listTool.inputSchema.safeParse({ routerId: "r1", limit: 0 }).success).toBe(false);
    });
  });

  describe("input schema — manage_user_group", () => {
    it("parses valid add input", () => {
      expect(
        manageTool.inputSchema.safeParse({
          routerId: "r1", action: "add", name: "ops", policy: "read,write",
        }).success,
      ).toBe(true);
    });
    it("dryRun defaults to false", () => {
      expect(
        manageTool.inputSchema.parse({ routerId: "r1", action: "add", name: "ops" }).dryRun,
      ).toBe(false);
    });
    it("rejects extra fields", () => {
      expect(
        manageTool.inputSchema.safeParse({
          routerId: "r1", action: "add", name: "ops", extra: true,
        }).success,
      ).toBe(false);
    });
    it("rejects invalid action", () => {
      expect(
        manageTool.inputSchema.safeParse({ routerId: "r1", action: "enable", name: "ops" }).success,
      ).toBe(false);
    });
  });

  describe("handler — list_user_groups", () => {
    it("returns all groups", async () => {
      const ctx = makeContext([
        { ".id": "*1", name: "full", policy: "ftp,reboot,read,write,policy" },
        { ".id": "*2", name: "read", policy: "ftp,read,test,winbox" },
      ]);
      const result = await listTool.handler({ routerId: "test-router" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect((sc.groups as unknown[]).length).toBe(2);
      expect(sc.total).toBe(2);
    });

    it("throws enriched error on network failure", async () => {
      const ctx = makeContext([]);
      (ctx.routerClient.get as ReturnType<typeof vi.fn>).mockRejectedValue(new Error("net"));
      await expect(listTool.handler({ routerId: "test-router" }, ctx)).rejects.toThrow();
    });
  });

  describe("handler — manage_user_group add", () => {
    it("creates group when not found", async () => {
      const ctx = makeContext([]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "add", name: "ops", policy: "read,write" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("created");
      expect(ctx.routerClient.create).toHaveBeenCalledWith(
        "user/group",
        expect.objectContaining({ name: "ops", policy: "read,write" }),
      );
    });

    it("returns already_exists when group found with same policy", async () => {
      const ctx = makeContext([{ ".id": "*1", name: "ops", policy: "read,write" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "add", name: "ops", policy: "read,write" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("already_exists");
      expect(ctx.routerClient.create).not.toHaveBeenCalled();
    });

    it("throws CONFLICT when group found with different policy", async () => {
      const ctx = makeContext([{ ".id": "*1", name: "ops", policy: "read" }]);
      await expect(
        manageTool.handler(
          { routerId: "test-router", action: "add", name: "ops", policy: "read,write" },
          ctx,
        ),
      ).rejects.toMatchObject({ category: ErrorCategory.CONFLICT });
    });

    it("dry_run returns preview without calling create", async () => {
      const ctx = makeContext([]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "add", name: "ops", policy: "read", dryRun: true },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("dry_run");
      expect(ctx.routerClient.create).not.toHaveBeenCalled();
    });
  });

  describe("handler — manage_user_group update", () => {
    it("updates policy when group exists", async () => {
      const ctx = makeContext([{ ".id": "*1", name: "ops", policy: "read" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "update", name: "ops", policy: "read,write" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("updated");
      expect(ctx.routerClient.update).toHaveBeenCalledWith("user/group", "*1", { policy: "read,write" });
    });

    it("returns no_change when policy unchanged", async () => {
      const ctx = makeContext([{ ".id": "*1", name: "ops", policy: "read,write" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "update", name: "ops", policy: "read,write" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("no_change");
    });

    it("throws NOT_FOUND when group does not exist", async () => {
      const ctx = makeContext([]);
      await expect(
        manageTool.handler(
          { routerId: "test-router", action: "update", name: "ops", policy: "read" },
          ctx,
        ),
      ).rejects.toMatchObject({ category: ErrorCategory.NOT_FOUND });
    });
  });

  describe("handler — manage_user_group remove", () => {
    it("removes group when found", async () => {
      const ctx = makeContext([{ ".id": "*1", name: "ops", policy: "read" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "remove", name: "ops" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("removed");
      expect(ctx.routerClient.remove).toHaveBeenCalledWith("user/group", "*1");
    });

    it("returns not_found gracefully when already gone", async () => {
      const ctx = makeContext([]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "remove", name: "ops" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("not_found");
      expect(ctx.routerClient.remove).not.toHaveBeenCalled();
    });

    it("dry_run returns preview without calling remove", async () => {
      const ctx = makeContext([{ ".id": "*1", name: "ops", policy: "read" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "remove", name: "ops", dryRun: true },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("dry_run");
      expect(ctx.routerClient.remove).not.toHaveBeenCalled();
    });
  });
});
```

- [ ] **Step 2: Run test to verify it fails**

```bash
npx vitest run test/unit/domain/user-group-tools.test.ts
```

Expected: FAIL — "Cannot find module '../../../src/domain/tools/user-group-tools.js'"

- [ ] **Step 3: Implement `src/domain/tools/user-group-tools.ts`**

```typescript
import { z } from "zod";
import type { ToolDefinition, ToolContext, ToolResult } from "./tool-definition.js";
import type { RouterOSRecord } from "../../types.js";
import { enrichError } from "../errors/error-enricher.js";
import { MikroMCPError, ErrorCategory } from "../errors/error-types.js";
import { createLogger } from "../../observability/logger.js";

const log = createLogger("user-group-tools");

const listUserGroupsInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    limit: z
      .number()
      .int()
      .min(1)
      .max(500)
      .default(100)
      .describe("Maximum number of groups to return"),
  })
  .strict();

const listUserGroupsTool: ToolDefinition = {
  name: "list_user_groups",
  title: "List User Groups",
  description: "List local user groups on a MikroTik router with their policy bitmask.",
  inputSchema: listUserGroupsInputSchema,
  annotations: {
    readOnlyHint: true,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = listUserGroupsInputSchema.parse(params);
    log.info({ routerId: context.routerId }, "Listing user groups");
    try {
      const all = await context.routerClient.get<RouterOSRecord>("user/group", {
        limit: undefined,
        offset: undefined,
      });
      const groups = (all as Record<string, string>[]).slice(0, parsed.limit);
      return {
        content: `User groups on ${context.routerId}: ${groups.length} returned (${all.length} total)`,
        structuredContent: { routerId: context.routerId, groups, total: all.length, returned: groups.length },
      };
    } catch (err) {
      throw enrichError(err, { routerId: context.routerId, tool: "list_user_groups" });
    }
  },
};

const manageUserGroupInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    action: z
      .enum(["add", "update", "remove"])
      .describe("Action to perform"),
    name: z.string().describe("Group name — idempotency key"),
    policy: z
      .string()
      .optional()
      .describe(
        "Comma-separated RouterOS policy list (e.g. 'read,write,ftp'). Required for add. Used to update policy on update action.",
      ),
    skin: z.string().optional().describe("Winbox skin (optional)"),
    dryRun: z.boolean().default(false).describe("Preview changes without applying"),
  })
  .strict();

const manageUserGroupTool: ToolDefinition = {
  name: "manage_user_group",
  title: "Manage User Group",
  description:
    "Add, update, or remove a RouterOS user group. Idempotent by name: add returns already_exists if the group exists with the same policy; throws CONFLICT if it exists with a different policy. update returns no_change if the policy is unchanged.",
  inputSchema: manageUserGroupInputSchema,
  annotations: {
    readOnlyHint: false,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  snapshotPaths: ["user/group"],
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = manageUserGroupInputSchema.parse(params);
    log.info({ routerId: context.routerId, action: parsed.action, name: parsed.name }, "Managing user group");
    try {
      const all = await context.routerClient.get<RouterOSRecord>("user/group", {
        limit: undefined,
        offset: undefined,
      });
      const existing = (all as Record<string, string>[]).find((g) => g.name === parsed.name);

      if (parsed.action === "add") {
        if (existing) {
          if (existing.policy === (parsed.policy ?? "")) {
            return {
              content: `User group "${parsed.name}" already exists with the same policy. No changes made.`,
              structuredContent: { action: "already_exists", name: parsed.name, policy: existing.policy },
            };
          }
          throw new MikroMCPError({
            category: ErrorCategory.CONFLICT,
            code: "USER_GROUP_CONFLICT",
            message: `User group "${parsed.name}" exists with a different policy.`,
            details: { existingPolicy: existing.policy, requestedPolicy: parsed.policy },
            recoverability: {
              retryable: false,
              suggestedAction: "Remove the existing group first or use manage_user_group with action=update.",
              alternativeTools: ["manage_user_group"],
            },
          });
        }

        if (parsed.dryRun) {
          return {
            content: `Dry run: Would add user group "${parsed.name}" with policy "${parsed.policy ?? ""}".`,
            structuredContent: {
              action: "dry_run",
              diff: [
                { property: "name", before: null, after: parsed.name },
                { property: "policy", before: null, after: parsed.policy ?? "" },
              ],
            },
          };
        }

        const body: Record<string, string> = { name: parsed.name };
        if (parsed.policy !== undefined) body.policy = parsed.policy;
        if (parsed.skin !== undefined) body.skin = parsed.skin;

        const created = await context.routerClient.create("user/group", body);
        log.info({ name: parsed.name, id: created[".id"] }, "User group added");
        return {
          content: `Added user group "${parsed.name}".`,
          structuredContent: { action: "created", routerId: context.routerId, name: parsed.name, id: created[".id"] },
        };
      }

      if (parsed.action === "update") {
        if (!existing) {
          throw new MikroMCPError({
            category: ErrorCategory.NOT_FOUND,
            code: "USER_GROUP_NOT_FOUND",
            message: `User group "${parsed.name}" not found.`,
            details: { name: parsed.name },
            recoverability: {
              retryable: false,
              suggestedAction: "Verify the group name with list_user_groups.",
              alternativeTools: ["list_user_groups"],
            },
          });
        }

        if (parsed.policy !== undefined && existing.policy === parsed.policy) {
          return {
            content: `User group "${parsed.name}" already has the requested policy. No changes made.`,
            structuredContent: { action: "no_change", name: parsed.name },
          };
        }

        if (parsed.dryRun) {
          return {
            content: `Dry run: Would update user group "${parsed.name}" policy: ${existing.policy} → ${parsed.policy}.`,
            structuredContent: {
              action: "dry_run",
              diff: [{ property: "policy", before: existing.policy ?? null, after: parsed.policy ?? null }],
            },
          };
        }

        const changes: Record<string, string> = {};
        if (parsed.policy !== undefined) changes.policy = parsed.policy;
        if (parsed.skin !== undefined) changes.skin = parsed.skin;

        await context.routerClient.update("user/group", existing[".id"], changes);
        log.info({ name: parsed.name }, "User group updated");
        return {
          content: `Updated user group "${parsed.name}".`,
          structuredContent: { action: "updated", routerId: context.routerId, name: parsed.name },
        };
      }

      // remove
      if (!existing) {
        return {
          content: `User group "${parsed.name}" not found. Nothing to remove.`,
          structuredContent: { action: "not_found", name: parsed.name },
        };
      }

      if (parsed.dryRun) {
        return {
          content: `Dry run: Would remove user group "${parsed.name}".`,
          structuredContent: {
            action: "dry_run",
            diff: [{ property: "name", before: parsed.name, after: null }],
          },
        };
      }

      await context.routerClient.remove("user/group", existing[".id"]);
      log.info({ name: parsed.name }, "User group removed");
      return {
        content: `Removed user group "${parsed.name}".`,
        structuredContent: { action: "removed", name: parsed.name, id: existing[".id"] },
      };
    } catch (err) {
      if (err instanceof MikroMCPError) throw err;
      throw enrichError(err, { routerId: context.routerId, tool: "manage_user_group" });
    }
  },
};

export const userGroupTools: ToolDefinition[] = [listUserGroupsTool, manageUserGroupTool];
```

- [ ] **Step 4: Run test to verify it passes**

```bash
npx vitest run test/unit/domain/user-group-tools.test.ts
```

Expected: All tests PASS.

- [ ] **Step 5: Commit**

```bash
git add src/domain/tools/user-group-tools.ts test/unit/domain/user-group-tools.test.ts
git commit -m "feat(tools): add list_user_groups and manage_user_group"
```

---

## Task 2: Upgrade Tools

**Files:**
- Create: `src/domain/tools/upgrade-tools.ts`
- Create: `test/unit/domain/upgrade-tools.test.ts`

RouterOS paths:
- `system/package/update` — read current update status (channel, installed-version, latest-version, status)
- `system/routerboard` — routerboard model and firmware info
- `system/package/update/check-for-updates` — execute to trigger a fresh check (POST)
- `system/package/update/install` — execute to trigger upgrade (POST, destructive)

`get_upgrade_status` is read-only. `manage_upgrade` with `action: "install"` is destructive (`destructiveHint: true`).

- [ ] **Step 1: Write the failing test**

```typescript
// test/unit/domain/upgrade-tools.test.ts
import { describe, it, expect, vi } from "vitest";
import { upgradeTools } from "../../../src/domain/tools/upgrade-tools.js";
import type { ToolContext } from "../../../src/domain/tools/tool-definition.js";
import type { RouterOSRestClient } from "../../../src/adapter/rest-client.js";
import type { RouterConfig, Identity } from "../../../src/types.js";
import type { SshClient } from "../../../src/adapter/ssh-client.js";
import type { FtpClient } from "../../../src/adapter/ftp-client.js";

const UPDATE_RECORD = {
  ".id": "*1",
  channel: "stable",
  "installed-version": "7.12.1",
  "latest-version": "7.14.2",
  status: "New version is available",
};

const ROUTERBOARD_RECORD = {
  ".id": "*1",
  routerboard: "yes",
  model: "RB750Gr3",
  "firmware-type": "ipq4000L",
  "factory-firmware": "3.49",
  "current-firmware": "7.12.1",
  "upgrade-firmware": "7.14.2",
};

function makeContext(
  updateRecords: Record<string, unknown>[] = [UPDATE_RECORD],
  routerboardRecords: Record<string, unknown>[] = [ROUTERBOARD_RECORD],
): ToolContext {
  const getMock = vi.fn().mockImplementation((path: string) => {
    if (path === "system/package/update") return Promise.resolve(updateRecords);
    if (path === "system/routerboard") return Promise.resolve(routerboardRecords);
    return Promise.resolve([]);
  });
  return {
    routerId: "test-router",
    correlationId: "test-corr",
    routerConfig: {} as RouterConfig,
    sshClient: {} as SshClient,
    ftpClient: {} as FtpClient,
    identity: {
      id: "superadmin-builtin",
      name: "admin",
      role: "superadmin",
      allowedRouters: ["*"],
      allowedToolPatterns: ["*"],
    } as unknown as Identity,
    routerClient: {
      get: getMock,
      execute: vi.fn().mockResolvedValue({ status: "triggered" }),
    } as unknown as RouterOSRestClient,
  };
}

const [getStatusTool, manageTool] = upgradeTools;

describe("upgradeTools", () => {
  describe("metadata", () => {
    it("exports 2 tools", () => expect(upgradeTools).toHaveLength(2));
    it("has correct names", () => {
      expect(getStatusTool.name).toBe("get_upgrade_status");
      expect(manageTool.name).toBe("manage_upgrade");
    });
    it("get_upgrade_status is readOnly", () => expect(getStatusTool.annotations.readOnlyHint).toBe(true));
    it("manage_upgrade is not readOnly", () => expect(manageTool.annotations.readOnlyHint).toBe(false));
    it("manage_upgrade is destructive", () => expect(manageTool.annotations.destructiveHint).toBe(true));
  });

  describe("input schema — get_upgrade_status", () => {
    it("parses valid input", () => {
      expect(getStatusTool.inputSchema.safeParse({ routerId: "r1" }).success).toBe(true);
    });
    it("rejects extra fields", () => {
      expect(getStatusTool.inputSchema.safeParse({ routerId: "r1", extra: true }).success).toBe(false);
    });
  });

  describe("input schema — manage_upgrade", () => {
    it("parses valid check input", () => {
      expect(
        manageTool.inputSchema.safeParse({ routerId: "r1", action: "check" }).success,
      ).toBe(true);
    });
    it("dryRun defaults to false", () => {
      expect(manageTool.inputSchema.parse({ routerId: "r1", action: "check" }).dryRun).toBe(false);
    });
    it("rejects extra fields", () => {
      expect(
        manageTool.inputSchema.safeParse({ routerId: "r1", action: "check", extra: true }).success,
      ).toBe(false);
    });
    it("rejects invalid action", () => {
      expect(
        manageTool.inputSchema.safeParse({ routerId: "r1", action: "download" }).success,
      ).toBe(false);
    });
  });

  describe("handler — get_upgrade_status", () => {
    it("returns update status and routerboard info", async () => {
      const ctx = makeContext();
      const result = await getStatusTool.handler({ routerId: "test-router" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.routerId).toBe("test-router");
      expect(sc.installedVersion).toBe("7.12.1");
      expect(sc.latestVersion).toBe("7.14.2");
      expect(sc.channel).toBe("stable");
    });

    it("handles missing routerboard gracefully", async () => {
      const ctx = makeContext([UPDATE_RECORD], []);
      const result = await getStatusTool.handler({ routerId: "test-router" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.routerboard).toBeNull();
    });

    it("throws enriched error on network failure", async () => {
      const ctx = makeContext();
      (ctx.routerClient.get as ReturnType<typeof vi.fn>).mockRejectedValue(new Error("net"));
      await expect(getStatusTool.handler({ routerId: "test-router" }, ctx)).rejects.toThrow();
    });
  });

  describe("handler — manage_upgrade check", () => {
    it("calls check-for-updates endpoint", async () => {
      const ctx = makeContext();
      const result = await manageTool.handler({ routerId: "test-router", action: "check" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("check_triggered");
      expect(ctx.routerClient.execute).toHaveBeenCalledWith("system/package/update/check-for-updates");
    });

    it("dry_run returns preview without calling execute", async () => {
      const ctx = makeContext();
      const result = await manageTool.handler(
        { routerId: "test-router", action: "check", dryRun: true },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("dry_run");
      expect(ctx.routerClient.execute).not.toHaveBeenCalled();
    });
  });

  describe("handler — manage_upgrade install", () => {
    it("calls install endpoint", async () => {
      const ctx = makeContext();
      const result = await manageTool.handler({ routerId: "test-router", action: "install" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("install_triggered");
      expect(ctx.routerClient.execute).toHaveBeenCalledWith("system/package/update/install");
    });

    it("dry_run returns preview without calling execute", async () => {
      const ctx = makeContext();
      const result = await manageTool.handler(
        { routerId: "test-router", action: "install", dryRun: true },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("dry_run");
      expect(ctx.routerClient.execute).not.toHaveBeenCalled();
    });
  });
});
```

- [ ] **Step 2: Run test to verify it fails**

```bash
npx vitest run test/unit/domain/upgrade-tools.test.ts
```

Expected: FAIL — "Cannot find module '../../../src/domain/tools/upgrade-tools.js'"

- [ ] **Step 3: Implement `src/domain/tools/upgrade-tools.ts`**

```typescript
import { z } from "zod";
import type { ToolDefinition, ToolContext, ToolResult } from "./tool-definition.js";
import type { RouterOSRecord } from "../../types.js";
import { enrichError } from "../errors/error-enricher.js";
import { MikroMCPError } from "../errors/error-types.js";
import { createLogger } from "../../observability/logger.js";

const log = createLogger("upgrade-tools");

const getUpgradeStatusInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
  })
  .strict();

const getUpgradeStatusTool: ToolDefinition = {
  name: "get_upgrade_status",
  title: "Get Upgrade Status",
  description:
    "Check available RouterOS/firmware upgrades and current channel. Returns installed version, latest available version, upgrade status, and routerboard firmware info.",
  inputSchema: getUpgradeStatusInputSchema,
  annotations: {
    readOnlyHint: true,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = getUpgradeStatusInputSchema.parse(params);
    log.info({ routerId: context.routerId }, "Getting upgrade status");
    try {
      const [updateResults, routerboardResults] = await Promise.all([
        context.routerClient.get<RouterOSRecord>("system/package/update"),
        context.routerClient.get<RouterOSRecord>("system/routerboard"),
      ]);

      const update = (updateResults[0] ?? {}) as Record<string, string>;
      const routerboard =
        routerboardResults.length > 0
          ? (routerboardResults[0] as Record<string, string>)
          : null;

      return {
        content: `Upgrade status on ${parsed.routerId}: installed=${update["installed-version"] ?? "?"} latest=${update["latest-version"] ?? "?"} (${update.status ?? "unknown"})`,
        structuredContent: {
          routerId: context.routerId,
          channel: update.channel ?? null,
          installedVersion: update["installed-version"] ?? null,
          latestVersion: update["latest-version"] ?? null,
          status: update.status ?? null,
          routerboard: routerboard
            ? {
                model: routerboard.model ?? null,
                currentFirmware: routerboard["current-firmware"] ?? null,
                upgradeFirmware: routerboard["upgrade-firmware"] ?? null,
                factoryFirmware: routerboard["factory-firmware"] ?? null,
              }
            : null,
        },
      };
    } catch (err) {
      throw enrichError(err, { routerId: context.routerId, tool: "get_upgrade_status" });
    }
  },
};

const manageUpgradeInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    action: z
      .enum(["check", "install"])
      .describe(
        "check: trigger a fresh package update check (non-destructive). install: download and install the latest version — the router will reboot to apply (destructive, requires confirmation).",
      ),
    dryRun: z.boolean().default(false).describe("Preview the action without executing"),
  })
  .strict();

const manageUpgradeTool: ToolDefinition = {
  name: "manage_upgrade",
  title: "Manage Upgrade",
  description:
    "Trigger a RouterOS package update check or start an upgrade installation. The install action causes a router reboot — it is destructive and requires a confirmation token for operator/readonly roles.",
  inputSchema: manageUpgradeInputSchema,
  annotations: {
    readOnlyHint: false,
    destructiveHint: true,
    idempotentHint: false,
    openWorldHint: false,
  },
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = manageUpgradeInputSchema.parse(params);
    log.info({ routerId: context.routerId, action: parsed.action }, "Managing upgrade");

    try {
      if (parsed.dryRun) {
        const description =
          parsed.action === "check"
            ? "Would trigger a package update check on the router."
            : "Would trigger RouterOS installation — the router will reboot to apply the upgrade.";
        return {
          content: `Dry run: ${description}`,
          structuredContent: { action: "dry_run", requestedAction: parsed.action, routerId: context.routerId },
        };
      }

      if (parsed.action === "check") {
        await context.routerClient.execute("system/package/update/check-for-updates");
        log.info({ routerId: context.routerId }, "Update check triggered");
        return {
          content: `Update check triggered on ${context.routerId}. Use get_upgrade_status to see results.`,
          structuredContent: { action: "check_triggered", routerId: context.routerId },
        };
      }

      // install
      await context.routerClient.execute("system/package/update/install");
      log.info({ routerId: context.routerId }, "Upgrade install triggered");
      return {
        content: `Upgrade installation triggered on ${context.routerId}. The router will reboot to apply the upgrade.`,
        structuredContent: { action: "install_triggered", routerId: context.routerId },
      };
    } catch (err) {
      if (err instanceof MikroMCPError) throw err;
      throw enrichError(err, { routerId: context.routerId, tool: "manage_upgrade" });
    }
  },
};

export const upgradeTools: ToolDefinition[] = [getUpgradeStatusTool, manageUpgradeTool];
```

- [ ] **Step 4: Run test to verify it passes**

```bash
npx vitest run test/unit/domain/upgrade-tools.test.ts
```

Expected: All tests PASS.

- [ ] **Step 5: Commit**

```bash
git add src/domain/tools/upgrade-tools.ts test/unit/domain/upgrade-tools.test.ts
git commit -m "feat(tools): add get_upgrade_status and manage_upgrade"
```

---

## Task 3: Backup Tools

**Files:**
- Create: `src/domain/tools/backup-tools.ts`
- Create: `test/unit/domain/backup-tools.test.ts`

RouterOS paths:
- `system/backup/save` — execute to create a binary backup file (params: `name`, `password` optional)
- `system/export` — execute to get a RouterOS script text export

Both are write actions (they create files or output), not destructive. `export_config` returns the raw script text in `structuredContent`.

- [ ] **Step 1: Write the failing test**

```typescript
// test/unit/domain/backup-tools.test.ts
import { describe, it, expect, vi } from "vitest";
import { backupTools } from "../../../src/domain/tools/backup-tools.js";
import type { ToolContext } from "../../../src/domain/tools/tool-definition.js";
import type { RouterOSRestClient } from "../../../src/adapter/rest-client.js";
import type { RouterConfig, Identity } from "../../../src/types.js";
import type { SshClient } from "../../../src/adapter/ssh-client.js";
import type { FtpClient } from "../../../src/adapter/ftp-client.js";

function makeContext(executeResult: unknown = {}): ToolContext {
  return {
    routerId: "test-router",
    correlationId: "test-corr",
    routerConfig: {} as RouterConfig,
    sshClient: {} as SshClient,
    ftpClient: {} as FtpClient,
    identity: {
      id: "superadmin-builtin",
      name: "admin",
      role: "superadmin",
      allowedRouters: ["*"],
      allowedToolPatterns: ["*"],
    } as unknown as Identity,
    routerClient: {
      execute: vi.fn().mockResolvedValue(executeResult),
    } as unknown as RouterOSRestClient,
  };
}

const [createBackupTool, exportConfigTool] = backupTools;

describe("backupTools", () => {
  describe("metadata", () => {
    it("exports 2 tools", () => expect(backupTools).toHaveLength(2));
    it("has correct names", () => {
      expect(createBackupTool.name).toBe("create_backup");
      expect(exportConfigTool.name).toBe("export_config");
    });
    it("create_backup is not readOnly", () => expect(createBackupTool.annotations.readOnlyHint).toBe(false));
    it("create_backup is not destructive", () => expect(createBackupTool.annotations.destructiveHint).toBe(false));
    it("export_config is readOnly", () => expect(exportConfigTool.annotations.readOnlyHint).toBe(true));
  });

  describe("input schema — create_backup", () => {
    it("parses valid input with defaults", () => {
      const result = createBackupTool.inputSchema.safeParse({ routerId: "r1" });
      expect(result.success).toBe(true);
    });
    it("rejects extra fields", () => {
      expect(createBackupTool.inputSchema.safeParse({ routerId: "r1", extra: true }).success).toBe(false);
    });
    it("dryRun defaults to false", () => {
      expect(createBackupTool.inputSchema.parse({ routerId: "r1" }).dryRun).toBe(false);
    });
  });

  describe("input schema — export_config", () => {
    it("parses valid minimal input", () => {
      expect(exportConfigTool.inputSchema.safeParse({ routerId: "r1" }).success).toBe(true);
    });
    it("rejects extra fields", () => {
      expect(exportConfigTool.inputSchema.safeParse({ routerId: "r1", extra: true }).success).toBe(false);
    });
    it("compact defaults to false", () => {
      expect(exportConfigTool.inputSchema.parse({ routerId: "r1" }).compact).toBe(false);
    });
  });

  describe("handler — create_backup", () => {
    it("calls backup save with default name", async () => {
      const ctx = makeContext({ name: "backup" });
      const result = await createBackupTool.handler({ routerId: "test-router" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("created");
      expect(ctx.routerClient.execute).toHaveBeenCalledWith(
        "system/backup/save",
        expect.objectContaining({ name: "backup" }),
      );
    });

    it("uses provided name", async () => {
      const ctx = makeContext({ name: "my-backup" });
      await createBackupTool.handler({ routerId: "test-router", name: "my-backup" }, ctx);
      expect(ctx.routerClient.execute).toHaveBeenCalledWith(
        "system/backup/save",
        expect.objectContaining({ name: "my-backup" }),
      );
    });

    it("includes password param when provided", async () => {
      const ctx = makeContext({});
      await createBackupTool.handler(
        { routerId: "test-router", name: "safe-backup", password: "secret" },
        ctx,
      );
      expect(ctx.routerClient.execute).toHaveBeenCalledWith(
        "system/backup/save",
        expect.objectContaining({ password: "secret" }),
      );
    });

    it("never includes password in structuredContent", async () => {
      const ctx = makeContext({});
      const result = await createBackupTool.handler(
        { routerId: "test-router", name: "safe-backup", password: "topsecret" },
        ctx,
      );
      expect(JSON.stringify(result.structuredContent)).not.toContain("topsecret");
    });

    it("dry_run returns preview without calling execute", async () => {
      const ctx = makeContext({});
      const result = await createBackupTool.handler(
        { routerId: "test-router", dryRun: true },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("dry_run");
      expect(ctx.routerClient.execute).not.toHaveBeenCalled();
    });
  });

  describe("handler — export_config", () => {
    it("calls system/export and returns script text", async () => {
      const ctx = makeContext("# RouterOS export\n/ip address add address=1.2.3.4/24");
      const result = await exportConfigTool.handler({ routerId: "test-router" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect(typeof sc.script).toBe("string");
      expect((sc.script as string).length).toBeGreaterThan(0);
    });

    it("passes compact=true when requested", async () => {
      const ctx = makeContext("# compact export");
      await exportConfigTool.handler({ routerId: "test-router", compact: true }, ctx);
      expect(ctx.routerClient.execute).toHaveBeenCalledWith(
        "system/export",
        expect.objectContaining({ compact: "yes" }),
      );
    });

    it("does not pass compact param when false", async () => {
      const ctx = makeContext("# full export");
      await exportConfigTool.handler({ routerId: "test-router", compact: false }, ctx);
      const callArgs = (ctx.routerClient.execute as ReturnType<typeof vi.fn>).mock.calls[0];
      expect(callArgs[1]).not.toHaveProperty("compact");
    });
  });
});
```

- [ ] **Step 2: Run test to verify it fails**

```bash
npx vitest run test/unit/domain/backup-tools.test.ts
```

Expected: FAIL — "Cannot find module '../../../src/domain/tools/backup-tools.js'"

- [ ] **Step 3: Implement `src/domain/tools/backup-tools.ts`**

```typescript
import { z } from "zod";
import type { ToolDefinition, ToolContext, ToolResult } from "./tool-definition.js";
import { enrichError } from "../errors/error-enricher.js";
import { MikroMCPError } from "../errors/error-types.js";
import { createLogger } from "../../observability/logger.js";

const log = createLogger("backup-tools");

const createBackupInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    name: z.string().default("backup").describe("Backup file name (without .backup extension)"),
    password: z
      .string()
      .optional()
      .describe("Optional encryption password for the backup file"),
    dryRun: z.boolean().default(false).describe("Preview without creating the backup"),
  })
  .strict();

const createBackupTool: ToolDefinition = {
  name: "create_backup",
  title: "Create Backup",
  description:
    "Create a binary RouterOS configuration backup file on the router. The file is saved to the router's filesystem and can be downloaded with get_file_content. Supports optional encryption.",
  inputSchema: createBackupInputSchema,
  annotations: {
    readOnlyHint: false,
    destructiveHint: false,
    idempotentHint: false,
    openWorldHint: false,
  },
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = createBackupInputSchema.parse(params);
    log.info({ routerId: context.routerId, name: parsed.name }, "Creating backup");

    try {
      if (parsed.dryRun) {
        return {
          content: `Dry run: Would create backup file "${parsed.name}.backup" on ${context.routerId}.`,
          structuredContent: { action: "dry_run", name: parsed.name, routerId: context.routerId },
        };
      }

      const body: Record<string, string> = { name: parsed.name };
      if (parsed.password !== undefined) body.password = parsed.password;

      await context.routerClient.execute("system/backup/save", body);
      const filePath = `${parsed.name}.backup`;
      log.info({ routerId: context.routerId, filePath }, "Backup created");

      return {
        content: `Backup created on ${context.routerId}: ${filePath}`,
        structuredContent: { action: "created", routerId: context.routerId, filePath },
      };
    } catch (err) {
      if (err instanceof MikroMCPError) throw err;
      throw enrichError(err, { routerId: context.routerId, tool: "create_backup" });
    }
  },
};

const exportConfigInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    compact: z
      .boolean()
      .default(false)
      .describe("Export only non-default settings (equivalent to /export compact)"),
    file: z
      .string()
      .optional()
      .describe(
        "Save export to a file on the router instead of returning inline. Provide filename without extension.",
      ),
  })
  .strict();

const exportConfigTool: ToolDefinition = {
  name: "export_config",
  title: "Export Config",
  description:
    "Export the running RouterOS configuration as a RouterOS script (equivalent to /export). Returns the full config text. Use compact=true to include only non-default values. Optionally save to a router file instead of returning inline.",
  inputSchema: exportConfigInputSchema,
  annotations: {
    readOnlyHint: true,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = exportConfigInputSchema.parse(params);
    log.info({ routerId: context.routerId, compact: parsed.compact }, "Exporting config");

    try {
      const body: Record<string, string> = {};
      if (parsed.compact) body.compact = "yes";
      if (parsed.file !== undefined) body.file = parsed.file;

      const script = await context.routerClient.execute<string>("system/export", body);

      if (parsed.file !== undefined) {
        return {
          content: `Config exported to file "${parsed.file}.rsc" on ${context.routerId}.`,
          structuredContent: { routerId: context.routerId, filePath: `${parsed.file}.rsc` },
        };
      }

      const scriptText = typeof script === "string" ? script : JSON.stringify(script);
      return {
        content: `Config export from ${context.routerId} (${scriptText.length} chars)`,
        structuredContent: { routerId: context.routerId, script: scriptText, compact: parsed.compact },
      };
    } catch (err) {
      throw enrichError(err, { routerId: context.routerId, tool: "export_config" });
    }
  },
};

export const backupTools: ToolDefinition[] = [createBackupTool, exportConfigTool];
```

- [ ] **Step 4: Run test to verify it passes**

```bash
npx vitest run test/unit/domain/backup-tools.test.ts
```

Expected: All tests PASS.

- [ ] **Step 5: Commit**

```bash
git add src/domain/tools/backup-tools.ts test/unit/domain/backup-tools.test.ts
git commit -m "feat(tools): add create_backup and export_config"
```

---

## Task 4: Log Rule Tools

**Files:**
- Create: `src/domain/tools/log-tools.ts`
- Create: `test/unit/domain/log-tools.test.ts`

RouterOS paths:
- `system/logging` — log rules (fields: `topics`, `action`, `prefix`, `disabled`)
- `system/logging/action` — log action targets (fields: `name`, `type`, `remote`, `remote-port`, `bsd-syslog`, `syslog-facility`, `syslog-severity`, `syslog-time-format`, `disk-file-name`, `disk-lines-per-file`)

**Idempotency keys:**
- `list_log_rules` / `manage_log_rule`: composite key = `topics` + `action`
- `list_log_actions` / `manage_log_action`: `name`

**`manage_log_action` actions:** `add`, `update`, `remove`. The `type` field is required for `add`.

- [ ] **Step 1: Write the failing test**

```typescript
// test/unit/domain/log-tools.test.ts
import { describe, it, expect, vi } from "vitest";
import { logTools } from "../../../src/domain/tools/log-tools.js";
import type { ToolContext } from "../../../src/domain/tools/tool-definition.js";
import type { RouterOSRestClient } from "../../../src/adapter/rest-client.js";
import type { RouterConfig, Identity } from "../../../src/types.js";
import type { SshClient } from "../../../src/adapter/ssh-client.js";
import type { FtpClient } from "../../../src/adapter/ftp-client.js";
import { ErrorCategory } from "../../../src/domain/errors/error-types.js";

function makeContext(
  rules: Record<string, unknown>[] = [],
  actions: Record<string, unknown>[] = [],
): ToolContext {
  const getMock = vi.fn().mockImplementation((path: string) => {
    if (path === "system/logging") return Promise.resolve(rules);
    if (path === "system/logging/action") return Promise.resolve(actions);
    return Promise.resolve([]);
  });
  return {
    routerId: "test-router",
    correlationId: "test-corr",
    routerConfig: {} as RouterConfig,
    sshClient: {} as SshClient,
    ftpClient: {} as FtpClient,
    identity: {
      id: "superadmin-builtin",
      name: "admin",
      role: "superadmin",
      allowedRouters: ["*"],
      allowedToolPatterns: ["*"],
    } as unknown as Identity,
    routerClient: {
      get: getMock,
      create: vi.fn().mockResolvedValue({ ".id": "*1" }),
      update: vi.fn().mockResolvedValue(undefined),
      remove: vi.fn().mockResolvedValue(undefined),
    } as unknown as RouterOSRestClient,
  };
}

const [listRulesTool, manageRuleTool, listActionsTool, manageActionTool] = logTools;

describe("logTools", () => {
  describe("metadata", () => {
    it("exports 4 tools", () => expect(logTools).toHaveLength(4));
    it("has correct names", () => {
      expect(listRulesTool.name).toBe("list_log_rules");
      expect(manageRuleTool.name).toBe("manage_log_rule");
      expect(listActionsTool.name).toBe("list_log_actions");
      expect(manageActionTool.name).toBe("manage_log_action");
    });
    it("list tools are readOnly", () => {
      expect(listRulesTool.annotations.readOnlyHint).toBe(true);
      expect(listActionsTool.annotations.readOnlyHint).toBe(true);
    });
    it("manage tools are not readOnly", () => {
      expect(manageRuleTool.annotations.readOnlyHint).toBe(false);
      expect(manageActionTool.annotations.readOnlyHint).toBe(false);
    });
    it("manage tools are not destructive", () => {
      expect(manageRuleTool.annotations.destructiveHint).toBe(false);
      expect(manageActionTool.annotations.destructiveHint).toBe(false);
    });
  });

  describe("input schema — list_log_rules", () => {
    it("parses minimal input", () => {
      expect(listRulesTool.inputSchema.safeParse({ routerId: "r1" }).success).toBe(true);
    });
    it("defaults limit to 100", () => {
      expect(listRulesTool.inputSchema.parse({ routerId: "r1" }).limit).toBe(100);
    });
    it("rejects extra fields", () => {
      expect(listRulesTool.inputSchema.safeParse({ routerId: "r1", extra: true }).success).toBe(false);
    });
  });

  describe("input schema — manage_log_rule", () => {
    it("parses valid add input", () => {
      expect(
        manageRuleTool.inputSchema.safeParse({
          routerId: "r1", action: "add", topics: "firewall", logAction: "memory",
        }).success,
      ).toBe(true);
    });
    it("dryRun defaults to false", () => {
      expect(
        manageRuleTool.inputSchema.parse({
          routerId: "r1", action: "add", topics: "firewall", logAction: "memory",
        }).dryRun,
      ).toBe(false);
    });
    it("rejects extra fields", () => {
      expect(
        manageRuleTool.inputSchema.safeParse({
          routerId: "r1", action: "add", topics: "firewall", logAction: "memory", extra: true,
        }).success,
      ).toBe(false);
    });
    it("rejects invalid action", () => {
      expect(
        manageRuleTool.inputSchema.safeParse({
          routerId: "r1", action: "update", topics: "firewall", logAction: "memory",
        }).success,
      ).toBe(false);
    });
  });

  describe("input schema — manage_log_action", () => {
    it("parses valid add input", () => {
      expect(
        manageActionTool.inputSchema.safeParse({
          routerId: "r1", action: "add", name: "syslog-remote", type: "remote", remote: "10.0.0.1",
        }).success,
      ).toBe(true);
    });
    it("dryRun defaults to false", () => {
      expect(
        manageActionTool.inputSchema.parse({ routerId: "r1", action: "remove", name: "old-action" }).dryRun,
      ).toBe(false);
    });
    it("rejects extra fields", () => {
      expect(
        manageActionTool.inputSchema.safeParse({
          routerId: "r1", action: "add", name: "test", type: "memory", extra: true,
        }).success,
      ).toBe(false);
    });
    it("rejects invalid type", () => {
      expect(
        manageActionTool.inputSchema.safeParse({
          routerId: "r1", action: "add", name: "test", type: "invalid",
        }).success,
      ).toBe(false);
    });
  });

  describe("handler — list_log_rules", () => {
    it("returns rules with topic filter", async () => {
      const ctx = makeContext([
        { ".id": "*1", topics: "firewall", action: "memory", disabled: "false" },
        { ".id": "*2", topics: "system", action: "disk", disabled: "false" },
      ]);
      const result = await listRulesTool.handler({ routerId: "test-router", topics: "firewall" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect((sc.rules as unknown[]).length).toBe(1);
    });

    it("returns all rules without filter", async () => {
      const ctx = makeContext([
        { ".id": "*1", topics: "firewall", action: "memory", disabled: "false" },
        { ".id": "*2", topics: "system", action: "disk", disabled: "false" },
      ]);
      const result = await listRulesTool.handler({ routerId: "test-router" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect((sc.rules as unknown[]).length).toBe(2);
    });
  });

  describe("handler — manage_log_rule add", () => {
    it("creates rule when not found", async () => {
      const ctx = makeContext([]);
      const result = await manageRuleTool.handler(
        { routerId: "test-router", action: "add", topics: "firewall", logAction: "memory" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("created");
      expect(ctx.routerClient.create).toHaveBeenCalledWith(
        "system/logging",
        expect.objectContaining({ topics: "firewall", action: "memory" }),
      );
    });

    it("returns already_exists for same topics+action", async () => {
      const ctx = makeContext([{ ".id": "*1", topics: "firewall", action: "memory" }]);
      const result = await manageRuleTool.handler(
        { routerId: "test-router", action: "add", topics: "firewall", logAction: "memory" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("already_exists");
      expect(ctx.routerClient.create).not.toHaveBeenCalled();
    });

    it("dry_run returns preview without calling create", async () => {
      const ctx = makeContext([]);
      const result = await manageRuleTool.handler(
        { routerId: "test-router", action: "add", topics: "firewall", logAction: "memory", dryRun: true },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("dry_run");
      expect(ctx.routerClient.create).not.toHaveBeenCalled();
    });
  });

  describe("handler — manage_log_rule enable/disable/remove", () => {
    it("disables an existing rule", async () => {
      const ctx = makeContext([{ ".id": "*1", topics: "firewall", action: "memory", disabled: "false" }]);
      const result = await manageRuleTool.handler(
        { routerId: "test-router", action: "disable", topics: "firewall", logAction: "memory" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("disabled");
      expect(ctx.routerClient.update).toHaveBeenCalledWith("system/logging", "*1", { disabled: "true" });
    });

    it("removes rule when found", async () => {
      const ctx = makeContext([{ ".id": "*1", topics: "firewall", action: "memory" }]);
      const result = await manageRuleTool.handler(
        { routerId: "test-router", action: "remove", topics: "firewall", logAction: "memory" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("removed");
      expect(ctx.routerClient.remove).toHaveBeenCalledWith("system/logging", "*1");
    });

    it("throws NOT_FOUND for disable on missing rule", async () => {
      const ctx = makeContext([]);
      await expect(
        manageRuleTool.handler(
          { routerId: "test-router", action: "disable", topics: "firewall", logAction: "memory" },
          ctx,
        ),
      ).rejects.toMatchObject({ category: ErrorCategory.NOT_FOUND });
    });

    it("returns not_found gracefully for remove on missing rule", async () => {
      const ctx = makeContext([]);
      const result = await manageRuleTool.handler(
        { routerId: "test-router", action: "remove", topics: "firewall", logAction: "memory" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("not_found");
    });
  });

  describe("handler — list_log_actions", () => {
    it("returns all log actions", async () => {
      const ctx = makeContext(
        [],
        [
          { ".id": "*1", name: "memory", type: "memory" },
          { ".id": "*2", name: "disk", type: "disk" },
        ],
      );
      const result = await listActionsTool.handler({ routerId: "test-router" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect((sc.actions as unknown[]).length).toBe(2);
    });
  });

  describe("handler — manage_log_action add", () => {
    it("throws VALIDATION when type missing on add", async () => {
      const ctx = makeContext([], []);
      await expect(
        manageActionTool.handler(
          { routerId: "test-router", action: "add", name: "mysyslog" },
          ctx,
        ),
      ).rejects.toMatchObject({ category: ErrorCategory.VALIDATION });
    });

    it("creates action when not found", async () => {
      const ctx = makeContext([], []);
      const result = await manageActionTool.handler(
        { routerId: "test-router", action: "add", name: "mysyslog", type: "remote", remote: "10.0.0.1" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("created");
      expect(ctx.routerClient.create).toHaveBeenCalledWith(
        "system/logging/action",
        expect.objectContaining({ name: "mysyslog", type: "remote" }),
      );
    });

    it("returns already_exists for same name", async () => {
      const ctx = makeContext([], [{ ".id": "*1", name: "mysyslog", type: "remote" }]);
      const result = await manageActionTool.handler(
        { routerId: "test-router", action: "add", name: "mysyslog", type: "remote" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("already_exists");
    });

    it("dry_run returns preview without calling create", async () => {
      const ctx = makeContext([], []);
      const result = await manageActionTool.handler(
        { routerId: "test-router", action: "add", name: "syslog", type: "memory", dryRun: true },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("dry_run");
      expect(ctx.routerClient.create).not.toHaveBeenCalled();
    });
  });

  describe("handler — manage_log_action remove", () => {
    it("removes action when found", async () => {
      const ctx = makeContext([], [{ ".id": "*1", name: "old-syslog", type: "remote" }]);
      const result = await manageActionTool.handler(
        { routerId: "test-router", action: "remove", name: "old-syslog" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("removed");
      expect(ctx.routerClient.remove).toHaveBeenCalledWith("system/logging/action", "*1");
    });

    it("returns not_found gracefully when already gone", async () => {
      const ctx = makeContext([], []);
      const result = await manageActionTool.handler(
        { routerId: "test-router", action: "remove", name: "old-syslog" },
        ctx,
      );
      expect((result.structuredContent as Record<string, unknown>).action).toBe("not_found");
    });
  });
});
```

- [ ] **Step 2: Run test to verify it fails**

```bash
npx vitest run test/unit/domain/log-tools.test.ts
```

Expected: FAIL — "Cannot find module '../../../src/domain/tools/log-tools.js'"

- [ ] **Step 3: Implement `src/domain/tools/log-tools.ts`**

```typescript
import { z } from "zod";
import type { ToolDefinition, ToolContext, ToolResult } from "./tool-definition.js";
import type { RouterOSRecord } from "../../types.js";
import { enrichError } from "../errors/error-enricher.js";
import { MikroMCPError, ErrorCategory } from "../errors/error-types.js";
import { createLogger } from "../../observability/logger.js";

const log = createLogger("log-tools");

// ---------------------------------------------------------------------------
// list_log_rules
// ---------------------------------------------------------------------------

const listLogRulesInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    topics: z.string().optional().describe("Filter by topics string (substring match)"),
    logAction: z.string().optional().describe("Filter by action name (exact match)"),
    limit: z
      .number()
      .int()
      .min(1)
      .max(500)
      .default(100)
      .describe("Maximum number of rules to return"),
  })
  .strict();

const listLogRulesTool: ToolDefinition = {
  name: "list_log_rules",
  title: "List Log Rules",
  description: "List RouterOS system logging rules with topics, action, and status.",
  inputSchema: listLogRulesInputSchema,
  annotations: {
    readOnlyHint: true,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = listLogRulesInputSchema.parse(params);
    log.info({ routerId: context.routerId }, "Listing log rules");
    try {
      const all = await context.routerClient.get<RouterOSRecord>("system/logging", {
        limit: undefined,
        offset: undefined,
      });
      const filtered = (all as Record<string, string>[])
        .filter((r) => (parsed.topics ? (r.topics ?? "").includes(parsed.topics) : true))
        .filter((r) => (parsed.logAction ? r.action === parsed.logAction : true));
      const rules = filtered.slice(0, parsed.limit);
      return {
        content: `Log rules on ${context.routerId}: ${rules.length} returned (${all.length} total)`,
        structuredContent: { routerId: context.routerId, rules, total: all.length, returned: rules.length },
      };
    } catch (err) {
      throw enrichError(err, { routerId: context.routerId, tool: "list_log_rules" });
    }
  },
};

// ---------------------------------------------------------------------------
// manage_log_rule
// ---------------------------------------------------------------------------

const manageLogRuleInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    action: z
      .enum(["add", "remove", "enable", "disable"])
      .describe("Action to perform"),
    topics: z
      .string()
      .describe(
        "Log topics string (e.g. 'firewall', 'system,!debug'). Composite idempotency key with logAction.",
      ),
    logAction: z
      .string()
      .describe("Log action name to route matching topics to (e.g. 'memory', 'disk')"),
    prefix: z.string().optional().describe("Optional prefix for log entries"),
    dryRun: z.boolean().default(false).describe("Preview changes without applying"),
  })
  .strict();

const manageLogRuleTool: ToolDefinition = {
  name: "manage_log_rule",
  title: "Manage Log Rule",
  description:
    "Add, remove, enable, or disable a RouterOS system logging rule. Idempotent by topics+action: add returns already_exists if an identical rule already exists.",
  inputSchema: manageLogRuleInputSchema,
  annotations: {
    readOnlyHint: false,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  snapshotPaths: ["system/logging"],
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = manageLogRuleInputSchema.parse(params);
    log.info(
      { routerId: context.routerId, action: parsed.action, topics: parsed.topics },
      "Managing log rule",
    );
    try {
      const all = await context.routerClient.get<RouterOSRecord>("system/logging", {
        limit: undefined,
        offset: undefined,
      });
      const existing = (all as Record<string, string>[]).find(
        (r) => r.topics === parsed.topics && r.action === parsed.logAction,
      );

      if (parsed.action === "add") {
        if (existing) {
          return {
            content: `Log rule for topics="${parsed.topics}" action="${parsed.logAction}" already exists. No changes made.`,
            structuredContent: { action: "already_exists", topics: parsed.topics, logAction: parsed.logAction },
          };
        }

        if (parsed.dryRun) {
          return {
            content: `Dry run: Would add log rule topics="${parsed.topics}" → action="${parsed.logAction}".`,
            structuredContent: {
              action: "dry_run",
              diff: [
                { property: "topics", before: null, after: parsed.topics },
                { property: "action", before: null, after: parsed.logAction },
              ],
            },
          };
        }

        const body: Record<string, string> = { topics: parsed.topics, action: parsed.logAction };
        if (parsed.prefix !== undefined) body.prefix = parsed.prefix;

        const created = await context.routerClient.create("system/logging", body);
        log.info({ topics: parsed.topics, id: created[".id"] }, "Log rule added");
        return {
          content: `Added log rule: topics="${parsed.topics}" → action="${parsed.logAction}".`,
          structuredContent: { action: "created", routerId: context.routerId, id: created[".id"] },
        };
      }

      if (parsed.action === "remove") {
        if (!existing) {
          return {
            content: `Log rule topics="${parsed.topics}" action="${parsed.logAction}" not found. Nothing to remove.`,
            structuredContent: { action: "not_found", topics: parsed.topics, logAction: parsed.logAction },
          };
        }
        if (parsed.dryRun) {
          return {
            content: `Dry run: Would remove log rule topics="${parsed.topics}" action="${parsed.logAction}".`,
            structuredContent: {
              action: "dry_run",
              diff: [{ property: "topics", before: parsed.topics, after: null }],
            },
          };
        }
        await context.routerClient.remove("system/logging", existing[".id"]);
        log.info({ topics: parsed.topics }, "Log rule removed");
        return {
          content: `Removed log rule: topics="${parsed.topics}" action="${parsed.logAction}".`,
          structuredContent: { action: "removed", id: existing[".id"] },
        };
      }

      // enable / disable
      if (!existing) {
        throw new MikroMCPError({
          category: ErrorCategory.NOT_FOUND,
          code: "LOG_RULE_NOT_FOUND",
          message: `Log rule topics="${parsed.topics}" action="${parsed.logAction}" not found.`,
          details: { topics: parsed.topics, logAction: parsed.logAction },
          recoverability: {
            retryable: false,
            suggestedAction: "Verify the rule with list_log_rules.",
            alternativeTools: ["list_log_rules"],
          },
        });
      }

      if (parsed.dryRun) {
        return {
          content: `Dry run: Would ${parsed.action} log rule topics="${parsed.topics}".`,
          structuredContent: {
            action: "dry_run",
            diff: [
              {
                property: "disabled",
                before: existing.disabled,
                after: parsed.action === "disable" ? "true" : "false",
              },
            ],
          },
        };
      }

      await context.routerClient.update("system/logging", existing[".id"], {
        disabled: parsed.action === "disable" ? "true" : "false",
      });
      const resultAction = parsed.action === "disable" ? "disabled" : "enabled";
      log.info({ topics: parsed.topics, action: resultAction }, "Log rule updated");
      return {
        content: `Log rule topics="${parsed.topics}" ${resultAction}.`,
        structuredContent: { action: resultAction, id: existing[".id"] },
      };
    } catch (err) {
      if (err instanceof MikroMCPError) throw err;
      throw enrichError(err, { routerId: context.routerId, tool: "manage_log_rule" });
    }
  },
};

// ---------------------------------------------------------------------------
// list_log_actions
// ---------------------------------------------------------------------------

const listLogActionsInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    type: z
      .enum(["memory", "disk", "remote", "echo", "email"])
      .optional()
      .describe("Filter by action type"),
    limit: z
      .number()
      .int()
      .min(1)
      .max(500)
      .default(100)
      .describe("Maximum number of actions to return"),
  })
  .strict();

const listLogActionsTool: ToolDefinition = {
  name: "list_log_actions",
  title: "List Log Actions",
  description:
    "List RouterOS log action targets (memory, disk, remote syslog, etc.) with their configuration.",
  inputSchema: listLogActionsInputSchema,
  annotations: {
    readOnlyHint: true,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = listLogActionsInputSchema.parse(params);
    log.info({ routerId: context.routerId }, "Listing log actions");
    try {
      const all = await context.routerClient.get<RouterOSRecord>("system/logging/action", {
        limit: undefined,
        offset: undefined,
      });
      const filtered = parsed.type
        ? (all as Record<string, string>[]).filter((a) => a.type === parsed.type)
        : (all as Record<string, string>[]);
      const actions = filtered.slice(0, parsed.limit);
      return {
        content: `Log actions on ${context.routerId}: ${actions.length} returned (${all.length} total)`,
        structuredContent: { routerId: context.routerId, actions, total: all.length, returned: actions.length },
      };
    } catch (err) {
      throw enrichError(err, { routerId: context.routerId, tool: "list_log_actions" });
    }
  },
};

// ---------------------------------------------------------------------------
// manage_log_action
// ---------------------------------------------------------------------------

const manageLogActionInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    action: z.enum(["add", "remove"]).describe("Action to perform"),
    name: z.string().describe("Log action name — idempotency key"),
    type: z
      .enum(["memory", "disk", "remote", "echo", "email"])
      .optional()
      .describe("Action type (required for add)"),
    remote: z.string().optional().describe("Remote syslog server IP (required when type=remote)"),
    remotePort: z
      .number()
      .int()
      .min(1)
      .max(65535)
      .optional()
      .describe("Remote syslog UDP port (default 514)"),
    diskFileName: z
      .string()
      .optional()
      .describe("Disk log file name (required when type=disk)"),
    dryRun: z.boolean().default(false).describe("Preview changes without applying"),
  })
  .strict();

const manageLogActionTool: ToolDefinition = {
  name: "manage_log_action",
  title: "Manage Log Action",
  description:
    "Add or remove a RouterOS log action target. Idempotent by name: add returns already_exists if an action with the same name already exists.",
  inputSchema: manageLogActionInputSchema,
  annotations: {
    readOnlyHint: false,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  snapshotPaths: ["system/logging/action"],
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = manageLogActionInputSchema.parse(params);
    log.info(
      { routerId: context.routerId, action: parsed.action, name: parsed.name },
      "Managing log action",
    );
    try {
      const all = await context.routerClient.get<RouterOSRecord>("system/logging/action", {
        limit: undefined,
        offset: undefined,
      });
      const existing = (all as Record<string, string>[]).find((a) => a.name === parsed.name);

      if (parsed.action === "add") {
        if (!parsed.type) {
          throw new MikroMCPError({
            category: ErrorCategory.VALIDATION,
            code: "LOG_ACTION_TYPE_REQUIRED",
            message: "type is required when adding a log action.",
            details: { name: parsed.name },
            recoverability: {
              retryable: false,
              suggestedAction: "Provide type: memory, disk, remote, echo, or email.",
            },
          });
        }

        if (existing) {
          return {
            content: `Log action "${parsed.name}" already exists. No changes made.`,
            structuredContent: { action: "already_exists", name: parsed.name, type: existing.type },
          };
        }

        if (parsed.dryRun) {
          return {
            content: `Dry run: Would add log action "${parsed.name}" (type=${parsed.type}).`,
            structuredContent: {
              action: "dry_run",
              diff: [
                { property: "name", before: null, after: parsed.name },
                { property: "type", before: null, after: parsed.type },
              ],
            },
          };
        }

        const body: Record<string, string> = { name: parsed.name, type: parsed.type };
        if (parsed.remote !== undefined) body.remote = parsed.remote;
        if (parsed.remotePort !== undefined) body["remote-port"] = String(parsed.remotePort);
        if (parsed.diskFileName !== undefined) body["disk-file-name"] = parsed.diskFileName;

        const created = await context.routerClient.create("system/logging/action", body);
        log.info({ name: parsed.name, id: created[".id"] }, "Log action added");
        return {
          content: `Added log action "${parsed.name}" (type=${parsed.type}).`,
          structuredContent: { action: "created", routerId: context.routerId, name: parsed.name, id: created[".id"] },
        };
      }

      // remove
      if (!existing) {
        return {
          content: `Log action "${parsed.name}" not found. Nothing to remove.`,
          structuredContent: { action: "not_found", name: parsed.name },
        };
      }

      if (parsed.dryRun) {
        return {
          content: `Dry run: Would remove log action "${parsed.name}".`,
          structuredContent: {
            action: "dry_run",
            diff: [{ property: "name", before: parsed.name, after: null }],
          },
        };
      }

      await context.routerClient.remove("system/logging/action", existing[".id"]);
      log.info({ name: parsed.name }, "Log action removed");
      return {
        content: `Removed log action "${parsed.name}".`,
        structuredContent: { action: "removed", name: parsed.name, id: existing[".id"] },
      };
    } catch (err) {
      if (err instanceof MikroMCPError) throw err;
      throw enrichError(err, { routerId: context.routerId, tool: "manage_log_action" });
    }
  },
};

export const logTools: ToolDefinition[] = [
  listLogRulesTool,
  manageLogRuleTool,
  listLogActionsTool,
  manageLogActionTool,
];
```

- [ ] **Step 4: Run test to verify it passes**

```bash
npx vitest run test/unit/domain/log-tools.test.ts
```

Expected: All tests PASS.

- [ ] **Step 5: Commit**

```bash
git add src/domain/tools/log-tools.ts test/unit/domain/log-tools.test.ts
git commit -m "feat(tools): add list_log_rules, manage_log_rule, list_log_actions, manage_log_action"
```

---

## Task 5: `manage_ntp_client`

**Files:**
- Modify: `src/domain/tools/network-services-tools.ts`
- Modify: `test/unit/domain/network-services-tools.test.ts`

`manage_ntp_client` updates the singleton NTP client config at `system/ntp/client`. Fields: `enabled` (true/false), `mode` (unicast/broadcast/multicast/manycast), `servers` (comma-separated IPs or hostnames), `vlan-interface` (optional). Idempotency: return `already_set` if all provided values already match.

- [ ] **Step 1: Read existing test file to locate where to add tests**

Open `test/unit/domain/network-services-tools.test.ts` and identify the last `describe` block — the new `manage_ntp_client` describe goes after it.

- [ ] **Step 2: Add `manage_ntp_client` tests to existing test file**

Append to `test/unit/domain/network-services-tools.test.ts`:

```typescript
// Add this import at the top of the file (after existing imports):
// import { ErrorCategory } from "../../../src/domain/errors/error-types.js";
// (only if not already imported)

// Add this section after the existing describe blocks:

describe("manage_ntp_client", () => {
  // networkServicesTools array: [snmp, ntp_settings, netwatch_list, netwatch_manage, neighbors, arp, ntp_manage]
  // index 6 is manage_ntp_client
  const manageTool = networkServicesTools[6];

  it("has correct name", () => expect(manageTool.name).toBe("manage_ntp_client"));
  it("is not readOnly", () => expect(manageTool.annotations.readOnlyHint).toBe(false));
  it("is not destructive", () => expect(manageTool.annotations.destructiveHint).toBe(false));
  it("is idempotent", () => expect(manageTool.annotations.idempotentHint).toBe(true));

  describe("input schema", () => {
    it("parses valid input with enabled only", () => {
      expect(
        manageTool.inputSchema.safeParse({ routerId: "r1", enabled: true }).success,
      ).toBe(true);
    });
    it("dryRun defaults to false", () => {
      expect(manageTool.inputSchema.parse({ routerId: "r1", enabled: true }).dryRun).toBe(false);
    });
    it("rejects extra fields", () => {
      expect(
        manageTool.inputSchema.safeParse({ routerId: "r1", extra: true }).success,
      ).toBe(false);
    });
    it("rejects invalid mode", () => {
      expect(
        manageTool.inputSchema.safeParse({ routerId: "r1", mode: "foobar" }).success,
      ).toBe(false);
    });
    it("validates mode enum", () => {
      expect(
        manageTool.inputSchema.safeParse({ routerId: "r1", mode: "unicast" }).success,
      ).toBe(true);
    });
  });

  function makeNtpContext(
    current: Record<string, unknown> = { ".id": "*1", enabled: "true", mode: "unicast", servers: "pool.ntp.org" },
  ): ToolContext {
    return {
      routerId: "test-router",
      correlationId: "test-corr",
      routerConfig: {} as RouterConfig,
      sshClient: {} as SshClient,
      ftpClient: {} as FtpClient,
      identity: {
        id: "superadmin-builtin",
        name: "admin",
        role: "superadmin",
        allowedRouters: ["*"],
        allowedToolPatterns: ["*"],
      } as unknown as Identity,
      routerClient: {
        get: vi.fn().mockResolvedValue([current]),
        update: vi.fn().mockResolvedValue(undefined),
      } as unknown as RouterOSRestClient,
    };
  }

  it("returns already_set when no changes needed", async () => {
    const ctx = makeNtpContext({ ".id": "*1", enabled: "true", mode: "unicast", servers: "pool.ntp.org" });
    const result = await manageTool.handler(
      { routerId: "test-router", enabled: true, mode: "unicast", servers: "pool.ntp.org" },
      ctx,
    );
    expect((result.structuredContent as Record<string, unknown>).action).toBe("already_set");
    expect(ctx.routerClient.update).not.toHaveBeenCalled();
  });

  it("updates enabled field", async () => {
    const ctx = makeNtpContext({ ".id": "*1", enabled: "true", mode: "unicast" });
    const result = await manageTool.handler(
      { routerId: "test-router", enabled: false },
      ctx,
    );
    expect((result.structuredContent as Record<string, unknown>).action).toBe("updated");
    expect(ctx.routerClient.update).toHaveBeenCalledWith(
      "system/ntp/client",
      "*1",
      expect.objectContaining({ enabled: "false" }),
    );
  });

  it("updates servers field", async () => {
    const ctx = makeNtpContext({ ".id": "*1", enabled: "true", servers: "old.ntp.org" });
    const result = await manageTool.handler(
      { routerId: "test-router", servers: "0.pool.ntp.org,1.pool.ntp.org" },
      ctx,
    );
    expect((result.structuredContent as Record<string, unknown>).action).toBe("updated");
    expect(ctx.routerClient.update).toHaveBeenCalledWith(
      "system/ntp/client",
      "*1",
      expect.objectContaining({ servers: "0.pool.ntp.org,1.pool.ntp.org" }),
    );
  });

  it("dry_run returns preview without calling update", async () => {
    const ctx = makeNtpContext({ ".id": "*1", enabled: "true" });
    const result = await manageTool.handler(
      { routerId: "test-router", enabled: false, dryRun: true },
      ctx,
    );
    expect((result.structuredContent as Record<string, unknown>).action).toBe("dry_run");
    expect(ctx.routerClient.update).not.toHaveBeenCalled();
  });
});
```

- [ ] **Step 3: Run the new tests to verify they fail**

```bash
npx vitest run test/unit/domain/network-services-tools.test.ts
```

Expected: FAIL — the new `describe("manage_ntp_client")` block fails because the tool doesn't exist yet.

- [ ] **Step 4: Add `manage_ntp_client` to `network-services-tools.ts`**

Add before the `export` line in `src/domain/tools/network-services-tools.ts`:

```typescript
// ---------------------------------------------------------------------------
// manage_ntp_client
// ---------------------------------------------------------------------------

const manageNtpClientInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    enabled: z.boolean().optional().describe("Enable or disable the NTP client"),
    mode: z
      .enum(["unicast", "broadcast", "multicast", "manycast"])
      .optional()
      .describe("NTP client mode"),
    servers: z
      .string()
      .optional()
      .describe("Comma-separated NTP server addresses or hostnames"),
    vlanInterface: z
      .string()
      .optional()
      .describe("Source VLAN interface for NTP packets"),
    dryRun: z.boolean().default(false).describe("Preview changes without applying"),
  })
  .strict();

const manageNtpClientTool: ToolDefinition = {
  name: "manage_ntp_client",
  title: "Manage NTP Client",
  description:
    "Configure the RouterOS NTP client: enable/disable, set server addresses, mode, and optional VLAN source interface. Idempotent: returns already_set if all provided values already match. Complements get_ntp_settings.",
  inputSchema: manageNtpClientInputSchema,
  annotations: {
    readOnlyHint: false,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  snapshotPaths: ["system/ntp/client"],
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = manageNtpClientInputSchema.parse(params);
    log.info({ routerId: context.routerId }, "Managing NTP client");
    try {
      const results = await context.routerClient.get<RouterOSRecord>("system/ntp/client");
      const current = (
        Array.isArray(results) && results.length > 0 ? results[0] : results
      ) as Record<string, string>;
      const id = current[".id"];

      const changes: Record<string, string> = {};
      const diff: Array<{ property: string; before: string | null; after: string }> = [];

      if (parsed.enabled !== undefined) {
        const wantEnabled = String(parsed.enabled);
        const isEnabled = current.enabled;
        if (isEnabled !== wantEnabled) {
          changes.enabled = wantEnabled;
          diff.push({ property: "enabled", before: isEnabled ?? null, after: wantEnabled });
        }
      }
      if (parsed.mode !== undefined && current.mode !== parsed.mode) {
        changes.mode = parsed.mode;
        diff.push({ property: "mode", before: current.mode ?? null, after: parsed.mode });
      }
      if (parsed.servers !== undefined && current.servers !== parsed.servers) {
        changes.servers = parsed.servers;
        diff.push({ property: "servers", before: current.servers ?? null, after: parsed.servers });
      }
      if (parsed.vlanInterface !== undefined && current["vlan-interface"] !== parsed.vlanInterface) {
        changes["vlan-interface"] = parsed.vlanInterface;
        diff.push({
          property: "vlan-interface",
          before: current["vlan-interface"] ?? null,
          after: parsed.vlanInterface,
        });
      }

      if (diff.length === 0) {
        return {
          content: `NTP client on ${context.routerId} already has the requested configuration. No changes made.`,
          structuredContent: { action: "already_set", routerId: context.routerId },
        };
      }

      if (parsed.dryRun) {
        return {
          content: `Dry run: Would update NTP client on ${context.routerId}: ${diff.map((d) => `${d.property}: ${d.before} → ${d.after}`).join(", ")}.`,
          structuredContent: { action: "dry_run", diff },
        };
      }

      await context.routerClient.update("system/ntp/client", id, changes);
      log.info({ routerId: context.routerId, changes }, "NTP client updated");
      return {
        content: `Updated NTP client on ${context.routerId}: ${diff.map((d) => `${d.property}: ${d.before} → ${d.after}`).join(", ")}.`,
        structuredContent: { action: "updated", routerId: context.routerId, diff },
      };
    } catch (err) {
      if (err instanceof MikroMCPError) throw err;
      throw enrichError(err, { routerId: context.routerId, tool: "manage_ntp_client" });
    }
  },
};
```

Also update the export at the bottom of `network-services-tools.ts`:

```typescript
export const networkServicesTools: ToolDefinition[] = [
  getSnmpSettingsTool,
  getNtpSettingsTool,
  listNetwatchEntriesTool,
  manageNetwatchEntryTool,
  listNeighborsTool,
  listArpEntriesTool,
  manageNtpClientTool,   // add this
];
```

- [ ] **Step 5: Run test to verify it passes**

```bash
npx vitest run test/unit/domain/network-services-tools.test.ts
```

Expected: All tests PASS (existing + new).

- [ ] **Step 6: Commit**

```bash
git add src/domain/tools/network-services-tools.ts test/unit/domain/network-services-tools.test.ts
git commit -m "feat(tools): add manage_ntp_client"
```

---

## Task 6: Wire New Tools into `index.ts`

**Files:**
- Modify: `src/domain/tools/index.ts`

- [ ] **Step 1: Add imports and spread new arrays**

Edit `src/domain/tools/index.ts` — add these 4 new imports after the `ovpnTools` import:

```typescript
import { userGroupTools } from "./user-group-tools.js";
import { upgradeTools } from "./upgrade-tools.js";
import { backupTools } from "./backup-tools.js";
import { logTools } from "./log-tools.js";
```

Add the 4 new spreads at the end of `baseTools`:

```typescript
const baseTools: ToolDefinition[] = [
  // ... all existing entries ...
  ...pppoeTools,
  ...ovpnTools,
  ...userGroupTools,   // add
  ...upgradeTools,     // add
  ...backupTools,      // add
  ...logTools,         // add
];
```

- [ ] **Step 2: Run the full test suite**

```bash
npm test
```

Expected: All tests PASS. No type errors. No lint errors.

- [ ] **Step 3: Verify tool count**

```bash
node -e "import('./dist/main.js').catch(() => {})" 2>/dev/null || true
# Use a quick vitest-based count instead:
npx vitest run --reporter=verbose 2>&1 | grep -E "Tests|passed" | tail -5
```

Or write a quick inline check:

```bash
node --input-type=module <<'EOF'
import { allTools } from "./src/domain/tools/index.js";
console.log("Tool count:", allTools.length);
EOF
```

Expected output: `Tool count: 99`

- [ ] **Step 4: Commit**

```bash
git add src/domain/tools/index.ts
git commit -m "feat(tools): register v1.4 tools in index (99 total)"
```

---

## Task 7: Documentation Updates

**Files:**
- Modify: `docs/wiki/Available-Tools.md` (add 11 tool entries)
- Modify: `README.md` (update badge 88→99, update tool count text, add table rows)
- Modify: `CHANGELOG.md` (add [Unreleased] entries)

- [ ] **Step 1: Update `CHANGELOG.md` — add Unreleased entries**

In the `[Unreleased]` section at the top of `CHANGELOG.md`, add under `### Added`:

```markdown
### Added
- `list_user_groups` — list local user groups with policy bitmask (`/user/group`)
- `manage_user_group` — create, update, or remove user groups (idempotent by name; action=update to change policy)
- `get_upgrade_status` — check RouterOS and routerboard firmware upgrade availability and current channel
- `manage_upgrade` — trigger a package update check (`action=check`) or install an upgrade (`action=install`, destructive)
- `create_backup` — create a binary router config backup file (optional encryption password)
- `export_config` — export running config as a RouterOS script text (equivalent to `/export`; compact mode supported)
- `list_log_rules` — list system logging rules with topic/action filters
- `manage_log_rule` — add, remove, enable, or disable log rules (idempotent by topics+action composite key)
- `list_log_actions` — list log action targets (memory, disk, remote syslog, etc.) with type filter
- `manage_log_action` — add or remove log action targets (idempotent by name; type required for add)
- `manage_ntp_client` — configure NTP client: enable/disable, set server addresses, mode, and VLAN source interface (complements `get_ntp_settings`)
```

- [ ] **Step 2: Update `README.md` badge and counts**

Replace badge line:
```
[![Tools](https://img.shields.io/badge/MCP%20tools-88-0f766e)](#available-tools)
```
with:
```
[![Tools](https://img.shields.io/badge/MCP%20tools-99-0f766e)](#available-tools)
```

Replace all occurrences of `88` referring to the tool count (e.g., "88 registered MikroMCP tools", "All 88 tools") with `99`.

Add rows to the Available Tools table for each new tool. Follow the existing format (subsystem group header + individual tool rows).

- [ ] **Step 3: Update `docs/wiki/Available-Tools.md`**

For each of the 11 new tools, add a section using this format (repeat for each tool):

```markdown
### `list_user_groups` — Read

List local user groups on a MikroTik router with their policy bitmask.

| Parameter | Type | Default | Description |
|---|---|---|---|
| `routerId` | string | — | Target router identifier |
| `limit` | number | 100 | Maximum groups to return (1–500) |

**Example prompt:** "List all user groups on router home-gw"

---

### `manage_user_group` — Write

Add, update, or remove a RouterOS user group. Idempotent by name.

| Parameter | Type | Default | Description |
|---|---|---|---|
| `routerId` | string | — | Target router identifier |
| `action` | enum | — | `add`, `update`, or `remove` |
| `name` | string | — | Group name (idempotency key) |
| `policy` | string | — | Comma-separated policy list (e.g. `read,write,ftp`). Required for add. |
| `skin` | string | — | Winbox skin (optional) |
| `dryRun` | boolean | false | Preview without applying |

**Example prompt:** "Add a user group named 'ops' with read and write policies on router home-gw"

---

### `get_upgrade_status` — Read

Check available RouterOS/firmware upgrades, current channel, and routerboard firmware.

| Parameter | Type | Default | Description |
|---|---|---|---|
| `routerId` | string | — | Target router identifier |

**Example prompt:** "Is there a RouterOS upgrade available for router core-sw?"

---

### `manage_upgrade` — Destructive

Trigger a RouterOS package update check or start an upgrade. The `install` action causes a router reboot.

| Parameter | Type | Default | Description |
|---|---|---|---|
| `routerId` | string | — | Target router identifier |
| `action` | enum | — | `check` (non-destructive) or `install` (destructive, reboots) |
| `dryRun` | boolean | false | Preview without executing |

**Example prompt:** "Check for RouterOS upgrades on router border-r1, then install if available"

---

### `create_backup` — Write

Create a binary RouterOS configuration backup file on the router.

| Parameter | Type | Default | Description |
|---|---|---|---|
| `routerId` | string | — | Target router identifier |
| `name` | string | `backup` | Backup file name (without .backup extension) |
| `password` | string | — | Optional encryption password |
| `dryRun` | boolean | false | Preview without creating |

**Example prompt:** "Create an encrypted backup named 'pre-upgrade' on router home-gw"

---

### `export_config` — Read

Export the running RouterOS configuration as a RouterOS script text.

| Parameter | Type | Default | Description |
|---|---|---|---|
| `routerId` | string | — | Target router identifier |
| `compact` | boolean | false | Export only non-default settings |
| `file` | string | — | Save to a router file instead of returning inline |

**Example prompt:** "Export the full running config from router core-sw as a script"

---

### `list_log_rules` — Read

List RouterOS system logging rules with topics, action target, and enabled status.

| Parameter | Type | Default | Description |
|---|---|---|---|
| `routerId` | string | — | Target router identifier |
| `topics` | string | — | Filter by topics (substring match) |
| `logAction` | string | — | Filter by action name (exact match) |
| `limit` | number | 100 | Maximum rules to return (1–500) |

**Example prompt:** "Show all log rules that route to disk on router home-gw"

---

### `manage_log_rule` — Write

Add, remove, enable, or disable a RouterOS system logging rule. Idempotent by topics+action.

| Parameter | Type | Default | Description |
|---|---|---|---|
| `routerId` | string | — | Target router identifier |
| `action` | enum | — | `add`, `remove`, `enable`, or `disable` |
| `topics` | string | — | Log topics (e.g. `firewall`, `system,!debug`) |
| `logAction` | string | — | Log action name to route to (e.g. `memory`, `disk`) |
| `prefix` | string | — | Optional log entry prefix |
| `dryRun` | boolean | false | Preview without applying |

**Example prompt:** "Add a log rule to send firewall topics to the remote syslog action on router edge-r1"

---

### `list_log_actions` — Read

List RouterOS log action targets (memory, disk, remote syslog, etc.).

| Parameter | Type | Default | Description |
|---|---|---|---|
| `routerId` | string | — | Target router identifier |
| `type` | enum | — | Filter by type: `memory`, `disk`, `remote`, `echo`, `email` |
| `limit` | number | 100 | Maximum actions to return (1–500) |

**Example prompt:** "List all remote syslog action targets on router home-gw"

---

### `manage_log_action` — Write

Add or remove a RouterOS log action target. Idempotent by name.

| Parameter | Type | Default | Description |
|---|---|---|---|
| `routerId` | string | — | Target router identifier |
| `action` | enum | — | `add` or `remove` |
| `name` | string | — | Action name (idempotency key) |
| `type` | enum | — | Action type (required for add): `memory`, `disk`, `remote`, `echo`, `email` |
| `remote` | string | — | Remote syslog server IP (for type=remote) |
| `remotePort` | number | — | Remote syslog UDP port (default 514) |
| `diskFileName` | string | — | Disk log file name (for type=disk) |
| `dryRun` | boolean | false | Preview without applying |

**Example prompt:** "Add a remote syslog action named 'central-syslog' pointing to 10.0.0.50 on router home-gw"

---

### `manage_ntp_client` — Write

Configure the RouterOS NTP client settings. Idempotent: no-op when values already match.

| Parameter | Type | Default | Description |
|---|---|---|---|
| `routerId` | string | — | Target router identifier |
| `enabled` | boolean | — | Enable or disable the NTP client |
| `mode` | enum | — | `unicast`, `broadcast`, `multicast`, or `manycast` |
| `servers` | string | — | Comma-separated NTP server addresses or hostnames |
| `vlanInterface` | string | — | Source VLAN interface for NTP packets |
| `dryRun` | boolean | false | Preview without applying |

**Example prompt:** "Set the NTP servers to pool.ntp.org on router home-gw and enable the client"
```

- [ ] **Step 4: Commit documentation**

```bash
git add docs/wiki/Available-Tools.md README.md CHANGELOG.md
git commit -m "docs: update Available-Tools, README, and CHANGELOG for v1.4"
```

---

## Task 8: Final Verification

- [ ] **Step 1: Run full test suite**

```bash
npm test
```

Expected: All tests pass, no TypeScript errors, no lint errors.

- [ ] **Step 2: Verify tool count is 99**

```bash
node --input-type=module <<'EOF'
import { allTools } from "./src/domain/tools/index.js";
console.log("Total tools:", allTools.length);
const names = allTools.map(t => t.name);
const v14 = ["list_user_groups","manage_user_group","get_upgrade_status","manage_upgrade","create_backup","export_config","list_log_rules","manage_log_rule","list_log_actions","manage_log_action","manage_ntp_client"];
v14.forEach(n => console.log(n, names.includes(n) ? "✓" : "MISSING"));
EOF
```

Expected: `Total tools: 99` and all 11 v1.4 tool names show `✓`.

- [ ] **Step 3: Run typecheck**

```bash
npm run typecheck
```

Expected: No errors.

- [ ] **Step 4: Open PR**

```bash
git push origin feat/v1.4-system-admin-depth
gh pr create --title "feat: v1.4 — System Administration Depth (99 tools)" \
  --body "$(cat <<'EOF'
## Summary
- Adds 11 new tools covering user groups, firmware upgrades, config backup/export, log rule management, and NTP write
- Tool count: 88 → 99
- All new tools follow established patterns: Zod strict schemas, idempotency, dry-run on every write

## New tools
- `list_user_groups`, `manage_user_group` — user group lifecycle (`/user/group`)
- `get_upgrade_status`, `manage_upgrade` — firmware upgrade status and install trigger
- `create_backup`, `export_config` — config backup and script export
- `list_log_rules`, `manage_log_rule` — log rule management (`/system/logging`)
- `list_log_actions`, `manage_log_action` — log action targets (`/system/logging/action`)
- `manage_ntp_client` — NTP client configuration (complements `get_ntp_settings`)

## Test plan
- [ ] All unit tests pass (`npm test`)
- [ ] `npm run typecheck` clean
- [ ] Tool count confirmed at 99
- [ ] README badge updated to 99
- [ ] CHANGELOG `[Unreleased]` section updated

🤖 Generated with [Claude Code](https://claude.com/claude-code)
EOF
)"
```

---

## Self-Review

**Spec coverage check:**

| Roadmap item | Task |
|---|---|
| `list_user_groups` | Task 1 |
| `manage_user_group` | Task 1 |
| `get_upgrade_status` | Task 2 |
| `manage_upgrade` | Task 2 |
| `create_backup` | Task 3 |
| `export_config` | Task 3 |
| `list_log_rules` | Task 4 |
| `manage_log_rule` | Task 4 |
| `list_log_actions` | Task 4 |
| `manage_log_action` | Task 4 |
| `manage_ntp_client` | Task 5 |
| Wire all tools | Task 6 |
| Doc updates (wiki, README, CHANGELOG) | Task 7 |

All 11 roadmap tools are covered. ✓

**Notes for implementer:**
- `network-services-tools.test.ts` uses a `makeContext` helper already — check the existing helper signature before adding new tests. The new `makeNtpContext` helper in Task 5 is local to the `describe` block to avoid colliding with the existing `makeContext`.
- `manage_upgrade` is `destructiveHint: true` — the tool-registry will require a confirmation token for `operator` and `readonly` roles. No extra code needed inside the handler.
- `export_config` uses `execute<string>` — the RouterOS REST response for `/system/export` returns a plain text string, not JSON. If the response is an object (e.g., when saving to file), the handler falls back to `JSON.stringify`.
