# v1.2 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:** Ship IP pool rename, VLAN manage tool, DHCP lease management, DHCP client tools, and IP service tools.

**Architecture:** Four new tool files (`vlan-tools.ts`, `ip-pool-tools.ts`, `dhcp-client-tools.ts`, `ip-service-tools.ts`); two existing files modified (`interface-tools.ts` drops `create_vlan`, `dhcp-server-tools.ts` drops pool tools + gains `offset`); `dhcp-tools.ts` gains `leaseType` filter and `manage_dhcp_lease`. All wired through `index.ts`. Tests mirror source structure.

**Tech Stack:** TypeScript, Zod, Vitest, RouterOS REST API, ESM with `.js` extensions.

---

## File map

| Status | File | Change |
|---|---|---|
| Create | `src/domain/tools/vlan-tools.ts` | `manage_vlan` (rename+enhance `create_vlan`) |
| Create | `src/domain/tools/ip-pool-tools.ts` | `list_ip_pools`, `manage_ip_pool` (moved from dhcp-server-tools) |
| Create | `src/domain/tools/dhcp-client-tools.ts` | `list_dhcp_clients`, `manage_dhcp_client` |
| Create | `src/domain/tools/ip-service-tools.ts` | `list_ip_services`, `manage_ip_service` |
| Modify | `src/domain/tools/interface-tools.ts` | Remove `createVlanTool`; export only `[listInterfacesTool]` |
| Modify | `src/domain/tools/dhcp-server-tools.ts` | Remove pool tools; add `offset` param to `list_dhcp_servers` |
| Modify | `src/domain/tools/dhcp-tools.ts` | Add `leaseType` filter + `manage_dhcp_lease` tool |
| Modify | `src/domain/tools/index.ts` | Import new files, remove old pool/vlan entries |
| Create | `test/unit/domain/vlan-tools.test.ts` | Full test coverage |
| Create | `test/unit/domain/ip-pool-tools.test.ts` | Full test coverage |
| Create | `test/unit/domain/dhcp-client-tools.test.ts` | Full test coverage |
| Create | `test/unit/domain/ip-service-tools.test.ts` | Full test coverage |
| Modify | `test/unit/domain/dhcp-server-tools.test.ts` | Remove pool tests; update tool count |
| Modify | `test/unit/domain/dhcp-tools.test.ts` | Add leaseType + lease management tests |
| Modify | `test/unit/domain/interface-tools.test.ts` | Remove create_vlan test; update tool count |
| Modify | `CHANGELOG.md` | Add [Unreleased] entries |
| Modify | `docs/wiki/Available-Tools.md` | New/renamed tool entries |
| Modify | `README.md` | Update tool count |

---

## Task 1: IP Pool tools (extract + rename)

**Files:**
- Create: `src/domain/tools/ip-pool-tools.ts`
- Modify: `src/domain/tools/dhcp-server-tools.ts` (remove pool tools, add offset)
- Create: `test/unit/domain/ip-pool-tools.test.ts`
- Modify: `test/unit/domain/dhcp-server-tools.test.ts`

- [ ] **Step 1: Write the failing test for ip-pool-tools**

Create `test/unit/domain/ip-pool-tools.test.ts`:

```typescript
import { describe, it, expect, vi } from "vitest";
import { ipPoolTools } from "../../../src/domain/tools/ip-pool-tools.js";
import type { ToolContext } from "../../../src/domain/tools/tool-definition.js";
import type { RouterOSRestClient } from "../../../src/adapter/rest-client.js";
import type { RouterConfig } 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 makeRouterConfig(): RouterConfig {
  return {
    id: "test-router",
    host: "192.168.1.1",
    port: 443,
    tls: { enabled: true, rejectUnauthorized: false },
    credentials: { source: "env", envPrefix: "ROUTER_TEST" },
    tags: [],
    rosVersion: "7",
  };
}

function makeContext(records: Record<string, unknown>[] = []): ToolContext {
  return {
    routerId: "test-router",
    correlationId: "test-corr",
    routerConfig: makeRouterConfig(),
    identity: { id: "superadmin-builtin", role: "superadmin" as const, allowedRouters: [], allowedToolPatterns: [] },
    sshClient: { execute: vi.fn().mockResolvedValue("") } as unknown as SshClient,
    ftpClient: { upload: vi.fn().mockResolvedValue(undefined), connect: vi.fn().mockResolvedValue(undefined) } as unknown as FtpClient,
    routerClient: {
      get: vi.fn().mockResolvedValue(records),
      create: vi.fn().mockResolvedValue({ ".id": "*1", name: "pool1", ranges: "192.168.1.100-192.168.1.200" }),
      update: vi.fn().mockResolvedValue(undefined),
      remove: vi.fn().mockResolvedValue(undefined),
    } as unknown as RouterOSRestClient,
  };
}

const [listTool, manageTool] = ipPoolTools;

describe("ipPoolTools", () => {
  describe("metadata", () => {
    it("exports 2 tools", () => expect(ipPoolTools).toHaveLength(2));
    it("has correct names", () => {
      expect(listTool.name).toBe("list_ip_pools");
      expect(manageTool.name).toBe("manage_ip_pool");
    });
    it("list_ip_pools is readOnly", () => expect(listTool.annotations.readOnlyHint).toBe(true));
    it("manage_ip_pool is not readOnly", () => expect(manageTool.annotations.readOnlyHint).toBe(false));
    it("manage_ip_pool has snapshotPaths", () => expect(manageTool.snapshotPaths).toContain("ip/pool"));
  });

  describe("list_ip_pools handler", () => {
    it("returns pools from ip/pool", async () => {
      const pools = [
        { ".id": "*1", name: "pool1", ranges: "192.168.1.100-192.168.1.200" },
        { ".id": "*2", name: "pool2", ranges: "10.0.0.10-10.0.0.50" },
      ];
      const ctx = makeContext(pools);
      const result = await listTool.handler({ routerId: "test-router" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect((sc.pools as unknown[]).length).toBe(2);
      expect(sc.total).toBe(2);
    });

    it("filters by name substring", async () => {
      const pools = [
        { ".id": "*1", name: "pool-lan", ranges: "192.168.1.100-192.168.1.200" },
        { ".id": "*2", name: "pool-wan", ranges: "10.0.0.10-10.0.0.50" },
      ];
      const ctx = makeContext(pools);
      const result = await listTool.handler({ routerId: "test-router", name: "lan" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect((sc.pools as unknown[]).length).toBe(1);
    });

    it("respects limit and offset", async () => {
      const pools = Array.from({ length: 5 }, (_, i) => ({
        ".id": `*${i}`,
        name: `pool${i}`,
        ranges: `192.168.${i}.1-192.168.${i}.100`,
      }));
      const ctx = makeContext(pools);
      const result = await listTool.handler({ routerId: "test-router", limit: 2, offset: 1 }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect((sc.pools as unknown[]).length).toBe(2);
      expect(sc.total).toBe(5);
      expect(sc.hasMore).toBe(true);
    });
  });

  describe("manage_ip_pool handler - add", () => {
    it("creates pool when not existing", async () => {
      const ctx = makeContext([]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "add", name: "pool1", ranges: "192.168.1.100-192.168.1.200" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("created");
    });

    it("returns already_exists when pool with same name and ranges exists", async () => {
      const ctx = makeContext([{ ".id": "*1", name: "pool1", ranges: "192.168.1.100-192.168.1.200" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "add", name: "pool1", ranges: "192.168.1.100-192.168.1.200" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("already_exists");
    });

    it("throws CONFLICT when pool exists with different ranges", async () => {
      const ctx = makeContext([{ ".id": "*1", name: "pool1", ranges: "10.0.0.1-10.0.0.100" }]);
      await expect(
        manageTool.handler(
          { routerId: "test-router", action: "add", name: "pool1", ranges: "192.168.1.100-192.168.1.200" },
          ctx,
        ),
      ).rejects.toMatchObject({ category: ErrorCategory.CONFLICT });
    });

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

    it("dry_run returns diff without creating", async () => {
      const ctx = makeContext([]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "add", name: "pool1", ranges: "192.168.1.100-192.168.1.200", dryRun: true },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("dry_run");
      expect(ctx.routerClient.create).not.toHaveBeenCalled();
    });
  });

  describe("manage_ip_pool handler - remove", () => {
    it("removes existing pool", async () => {
      const ctx = makeContext([{ ".id": "*1", name: "pool1", ranges: "192.168.1.100-192.168.1.200" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "remove", name: "pool1" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("removed");
      expect(ctx.routerClient.remove).toHaveBeenCalledWith("ip/pool", "*1");
    });

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

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

```bash
cd /Users/alikarami/Projects/MikroMCP && npx vitest run test/unit/domain/ip-pool-tools.test.ts 2>&1 | tail -20
```

Expected: FAIL — `ip-pool-tools.ts` module not found.

- [ ] **Step 3: Create `src/domain/tools/ip-pool-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("ip-pool-tools");

const listIpPoolsInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    name: z.string().optional().describe("Filter by pool name (substring match)"),
    limit: z.number().int().min(1).max(500).default(100).describe("Maximum number of pools to return"),
    offset: z.number().int().min(0).default(0).describe("Offset for pagination"),
  })
  .strict();

const listIpPoolsTool: ToolDefinition = {
  name: "list_ip_pools",
  title: "List IP Pools",
  description: "List IP address pools on a MikroTik router. Supports filtering by name and pagination.",
  inputSchema: listIpPoolsInputSchema,
  annotations: {
    readOnlyHint: true,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = listIpPoolsInputSchema.parse(params);
    log.info({ routerId: context.routerId }, "Listing IP pools");
    try {
      const allPools = await context.routerClient.get<RouterOSRecord>("ip/pool", {
        limit: undefined,
        offset: undefined,
      });

      let filtered = allPools as Record<string, string>[];
      if (parsed.name) {
        filtered = filtered.filter((p) => (p.name ?? "").includes(parsed.name!));
      }

      const total = filtered.length;
      const pools = filtered.slice(parsed.offset, parsed.offset + parsed.limit);
      const hasMore = parsed.offset + parsed.limit < total;

      return {
        content: `IP pools on ${context.routerId}: ${total} total, showing ${pools.length} (offset ${parsed.offset})`,
        structuredContent: {
          routerId: context.routerId,
          pools,
          total,
          hasMore,
          offset: parsed.offset,
          limit: parsed.limit,
        },
      };
    } catch (err) {
      throw enrichError(err, { routerId: context.routerId, tool: "list_ip_pools" });
    }
  },
};

const manageIpPoolInputSchema = 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("Pool name — idempotency key"),
    ranges: z
      .string()
      .optional()
      .describe("IP range (e.g. '192.168.1.100-192.168.1.200'; required for add)"),
    nextPool: z.string().optional().describe("Next pool name for overflow"),
    dryRun: z.boolean().default(false).describe("Preview changes without applying"),
  })
  .strict();

const manageIpPoolTool: ToolDefinition = {
  name: "manage_ip_pool",
  title: "Manage IP Pool",
  description:
    "Add or remove an IP address pool. Idempotent by name: add returns already_exists if a pool with the same name and ranges already exists.",
  inputSchema: manageIpPoolInputSchema,
  annotations: {
    readOnlyHint: false,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  snapshotPaths: ["ip/pool"],
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = manageIpPoolInputSchema.parse(params);
    log.info({ routerId: context.routerId, action: parsed.action, name: parsed.name }, "Managing IP pool");
    try {
      const allPools = await context.routerClient.get<RouterOSRecord>("ip/pool", {
        limit: undefined,
        offset: undefined,
      });
      const existing = (allPools as Record<string, string>[]).find((p) => p.name === parsed.name);

      if (parsed.action === "add") {
        if (!parsed.ranges) {
          throw new MikroMCPError({
            category: ErrorCategory.VALIDATION,
            code: "IP_POOL_RANGES_REQUIRED",
            message: "ranges is required when adding an IP pool.",
            details: { name: parsed.name },
            recoverability: {
              retryable: false,
              suggestedAction: "Provide the IP range (e.g. '192.168.1.100-192.168.1.200').",
            },
          });
        }

        if (existing) {
          if (existing.ranges === parsed.ranges) {
            return {
              content: `IP pool "${parsed.name}" already exists with the same ranges. No changes made.`,
              structuredContent: { action: "already_exists", pool: existing },
            };
          }
          throw new MikroMCPError({
            category: ErrorCategory.CONFLICT,
            code: "IP_POOL_CONFLICT",
            message: `IP pool "${parsed.name}" exists with different ranges.`,
            details: { existing: existing.ranges, requested: parsed.ranges },
            recoverability: {
              retryable: false,
              suggestedAction: "Remove the existing pool first or use a different name.",
              alternativeTools: ["manage_ip_pool"],
            },
          });
        }

        if (parsed.dryRun) {
          const diff = [
            { property: "name", before: null, after: parsed.name },
            { property: "ranges", before: null, after: parsed.ranges },
            ...(parsed.nextPool ? [{ property: "next-pool", before: null, after: parsed.nextPool }] : []),
          ];
          return {
            content: `Dry run: Would add IP pool "${parsed.name}" with ranges ${parsed.ranges}.`,
            structuredContent: { action: "dry_run", diff },
          };
        }

        const body: Record<string, string> = { name: parsed.name, ranges: parsed.ranges };
        if (parsed.nextPool) body["next-pool"] = parsed.nextPool;

        const created = await context.routerClient.create("ip/pool", body);
        log.info({ name: parsed.name, id: created[".id"] }, "IP pool added");
        return {
          content: `Added IP pool "${parsed.name}" with ranges ${parsed.ranges}.`,
          structuredContent: { action: "created", pool: created },
        };
      }

      // remove
      if (!existing) {
        return {
          content: `IP pool "${parsed.name}" not found. Nothing to remove.`,
          structuredContent: { action: "not_found", name: parsed.name },
        };
      }
      if (parsed.dryRun) {
        return {
          content: `Dry run: Would remove IP pool "${parsed.name}".`,
          structuredContent: { action: "dry_run", diff: [{ property: "name", before: parsed.name, after: null }] },
        };
      }
      await context.routerClient.remove("ip/pool", existing[".id"]);
      log.info({ name: parsed.name }, "IP pool removed");
      return {
        content: `Removed IP pool "${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_ip_pool" });
    }
  },
};

export const ipPoolTools: ToolDefinition[] = [listIpPoolsTool, manageIpPoolTool];
```

- [ ] **Step 4: Run tests to verify they pass**

```bash
cd /Users/alikarami/Projects/MikroMCP && npx vitest run test/unit/domain/ip-pool-tools.test.ts 2>&1 | tail -20
```

Expected: All tests PASS.

- [ ] **Step 5: Remove pool tools from `dhcp-server-tools.ts` and add `offset` to `list_dhcp_servers`**

In `src/domain/tools/dhcp-server-tools.ts`:

Replace the `listDhcpServersInputSchema` with:

```typescript
const listDhcpServersInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    interface: z.string().optional().describe("Filter by interface name (exact match)"),
    limit: z
      .number()
      .int()
      .min(1)
      .max(500)
      .default(100)
      .describe("Maximum number of DHCP servers to return"),
    offset: z.number().int().min(0).default(0).describe("Offset for pagination"),
  })
  .strict();
```

Replace the `listDhcpServersTool` handler body with:

```typescript
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = listDhcpServersInputSchema.parse(params);
    log.info({ routerId: context.routerId }, "Listing DHCP servers");
    try {
      const allServers = await context.routerClient.get<RouterOSRecord>("ip/dhcp-server", {
        limit: undefined,
        offset: undefined,
      });

      let filtered = parsed.interface
        ? (allServers as Record<string, string>[]).filter((s) => s.interface === parsed.interface)
        : (allServers as Record<string, string>[]);

      const total = filtered.length;
      const servers = filtered.slice(parsed.offset, parsed.offset + parsed.limit);
      const hasMore = parsed.offset + parsed.limit < total;

      return {
        content: `DHCP servers on ${context.routerId}: ${total} total, showing ${servers.length} (offset ${parsed.offset})`,
        structuredContent: {
          routerId: context.routerId,
          servers,
          total,
          hasMore,
          offset: parsed.offset,
          limit: parsed.limit,
        },
      };
    } catch (err) {
      throw enrichError(err, { routerId: context.routerId, tool: "list_dhcp_servers" });
    }
  },
```

Then delete the entire `listDhcpPoolsInputSchema`, `listDhcpPoolsTool`, `manageDhcpPoolInputSchema`, `manageDhcpPoolTool` const blocks and update the export:

```typescript
export const dhcpServerTools: ToolDefinition[] = [listDhcpServersTool, manageDhcpServerTool];
```

- [ ] **Step 6: Update `dhcp-server-tools.test.ts` — remove pool tests, update tool count**

In `test/unit/domain/dhcp-server-tools.test.ts`:

Replace line 47:
```typescript
const [listServersTool, manageServerTool, listPoolsTool, managePoolTool] = dhcpServerTools;
```
with:
```typescript
const [listServersTool, manageServerTool] = dhcpServerTools;
```

Change the metadata test from `exports 4 tools` to `exports 2 tools`:
```typescript
it("exports 2 tools", () => expect(dhcpServerTools).toHaveLength(2));
```

And remove the `it("has correct tool names"...)` lines for `listPoolsTool` and `managePoolTool`, plus delete all `describe` blocks for `list_dhcp_pools` and `manage_dhcp_pool`.

Also update the tool names assertion to only check the two remaining tools.

- [ ] **Step 7: Run all affected tests**

```bash
cd /Users/alikarami/Projects/MikroMCP && npx vitest run test/unit/domain/ip-pool-tools.test.ts test/unit/domain/dhcp-server-tools.test.ts 2>&1 | tail -30
```

Expected: All PASS.

- [ ] **Step 8: Commit**

```bash
cd /Users/alikarami/Projects/MikroMCP && git add src/domain/tools/ip-pool-tools.ts src/domain/tools/dhcp-server-tools.ts test/unit/domain/ip-pool-tools.test.ts test/unit/domain/dhcp-server-tools.test.ts
git commit -m "$(cat <<'EOF'
feat: extract ip-pool tools; rename list_dhcp_pools→list_ip_pools, manage_dhcp_pool→manage_ip_pool; add offset to list_dhcp_servers

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
EOF
)"
```

---

## Task 2: VLAN manage tool (rename + add remove/enable/disable)

**Files:**
- Create: `src/domain/tools/vlan-tools.ts`
- Modify: `src/domain/tools/interface-tools.ts` (remove `createVlanTool`)
- Create: `test/unit/domain/vlan-tools.test.ts`
- Modify: `test/unit/domain/interface-tools.test.ts` (remove create_vlan test)

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

Create `test/unit/domain/vlan-tools.test.ts`:

```typescript
import { describe, it, expect, vi } from "vitest";
import { vlanTools } from "../../../src/domain/tools/vlan-tools.js";
import type { ToolContext } from "../../../src/domain/tools/tool-definition.js";
import type { RouterOSRestClient } from "../../../src/adapter/rest-client.js";
import type { RouterConfig } 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 makeRouterConfig(): RouterConfig {
  return {
    id: "test-router",
    host: "192.168.1.1",
    port: 443,
    tls: { enabled: true, rejectUnauthorized: false },
    credentials: { source: "env", envPrefix: "ROUTER_TEST" },
    tags: [],
    rosVersion: "7",
  };
}

function makeContext(records: Record<string, unknown>[] = []): ToolContext {
  return {
    routerId: "test-router",
    correlationId: "test-corr",
    routerConfig: makeRouterConfig(),
    identity: { id: "superadmin-builtin", role: "superadmin" as const, allowedRouters: [], allowedToolPatterns: [] },
    sshClient: { execute: vi.fn().mockResolvedValue("") } as unknown as SshClient,
    ftpClient: { upload: vi.fn().mockResolvedValue(undefined), connect: vi.fn().mockResolvedValue(undefined) } as unknown as FtpClient,
    routerClient: {
      get: vi.fn().mockResolvedValue(records),
      create: vi.fn().mockResolvedValue({ ".id": "*1", name: "vlan10", "vlan-id": "10" }),
      update: vi.fn().mockResolvedValue(undefined),
      remove: vi.fn().mockResolvedValue(undefined),
    } as unknown as RouterOSRestClient,
  };
}

const [manageTool] = vlanTools;

describe("vlanTools", () => {
  describe("metadata", () => {
    it("exports 1 tool", () => expect(vlanTools).toHaveLength(1));
    it("tool is named manage_vlan", () => expect(manageTool.name).toBe("manage_vlan"));
    it("is not readOnly", () => expect(manageTool.annotations.readOnlyHint).toBe(false));
    it("has snapshotPaths", () => expect(manageTool.snapshotPaths).toContain("interface/vlan"));
  });

  describe("manage_vlan - add", () => {
    it("creates vlan when not existing", async () => {
      const ctx = makeContext([]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "add", name: "vlan10", vlanId: 10, parentInterface: "ether1" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("created");
    });

    it("returns already_exists when vlan with same name, vlanId, parentInterface exists", async () => {
      const ctx = makeContext([
        { ".id": "*1", name: "vlan10", "vlan-id": "10", interface: "ether1", disabled: "false" },
      ]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "add", name: "vlan10", vlanId: 10, parentInterface: "ether1" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("already_exists");
    });

    it("idempotency handles RouterOS boolean disabled=false (string)", async () => {
      const ctx = makeContext([
        { ".id": "*1", name: "vlan10", "vlan-id": "10", interface: "ether1", disabled: "false" },
      ]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "add", name: "vlan10", vlanId: 10, parentInterface: "ether1", disabled: false },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("already_exists");
    });

    it("throws CONFLICT when vlan exists with different config", async () => {
      const ctx = makeContext([
        { ".id": "*1", name: "vlan10", "vlan-id": "20", interface: "ether1", disabled: "false" },
      ]);
      await expect(
        manageTool.handler(
          { routerId: "test-router", action: "add", name: "vlan10", vlanId: 10, parentInterface: "ether1" },
          ctx,
        ),
      ).rejects.toMatchObject({ category: ErrorCategory.CONFLICT });
    });

    it("dry_run returns diff without creating", async () => {
      const ctx = makeContext([]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "add", name: "vlan10", vlanId: 10, parentInterface: "ether1", dryRun: true },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("dry_run");
      expect(ctx.routerClient.create).not.toHaveBeenCalled();
    });
  });

  describe("manage_vlan - remove", () => {
    it("removes existing vlan", async () => {
      const ctx = makeContext([{ ".id": "*1", name: "vlan10", "vlan-id": "10", interface: "ether1" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "remove", name: "vlan10" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("removed");
      expect(ctx.routerClient.remove).toHaveBeenCalledWith("interface/vlan", "*1");
    });

    it("returns not_found when vlan missing on remove", async () => {
      const ctx = makeContext([]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "remove", name: "vlan99" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("not_found");
    });
  });

  describe("manage_vlan - enable/disable", () => {
    it("disables an existing vlan", async () => {
      const ctx = makeContext([{ ".id": "*1", name: "vlan10", "vlan-id": "10", interface: "ether1", disabled: "false" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "disable", name: "vlan10" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("disabled");
      expect(ctx.routerClient.update).toHaveBeenCalledWith("interface/vlan", "*1", { disabled: "true" });
    });

    it("enables an existing vlan", async () => {
      const ctx = makeContext([{ ".id": "*1", name: "vlan10", "vlan-id": "10", interface: "ether1", disabled: "true" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "enable", name: "vlan10" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("enabled");
    });

    it("throws NOT_FOUND on enable/disable when vlan missing", async () => {
      const ctx = makeContext([]);
      await expect(
        manageTool.handler({ routerId: "test-router", action: "enable", name: "vlan99" }, ctx),
      ).rejects.toMatchObject({ category: ErrorCategory.NOT_FOUND });
    });
  });
});
```

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

```bash
cd /Users/alikarami/Projects/MikroMCP && npx vitest run test/unit/domain/vlan-tools.test.ts 2>&1 | tail -10
```

Expected: FAIL — module not found.

- [ ] **Step 3: Create `src/domain/tools/vlan-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("vlan-tools");

function sanitizeComment(comment: string | undefined): string | undefined {
  if (comment === undefined) return undefined;
  return comment.replace(/[\x00-\x1f\x7f]/g, "");
}

const manageVlanInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    action: z
      .enum(["add", "remove", "enable", "disable"])
      .describe("Action to perform"),
    name: z
      .string()
      .regex(/^[a-zA-Z0-9_-]+$/)
      .max(15)
      .describe("VLAN interface name — idempotency key (alphanumeric, hyphens, underscores, max 15 chars)"),
    vlanId: z.number().int().min(1).max(4094).optional().describe("VLAN ID (1-4094; required for add)"),
    parentInterface: z.string().optional().describe("Parent interface name (e.g., ether1, bridge1; required for add)"),
    mtu: z.number().int().min(68).max(9000).default(1500).describe("MTU size (add only)"),
    disabled: z.boolean().default(false).describe("Whether to create the VLAN disabled (add only)"),
    comment: z.string().max(255).optional().describe("Optional comment (add only)"),
    dryRun: z.boolean().default(false).describe("Preview changes without applying"),
  })
  .strict();

const manageVlanTool: ToolDefinition = {
  name: "manage_vlan",
  title: "Manage VLAN Interface",
  description:
    "Add, remove, enable, or disable a VLAN interface. Idempotent by name: add returns already_exists when a VLAN with matching name, vlan-id, and parent interface exists. Supports dry-run mode.",
  inputSchema: manageVlanInputSchema,
  annotations: {
    readOnlyHint: false,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  snapshotPaths: ["interface/vlan"],
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = manageVlanInputSchema.parse(params);
    const comment = sanitizeComment(parsed.comment);

    log.info({ routerId: context.routerId, action: parsed.action, name: parsed.name }, "Managing VLAN");

    try {
      const allVlans = await context.routerClient.get<RouterOSRecord>("interface/vlan", {
        limit: undefined,
        offset: undefined,
      });
      const existing = (allVlans as Record<string, string>[]).find((v) => v.name === parsed.name);

      if (parsed.action === "add") {
        if (parsed.vlanId === undefined) {
          throw new MikroMCPError({
            category: ErrorCategory.VALIDATION,
            code: "VLAN_ID_REQUIRED",
            message: "vlanId is required when adding a VLAN.",
            details: { name: parsed.name },
            recoverability: { retryable: false, suggestedAction: "Provide vlanId (1-4094)." },
          });
        }
        if (!parsed.parentInterface) {
          throw new MikroMCPError({
            category: ErrorCategory.VALIDATION,
            code: "VLAN_PARENT_REQUIRED",
            message: "parentInterface is required when adding a VLAN.",
            details: { name: parsed.name },
            recoverability: { retryable: false, suggestedAction: "Provide the parent interface name." },
          });
        }

        if (existing) {
          if (
            existing["vlan-id"] === String(parsed.vlanId) &&
            existing.interface === parsed.parentInterface
          ) {
            log.info({ name: parsed.name }, "VLAN already exists with matching config");
            return {
              content: `VLAN "${parsed.name}" already exists with VLAN ID ${parsed.vlanId} on ${parsed.parentInterface}. No changes made.`,
              structuredContent: { action: "already_exists", vlan: existing },
            };
          }
          throw new MikroMCPError({
            category: ErrorCategory.CONFLICT,
            code: "VLAN_NAME_CONFLICT",
            message: `VLAN "${parsed.name}" already exists but with different configuration (vlan-id=${existing["vlan-id"]}, interface=${existing.interface}).`,
            details: {
              existing: { vlanId: existing["vlan-id"], interface: existing.interface },
              requested: { vlanId: parsed.vlanId, interface: parsed.parentInterface },
            },
            recoverability: {
              retryable: false,
              suggestedAction: "Use a different name or remove the existing VLAN first.",
              alternativeTools: ["manage_vlan"],
            },
          });
        }

        if (parsed.dryRun) {
          const diff = [
            { property: "name", before: null, after: parsed.name },
            { property: "vlan-id", before: null, after: String(parsed.vlanId) },
            { property: "interface", before: null, after: parsed.parentInterface },
            { property: "mtu", before: null, after: String(parsed.mtu) },
            { property: "disabled", before: null, after: parsed.disabled ? "true" : "false" },
            ...(comment ? [{ property: "comment", before: null, after: comment }] : []),
          ];
          return {
            content: `Dry run: Would create VLAN "${parsed.name}" (ID ${parsed.vlanId}) on ${parsed.parentInterface} with MTU ${parsed.mtu}.`,
            structuredContent: { action: "dry_run", diff },
          };
        }

        const body: Record<string, string> = {
          name: parsed.name,
          "vlan-id": String(parsed.vlanId),
          interface: parsed.parentInterface,
          mtu: String(parsed.mtu),
          disabled: parsed.disabled ? "true" : "false",
        };
        if (comment) body.comment = comment;

        const created = await context.routerClient.create("interface/vlan", body);
        log.info({ name: parsed.name, id: created[".id"] }, "VLAN created");
        return {
          content: `Created VLAN "${parsed.name}" (ID ${parsed.vlanId}) on ${parsed.parentInterface} with MTU ${parsed.mtu}.`,
          structuredContent: { action: "created", vlan: created },
        };
      }

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

      // enable / disable
      if (!existing) {
        throw new MikroMCPError({
          category: ErrorCategory.NOT_FOUND,
          code: "VLAN_NOT_FOUND",
          message: `VLAN "${parsed.name}" not found.`,
          details: { name: parsed.name },
          recoverability: {
            retryable: false,
            suggestedAction: "Verify the VLAN name with list_interfaces.",
            alternativeTools: ["list_interfaces"],
          },
        });
      }

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

      const disabledValue = parsed.action === "disable" ? "true" : "false";
      await context.routerClient.update("interface/vlan", existing[".id"], { disabled: disabledValue });
      const resultAction = parsed.action === "disable" ? "disabled" : "enabled";
      log.info({ name: parsed.name, action: resultAction }, "VLAN updated");
      return {
        content: `VLAN "${parsed.name}" ${resultAction}.`,
        structuredContent: { action: resultAction, name: parsed.name, id: existing[".id"] },
      };
    } catch (err) {
      if (err instanceof MikroMCPError) throw err;
      throw enrichError(err, { routerId: context.routerId, tool: "manage_vlan" });
    }
  },
};

export const vlanTools: ToolDefinition[] = [manageVlanTool];
```

- [ ] **Step 4: Run tests to verify they pass**

```bash
cd /Users/alikarami/Projects/MikroMCP && npx vitest run test/unit/domain/vlan-tools.test.ts 2>&1 | tail -20
```

Expected: All PASS.

- [ ] **Step 5: Remove `createVlanTool` from `interface-tools.ts`**

In `src/domain/tools/interface-tools.ts`, delete everything from `// create_vlan` comment through `const createVlanTool: ToolDefinition = { ... }` (the entire create_vlan block including `sanitizeComment`, `createVlanInputSchema`, and `createVlanTool`), then change the export from:

```typescript
export const interfaceTools: ToolDefinition[] = [listInterfacesTool, createVlanTool];
```
to:
```typescript
export const interfaceTools: ToolDefinition[] = [listInterfacesTool];
```

- [ ] **Step 6: Update `interface-tools.test.ts` — remove create_vlan test block**

In `test/unit/domain/interface-tools.test.ts`, delete the entire `describe("create_vlan - idempotency boolean comparison", ...)` block (lines 50–85). Also remove the import of `{ z }` if it's no longer used, and remove the `listInterfacesInputSchema` const if it was only used by those tests (check if still needed).

- [ ] **Step 7: Run affected tests**

```bash
cd /Users/alikarami/Projects/MikroMCP && npx vitest run test/unit/domain/vlan-tools.test.ts test/unit/domain/interface-tools.test.ts 2>&1 | tail -20
```

Expected: All PASS.

- [ ] **Step 8: Commit**

```bash
cd /Users/alikarami/Projects/MikroMCP && git add src/domain/tools/vlan-tools.ts src/domain/tools/interface-tools.ts test/unit/domain/vlan-tools.test.ts test/unit/domain/interface-tools.test.ts
git commit -m "$(cat <<'EOF'
feat: add manage_vlan tool (add/remove/enable/disable); remove deprecated create_vlan

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
EOF
)"
```

---

## Task 3: DHCP lease improvements (leaseType filter + manage_dhcp_lease)

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

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

Add these test blocks to `test/unit/domain/dhcp-tools.test.ts` (after existing tests):

```typescript
describe("list_dhcp_leases - leaseType filter", () => {
  const leases = [
    { ".id": "*1", address: "192.168.1.10", "mac-address": "AA:BB:CC:DD:EE:01", status: "bound", "address-lists": "", "dynamic": "true" },
    { ".id": "*2", address: "192.168.1.20", "mac-address": "AA:BB:CC:DD:EE:02", status: "bound", "dynamic": "false" },
  ];

  it("filters dynamic leases", async () => {
    const ctx = makeContext(leases);
    const result = await listTool.handler({ routerId: "test-router", leaseType: "dynamic" }, ctx);
    const sc = result.structuredContent as Record<string, unknown>;
    expect((sc.leases as unknown[]).length).toBe(1);
  });

  it("filters static leases", async () => {
    const ctx = makeContext(leases);
    const result = await listTool.handler({ routerId: "test-router", leaseType: "static" }, ctx);
    const sc = result.structuredContent as Record<string, unknown>;
    expect((sc.leases as unknown[]).length).toBe(1);
  });

  it("returns all when leaseType is all", async () => {
    const ctx = makeContext(leases);
    const result = await listTool.handler({ routerId: "test-router", leaseType: "all" }, ctx);
    const sc = result.structuredContent as Record<string, unknown>;
    expect((sc.leases as unknown[]).length).toBe(2);
  });
});

describe("manage_dhcp_lease", () => {
  // manageTool is dhcpTools[1]
  const manageLeaseTool = dhcpTools[1];

  it("tool is named manage_dhcp_lease", () => expect(manageLeaseTool.name).toBe("manage_dhcp_lease"));
  it("is not readOnly", () => expect(manageLeaseTool.annotations.readOnlyHint).toBe(false));
  it("has snapshotPaths", () => expect(manageLeaseTool.snapshotPaths).toContain("ip/dhcp-server/lease"));

  it("make-static converts dynamic lease", async () => {
    const dynamicLease = { ".id": "*1", "mac-address": "AA:BB:CC:DD:EE:01", address: "192.168.1.10", dynamic: "true" };
    const ctx = makeContext([dynamicLease]);
    const result = await manageLeaseTool.handler(
      { routerId: "test-router", action: "make-static", macAddress: "AA:BB:CC:DD:EE:01" },
      ctx,
    );
    const sc = result.structuredContent as Record<string, unknown>;
    expect(sc.action).toBe("made-static");
  });

  it("throws NOT_FOUND when MAC not in lease table", async () => {
    const ctx = makeContext([]);
    await expect(
      manageLeaseTool.handler({ routerId: "test-router", action: "make-static", macAddress: "FF:FF:FF:FF:FF:FF" }, ctx),
    ).rejects.toMatchObject({ category: ErrorCategory.NOT_FOUND });
  });

  it("removes lease by MAC address", async () => {
    const lease = { ".id": "*1", "mac-address": "AA:BB:CC:DD:EE:01", address: "192.168.1.10" };
    const ctx = makeContext([lease]);
    const result = await manageLeaseTool.handler(
      { routerId: "test-router", action: "remove", macAddress: "AA:BB:CC:DD:EE:01" },
      ctx,
    );
    const sc = result.structuredContent as Record<string, unknown>;
    expect(sc.action).toBe("removed");
    expect(ctx.routerClient.remove).toHaveBeenCalledWith("ip/dhcp-server/lease", "*1");
  });

  it("returns not_found on remove when MAC missing", async () => {
    const ctx = makeContext([]);
    const result = await manageLeaseTool.handler(
      { routerId: "test-router", action: "remove", macAddress: "FF:FF:FF:FF:FF:FF" },
      ctx,
    );
    const sc = result.structuredContent as Record<string, unknown>;
    expect(sc.action).toBe("not_found");
  });

  it("dry_run for make-static returns diff without updating", async () => {
    const dynamicLease = { ".id": "*1", "mac-address": "AA:BB:CC:DD:EE:01", address: "192.168.1.10", dynamic: "true" };
    const ctx = makeContext([dynamicLease]);
    const result = await manageLeaseTool.handler(
      { routerId: "test-router", action: "make-static", macAddress: "AA:BB:CC:DD:EE:01", dryRun: true },
      ctx,
    );
    const sc = result.structuredContent as Record<string, unknown>;
    expect(sc.action).toBe("dry_run");
    expect(ctx.routerClient.update).not.toHaveBeenCalled();
  });
});
```

Also ensure imports in the test file include `ErrorCategory` and reference `dhcpTools` correctly (the test file currently only imports `listDhcpLeasesTool` — update to import the full array and reference `dhcpTools[1]`).

- [ ] **Step 2: Run test to verify failures**

```bash
cd /Users/alikarami/Projects/MikroMCP && npx vitest run test/unit/domain/dhcp-tools.test.ts 2>&1 | tail -20
```

Expected: FAIL on leaseType filter + manage_dhcp_lease tests.

- [ ] **Step 3: Update `dhcp-tools.ts`**

Add `leaseType` to the input schema:
```typescript
leaseType: z
  .enum(["dynamic", "static", "all"])
  .default("all")
  .describe("Filter by lease type (dynamic or static)"),
```

Add filter logic in the handler after the existing `macAddress` filter:
```typescript
if (parsed.leaseType !== "all") {
  leases = leases.filter((lease) => {
    const rec = lease as Record<string, unknown>;
    const isDynamic = rec.dynamic === true || rec.dynamic === "true";
    if (parsed.leaseType === "dynamic") return isDynamic;
    return !isDynamic; // static
  });
}
```

Add `manage_dhcp_lease` tool:

```typescript
import { MikroMCPError, ErrorCategory } from "../errors/error-types.js";

const manageDhcpLeaseInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    action: z
      .enum(["make-static", "remove"])
      .describe("Action to perform: make-static converts a dynamic lease to static; remove deletes the lease"),
    macAddress: z
      .string()
      .describe("MAC address of the lease — idempotency key (case-insensitive)"),
    dryRun: z.boolean().default(false).describe("Preview changes without applying"),
  })
  .strict();

const manageDhcpLeaseTool: ToolDefinition = {
  name: "manage_dhcp_lease",
  title: "Manage DHCP Lease",
  description:
    "Convert a dynamic DHCP lease to static (make-static) or remove a lease. Idempotent by MAC address.",
  inputSchema: manageDhcpLeaseInputSchema,
  annotations: {
    readOnlyHint: false,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  snapshotPaths: ["ip/dhcp-server/lease"],
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = manageDhcpLeaseInputSchema.parse(params);
    const upperMac = parsed.macAddress.toUpperCase();

    log.info({ routerId: context.routerId, action: parsed.action, macAddress: upperMac }, "Managing DHCP lease");

    try {
      const allLeases = await context.routerClient.get<RouterOSRecord>("ip/dhcp-server/lease", {
        limit: undefined,
        offset: undefined,
      });
      const existing = (allLeases as Record<string, string>[]).find(
        (l) => (l["mac-address"] ?? "").toUpperCase() === upperMac,
      );

      if (parsed.action === "make-static") {
        if (!existing) {
          throw new MikroMCPError({
            category: ErrorCategory.NOT_FOUND,
            code: "DHCP_LEASE_NOT_FOUND",
            message: `No DHCP lease found for MAC address ${upperMac}.`,
            details: { macAddress: upperMac },
            recoverability: {
              retryable: false,
              suggestedAction: "Verify the MAC address with list_dhcp_leases.",
              alternativeTools: ["list_dhcp_leases"],
            },
          });
        }

        if (parsed.dryRun) {
          return {
            content: `Dry run: Would convert lease for ${upperMac} (${existing.address ?? "unknown"}) to static.`,
            structuredContent: {
              action: "dry_run",
              diff: [{ property: "dynamic", before: "true", after: "false" }],
              lease: existing,
            },
          };
        }

        await context.routerClient.update("ip/dhcp-server/lease", existing[".id"], { dynamic: "false" });
        log.info({ macAddress: upperMac, id: existing[".id"] }, "DHCP lease converted to static");
        return {
          content: `Lease for ${upperMac} (${existing.address ?? "unknown"}) converted to static.`,
          structuredContent: { action: "made-static", macAddress: upperMac, id: existing[".id"] },
        };
      }

      // remove
      if (!existing) {
        return {
          content: `No DHCP lease found for MAC address ${upperMac}. Nothing to remove.`,
          structuredContent: { action: "not_found", macAddress: upperMac },
        };
      }
      if (parsed.dryRun) {
        return {
          content: `Dry run: Would remove lease for ${upperMac}.`,
          structuredContent: { action: "dry_run", diff: [{ property: "mac-address", before: upperMac, after: null }] },
        };
      }
      await context.routerClient.remove("ip/dhcp-server/lease", existing[".id"]);
      log.info({ macAddress: upperMac }, "DHCP lease removed");
      return {
        content: `Removed DHCP lease for ${upperMac}.`,
        structuredContent: { action: "removed", macAddress: upperMac, id: existing[".id"] },
      };
    } catch (err) {
      if (err instanceof MikroMCPError) throw err;
      throw enrichError(err, { routerId: context.routerId, tool: "manage_dhcp_lease" });
    }
  },
};

export const dhcpTools: ToolDefinition[] = [listDhcpLeasesTool, manageDhcpLeaseTool];
```

- [ ] **Step 4: Run tests to verify they pass**

```bash
cd /Users/alikarami/Projects/MikroMCP && npx vitest run test/unit/domain/dhcp-tools.test.ts 2>&1 | tail -20
```

Expected: All PASS.

- [ ] **Step 5: Commit**

```bash
cd /Users/alikarami/Projects/MikroMCP && git add src/domain/tools/dhcp-tools.ts test/unit/domain/dhcp-tools.test.ts
git commit -m "$(cat <<'EOF'
feat: add manage_dhcp_lease tool and leaseType filter to list_dhcp_leases

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
EOF
)"
```

---

## Task 4: DHCP client tools

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

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

Create `test/unit/domain/dhcp-client-tools.test.ts`:

```typescript
import { describe, it, expect, vi } from "vitest";
import { dhcpClientTools } from "../../../src/domain/tools/dhcp-client-tools.js";
import type { ToolContext } from "../../../src/domain/tools/tool-definition.js";
import type { RouterOSRestClient } from "../../../src/adapter/rest-client.js";
import type { RouterConfig } 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 makeRouterConfig(): RouterConfig {
  return {
    id: "test-router",
    host: "192.168.1.1",
    port: 443,
    tls: { enabled: true, rejectUnauthorized: false },
    credentials: { source: "env", envPrefix: "ROUTER_TEST" },
    tags: [],
    rosVersion: "7",
  };
}

function makeContext(records: Record<string, unknown>[] = []): ToolContext {
  return {
    routerId: "test-router",
    correlationId: "test-corr",
    routerConfig: makeRouterConfig(),
    identity: { id: "superadmin-builtin", role: "superadmin" as const, allowedRouters: [], allowedToolPatterns: [] },
    sshClient: { execute: vi.fn().mockResolvedValue("") } as unknown as SshClient,
    ftpClient: { upload: vi.fn().mockResolvedValue(undefined), connect: vi.fn().mockResolvedValue(undefined) } as unknown as FtpClient,
    routerClient: {
      get: vi.fn().mockResolvedValue(records),
      create: vi.fn().mockResolvedValue({ ".id": "*1", interface: "ether1" }),
      update: vi.fn().mockResolvedValue(undefined),
      remove: vi.fn().mockResolvedValue(undefined),
    } as unknown as RouterOSRestClient,
  };
}

const [listTool, manageTool] = dhcpClientTools;

describe("dhcpClientTools", () => {
  describe("metadata", () => {
    it("exports 2 tools", () => expect(dhcpClientTools).toHaveLength(2));
    it("has correct names", () => {
      expect(listTool.name).toBe("list_dhcp_clients");
      expect(manageTool.name).toBe("manage_dhcp_client");
    });
    it("list_dhcp_clients is readOnly", () => expect(listTool.annotations.readOnlyHint).toBe(true));
    it("manage_dhcp_client is not readOnly", () => expect(manageTool.annotations.readOnlyHint).toBe(false));
    it("manage_dhcp_client has snapshotPaths", () => expect(manageTool.snapshotPaths).toContain("ip/dhcp-client"));
  });

  describe("list_dhcp_clients handler", () => {
    const clients = [
      { ".id": "*1", interface: "ether1", status: "bound", address: "192.168.1.100/24", disabled: "false" },
      { ".id": "*2", interface: "ether2", status: "searching", disabled: "false" },
    ];

    it("returns all clients", async () => {
      const ctx = makeContext(clients);
      const result = await listTool.handler({ routerId: "test-router" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect((sc.clients as unknown[]).length).toBe(2);
      expect(sc.total).toBe(2);
    });

    it("filters by interface", async () => {
      const ctx = makeContext(clients);
      const result = await listTool.handler({ routerId: "test-router", interface: "ether1" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect((sc.clients as unknown[]).length).toBe(1);
    });

    it("filters by status", async () => {
      const ctx = makeContext(clients);
      const result = await listTool.handler({ routerId: "test-router", status: "bound" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect((sc.clients as unknown[]).length).toBe(1);
    });
  });

  describe("manage_dhcp_client - add", () => {
    it("adds client when interface not configured", async () => {
      const ctx = makeContext([]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "add", interface: "ether1" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("created");
    });

    it("returns already_exists when same interface exists", async () => {
      const ctx = makeContext([{ ".id": "*1", interface: "ether1", disabled: "false" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "add", interface: "ether1" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("already_exists");
    });

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

  describe("manage_dhcp_client - remove", () => {
    it("removes existing client", async () => {
      const ctx = makeContext([{ ".id": "*1", interface: "ether1" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "remove", interface: "ether1" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("removed");
      expect(ctx.routerClient.remove).toHaveBeenCalledWith("ip/dhcp-client", "*1");
    });

    it("returns not_found when client missing", async () => {
      const ctx = makeContext([]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "remove", interface: "ether99" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("not_found");
    });
  });

  describe("manage_dhcp_client - enable/disable", () => {
    it("disables client", async () => {
      const ctx = makeContext([{ ".id": "*1", interface: "ether1", disabled: "false" }]);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "disable", interface: "ether1" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("disabled");
      expect(ctx.routerClient.update).toHaveBeenCalledWith("ip/dhcp-client", "*1", { disabled: "true" });
    });

    it("throws NOT_FOUND on enable/disable when client missing", async () => {
      const ctx = makeContext([]);
      await expect(
        manageTool.handler({ routerId: "test-router", action: "enable", interface: "ether99" }, ctx),
      ).rejects.toMatchObject({ category: ErrorCategory.NOT_FOUND });
    });
  });
});
```

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

```bash
cd /Users/alikarami/Projects/MikroMCP && npx vitest run test/unit/domain/dhcp-client-tools.test.ts 2>&1 | tail -10
```

Expected: FAIL — module not found.

- [ ] **Step 3: Create `src/domain/tools/dhcp-client-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("dhcp-client-tools");

const listDhcpClientsInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    interface: z.string().optional().describe("Filter by interface name (exact match)"),
    status: z
      .enum(["bound", "searching", "requesting", "init", "all"])
      .default("all")
      .describe("Filter by DHCP client status"),
    limit: z.number().int().min(1).max(500).default(100).describe("Maximum number of clients to return"),
    offset: z.number().int().min(0).default(0).describe("Offset for pagination"),
  })
  .strict();

const listDhcpClientsTool: ToolDefinition = {
  name: "list_dhcp_clients",
  title: "List DHCP Clients",
  description:
    "List DHCP client configurations on a MikroTik router. Shows which interfaces obtain their IP via DHCP, current status, and assigned address.",
  inputSchema: listDhcpClientsInputSchema,
  annotations: {
    readOnlyHint: true,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = listDhcpClientsInputSchema.parse(params);
    log.info({ routerId: context.routerId }, "Listing DHCP clients");
    try {
      const allClients = await context.routerClient.get<RouterOSRecord>("ip/dhcp-client", {
        limit: undefined,
        offset: undefined,
      });

      let filtered = allClients as Record<string, unknown>[];

      if (parsed.interface) {
        filtered = filtered.filter((c) => c.interface === parsed.interface);
      }

      if (parsed.status !== "all") {
        filtered = filtered.filter((c) => c.status === parsed.status);
      }

      const total = filtered.length;
      const clients = filtered.slice(parsed.offset, parsed.offset + parsed.limit);
      const hasMore = parsed.offset + parsed.limit < total;

      const lines: string[] = [
        `DHCP clients on ${context.routerId}: ${total} total, showing ${clients.length} (offset ${parsed.offset})`,
      ];
      for (const c of clients) {
        const rec = c as Record<string, unknown>;
        const iface = rec.interface ?? "unknown";
        const status = rec.status ?? "unknown";
        const address = rec.address ?? rec["active-address"] ?? "-";
        const disabled = rec.disabled === true || rec.disabled === "true";
        lines.push(`  ${iface}  [${status}]  ${address}${disabled ? "  DISABLED" : ""}`);
      }

      return {
        content: lines.join("\n"),
        structuredContent: {
          routerId: context.routerId,
          clients,
          total,
          hasMore,
          offset: parsed.offset,
          limit: parsed.limit,
        },
      };
    } catch (err) {
      throw enrichError(err, { routerId: context.routerId, tool: "list_dhcp_clients" });
    }
  },
};

const manageDhcpClientInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    action: z.enum(["add", "remove", "enable", "disable"]).describe("Action to perform"),
    interface: z.string().describe("Interface name — idempotency key (e.g. ether1, ether2)"),
    usePeerDns: z.boolean().default(true).describe("Use DNS servers provided by DHCP server (add only)"),
    usePeerNtp: z.boolean().default(false).describe("Use NTP servers provided by DHCP server (add only)"),
    addDefaultRoute: z.boolean().default(true).describe("Add default route from DHCP (add only)"),
    comment: z.string().max(255).optional().describe("Optional comment (add only)"),
    dryRun: z.boolean().default(false).describe("Preview changes without applying"),
  })
  .strict();

const manageDhcpClientTool: ToolDefinition = {
  name: "manage_dhcp_client",
  title: "Manage DHCP Client",
  description:
    "Add, remove, enable, or disable a DHCP client on an interface. Idempotent by interface name: add returns already_exists if a DHCP client is already configured on the same interface.",
  inputSchema: manageDhcpClientInputSchema,
  annotations: {
    readOnlyHint: false,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  snapshotPaths: ["ip/dhcp-client"],
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = manageDhcpClientInputSchema.parse(params);
    log.info({ routerId: context.routerId, action: parsed.action, interface: parsed.interface }, "Managing DHCP client");

    try {
      const allClients = await context.routerClient.get<RouterOSRecord>("ip/dhcp-client", {
        limit: undefined,
        offset: undefined,
      });
      const existing = (allClients as Record<string, string>[]).find(
        (c) => c.interface === parsed.interface,
      );

      if (parsed.action === "add") {
        if (existing) {
          return {
            content: `DHCP client already configured on "${parsed.interface}". No changes made.`,
            structuredContent: { action: "already_exists", client: existing },
          };
        }

        if (parsed.dryRun) {
          const diff = [
            { property: "interface", before: null, after: parsed.interface },
            { property: "use-peer-dns", before: null, after: parsed.usePeerDns ? "yes" : "no" },
            { property: "use-peer-ntp", before: null, after: parsed.usePeerNtp ? "yes" : "no" },
            { property: "add-default-route", before: null, after: parsed.addDefaultRoute ? "yes" : "no" },
          ];
          return {
            content: `Dry run: Would add DHCP client on "${parsed.interface}".`,
            structuredContent: { action: "dry_run", diff },
          };
        }

        const body: Record<string, string> = {
          interface: parsed.interface,
          "use-peer-dns": parsed.usePeerDns ? "yes" : "no",
          "use-peer-ntp": parsed.usePeerNtp ? "yes" : "no",
          "add-default-route": parsed.addDefaultRoute ? "yes" : "no",
        };
        if (parsed.comment) body.comment = parsed.comment;

        const created = await context.routerClient.create("ip/dhcp-client", body);
        log.info({ interface: parsed.interface, id: created[".id"] }, "DHCP client added");
        return {
          content: `Added DHCP client on "${parsed.interface}".`,
          structuredContent: { action: "created", client: created },
        };
      }

      if (parsed.action === "remove") {
        if (!existing) {
          return {
            content: `No DHCP client found on "${parsed.interface}". Nothing to remove.`,
            structuredContent: { action: "not_found", interface: parsed.interface },
          };
        }
        if (parsed.dryRun) {
          return {
            content: `Dry run: Would remove DHCP client on "${parsed.interface}".`,
            structuredContent: { action: "dry_run", diff: [{ property: "interface", before: parsed.interface, after: null }] },
          };
        }
        await context.routerClient.remove("ip/dhcp-client", existing[".id"]);
        log.info({ interface: parsed.interface }, "DHCP client removed");
        return {
          content: `Removed DHCP client on "${parsed.interface}".`,
          structuredContent: { action: "removed", interface: parsed.interface, id: existing[".id"] },
        };
      }

      // enable / disable
      if (!existing) {
        throw new MikroMCPError({
          category: ErrorCategory.NOT_FOUND,
          code: "DHCP_CLIENT_NOT_FOUND",
          message: `No DHCP client found on interface "${parsed.interface}".`,
          details: { interface: parsed.interface },
          recoverability: {
            retryable: false,
            suggestedAction: "Verify with list_dhcp_clients.",
            alternativeTools: ["list_dhcp_clients"],
          },
        });
      }

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

      const disabledValue = parsed.action === "disable" ? "true" : "false";
      await context.routerClient.update("ip/dhcp-client", existing[".id"], { disabled: disabledValue });
      const resultAction = parsed.action === "disable" ? "disabled" : "enabled";
      log.info({ interface: parsed.interface, action: resultAction }, "DHCP client updated");
      return {
        content: `DHCP client on "${parsed.interface}" ${resultAction}.`,
        structuredContent: { action: resultAction, interface: parsed.interface, id: existing[".id"] },
      };
    } catch (err) {
      if (err instanceof MikroMCPError) throw err;
      throw enrichError(err, { routerId: context.routerId, tool: "manage_dhcp_client" });
    }
  },
};

export const dhcpClientTools: ToolDefinition[] = [listDhcpClientsTool, manageDhcpClientTool];
```

- [ ] **Step 4: Run tests to verify they pass**

```bash
cd /Users/alikarami/Projects/MikroMCP && npx vitest run test/unit/domain/dhcp-client-tools.test.ts 2>&1 | tail -20
```

Expected: All PASS.

- [ ] **Step 5: Commit**

```bash
cd /Users/alikarami/Projects/MikroMCP && git add src/domain/tools/dhcp-client-tools.ts test/unit/domain/dhcp-client-tools.test.ts
git commit -m "$(cat <<'EOF'
feat: add list_dhcp_clients and manage_dhcp_client tools

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
EOF
)"
```

---

## Task 5: IP service tools

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

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

Create `test/unit/domain/ip-service-tools.test.ts`:

```typescript
import { describe, it, expect, vi } from "vitest";
import { ipServiceTools } from "../../../src/domain/tools/ip-service-tools.js";
import type { ToolContext } from "../../../src/domain/tools/tool-definition.js";
import type { RouterOSRestClient } from "../../../src/adapter/rest-client.js";
import type { RouterConfig } 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 makeRouterConfig(): RouterConfig {
  return {
    id: "test-router",
    host: "192.168.1.1",
    port: 443,
    tls: { enabled: true, rejectUnauthorized: false },
    credentials: { source: "env", envPrefix: "ROUTER_TEST" },
    tags: [],
    rosVersion: "7",
  };
}

function makeContext(records: Record<string, unknown>[] = []): ToolContext {
  return {
    routerId: "test-router",
    correlationId: "test-corr",
    routerConfig: makeRouterConfig(),
    identity: { id: "superadmin-builtin", role: "superadmin" as const, allowedRouters: [], allowedToolPatterns: [] },
    sshClient: { execute: vi.fn().mockResolvedValue("") } as unknown as SshClient,
    ftpClient: { upload: vi.fn().mockResolvedValue(undefined), connect: vi.fn().mockResolvedValue(undefined) } as unknown as FtpClient,
    routerClient: {
      get: vi.fn().mockResolvedValue(records),
      update: vi.fn().mockResolvedValue(undefined),
    } as unknown as RouterOSRestClient,
  };
}

const [listTool, manageTool] = ipServiceTools;

const sampleServices = [
  { ".id": "*1", name: "api", port: "8728", disabled: "false" },
  { ".id": "*2", name: "api-ssl", port: "8729", disabled: "false" },
  { ".id": "*3", name: "ssh", port: "22", disabled: "false" },
  { ".id": "*4", name: "telnet", port: "23", disabled: "true" },
  { ".id": "*5", name: "www", port: "80", disabled: "false" },
  { ".id": "*6", name: "www-ssl", port: "443", disabled: "false" },
  { ".id": "*7", name: "winbox", port: "8291", disabled: "false" },
];

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

  describe("list_ip_services handler", () => {
    it("returns all services", async () => {
      const ctx = makeContext(sampleServices);
      const result = await listTool.handler({ routerId: "test-router" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect((sc.services as unknown[]).length).toBe(7);
    });

    it("filters by name", async () => {
      const ctx = makeContext(sampleServices);
      const result = await listTool.handler({ routerId: "test-router", name: "ssh" }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      expect((sc.services as unknown[]).length).toBe(1);
    });

    it("filters disabled services", async () => {
      const ctx = makeContext(sampleServices);
      const result = await listTool.handler({ routerId: "test-router", enabled: true }, ctx);
      const sc = result.structuredContent as Record<string, unknown>;
      // 6 enabled (telnet disabled)
      expect((sc.services as unknown[]).length).toBe(6);
    });
  });

  describe("manage_ip_service handler", () => {
    it("disables a service", async () => {
      const ctx = makeContext(sampleServices);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "disable", name: "telnet" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("disabled");
      expect(ctx.routerClient.update).toHaveBeenCalledWith("ip/service", "*4", { disabled: "true" });
    });

    it("enables a service", async () => {
      const ctx = makeContext(sampleServices);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "enable", name: "telnet" },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("enabled");
      expect(ctx.routerClient.update).toHaveBeenCalledWith("ip/service", "*4", { disabled: "false" });
    });

    it("dry_run returns diff without updating", async () => {
      const ctx = makeContext(sampleServices);
      const result = await manageTool.handler(
        { routerId: "test-router", action: "disable", name: "telnet", dryRun: true },
        ctx,
      );
      const sc = result.structuredContent as Record<string, unknown>;
      expect(sc.action).toBe("dry_run");
      expect(ctx.routerClient.update).not.toHaveBeenCalled();
    });

    it("throws NOT_FOUND for unknown service name", async () => {
      const ctx = makeContext(sampleServices);
      await expect(
        manageTool.handler({ routerId: "test-router", action: "enable", name: "nonexistent" }, ctx),
      ).rejects.toMatchObject({ category: ErrorCategory.NOT_FOUND });
    });
  });
});
```

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

```bash
cd /Users/alikarami/Projects/MikroMCP && npx vitest run test/unit/domain/ip-service-tools.test.ts 2>&1 | tail -10
```

Expected: FAIL — module not found.

- [ ] **Step 3: Create `src/domain/tools/ip-service-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("ip-service-tools");

const SERVICE_NAMES = ["api", "api-ssl", "ssh", "telnet", "www", "www-ssl", "winbox", "ftp"] as const;

const listIpServicesInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    name: z
      .enum(SERVICE_NAMES)
      .optional()
      .describe("Filter by service name"),
    enabled: z
      .boolean()
      .optional()
      .describe("When true, return only enabled services; when false, only disabled"),
  })
  .strict();

const listIpServicesTool: ToolDefinition = {
  name: "list_ip_services",
  title: "List IP Services",
  description:
    "List IP services on a MikroTik router (api, api-ssl, ssh, telnet, www, www-ssl, winbox, ftp) with their port numbers and enabled/disabled status.",
  inputSchema: listIpServicesInputSchema,
  annotations: {
    readOnlyHint: true,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = listIpServicesInputSchema.parse(params);
    log.info({ routerId: context.routerId }, "Listing IP services");
    try {
      const allServices = await context.routerClient.get<RouterOSRecord>("ip/service", {
        limit: undefined,
        offset: undefined,
      });

      let filtered = allServices as Record<string, unknown>[];

      if (parsed.name) {
        filtered = filtered.filter((s) => s.name === parsed.name);
      }

      if (parsed.enabled !== undefined) {
        filtered = filtered.filter((s) => {
          const isDisabled = s.disabled === true || s.disabled === "true";
          return parsed.enabled ? !isDisabled : isDisabled;
        });
      }

      const lines: string[] = [`IP services on ${context.routerId}: ${filtered.length} returned`];
      for (const s of filtered) {
        const rec = s as Record<string, unknown>;
        const isDisabled = rec.disabled === true || rec.disabled === "true";
        lines.push(`  ${rec.name}  port:${rec.port ?? "-"}  [${isDisabled ? "DISABLED" : "enabled"}]`);
      }

      return {
        content: lines.join("\n"),
        structuredContent: {
          routerId: context.routerId,
          services: filtered,
          total: filtered.length,
        },
      };
    } catch (err) {
      throw enrichError(err, { routerId: context.routerId, tool: "list_ip_services" });
    }
  },
};

const manageIpServiceInputSchema = z
  .object({
    routerId: z.string().describe("Target router identifier from the router registry"),
    action: z
      .enum(["enable", "disable"])
      .describe("Action to perform — only enable/disable to prevent accidental lockout from changing ports"),
    name: z.enum(SERVICE_NAMES).describe("Service name to manage"),
    dryRun: z.boolean().default(false).describe("Preview changes without applying"),
  })
  .strict();

const manageIpServiceTool: ToolDefinition = {
  name: "manage_ip_service",
  title: "Manage IP Service",
  description:
    "Enable or disable a RouterOS IP service (api, api-ssl, ssh, telnet, www, www-ssl, winbox, ftp). Port number changes are intentionally not supported to prevent accidental lockout.",
  inputSchema: manageIpServiceInputSchema,
  annotations: {
    readOnlyHint: false,
    destructiveHint: false,
    idempotentHint: true,
    openWorldHint: false,
  },
  async handler(params: Record<string, unknown>, context: ToolContext): Promise<ToolResult> {
    const parsed = manageIpServiceInputSchema.parse(params);
    log.info({ routerId: context.routerId, action: parsed.action, name: parsed.name }, "Managing IP service");

    try {
      const allServices = await context.routerClient.get<RouterOSRecord>("ip/service", {
        limit: undefined,
        offset: undefined,
      });
      const existing = (allServices as Record<string, string>[]).find((s) => s.name === parsed.name);

      if (!existing) {
        throw new MikroMCPError({
          category: ErrorCategory.NOT_FOUND,
          code: "IP_SERVICE_NOT_FOUND",
          message: `IP service "${parsed.name}" not found on router.`,
          details: { name: parsed.name },
          recoverability: {
            retryable: false,
            suggestedAction: "Verify service name with list_ip_services.",
            alternativeTools: ["list_ip_services"],
          },
        });
      }

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

      const disabledValue = parsed.action === "disable" ? "true" : "false";
      await context.routerClient.update("ip/service", existing[".id"], { disabled: disabledValue });
      const resultAction = parsed.action === "disable" ? "disabled" : "enabled";
      log.info({ name: parsed.name, action: resultAction }, "IP service updated");
      return {
        content: `IP service "${parsed.name}" ${resultAction}.`,
        structuredContent: { action: resultAction, name: parsed.name, id: existing[".id"] },
      };
    } catch (err) {
      if (err instanceof MikroMCPError) throw err;
      throw enrichError(err, { routerId: context.routerId, tool: "manage_ip_service" });
    }
  },
};

export const ipServiceTools: ToolDefinition[] = [listIpServicesTool, manageIpServiceTool];
```

- [ ] **Step 4: Run tests to verify they pass**

```bash
cd /Users/alikarami/Projects/MikroMCP && npx vitest run test/unit/domain/ip-service-tools.test.ts 2>&1 | tail -20
```

Expected: All PASS.

- [ ] **Step 5: Commit**

```bash
cd /Users/alikarami/Projects/MikroMCP && git add src/domain/tools/ip-service-tools.ts test/unit/domain/ip-service-tools.test.ts
git commit -m "$(cat <<'EOF'
feat: add list_ip_services and manage_ip_service tools (enable/disable only)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
EOF
)"
```

---

## Task 6: Wire index.ts + full test suite

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

- [ ] **Step 1: Update `src/domain/tools/index.ts`**

```typescript
import type { ToolDefinition } from "./tool-definition.js";
import { systemTools } from "./system-tools.js";
import { interfaceTools } from "./interface-tools.js";
import { vlanTools } from "./vlan-tools.js";
import { ipTools } from "./ip-tools.js";
import { dhcpTools } from "./dhcp-tools.js";
import { dhcpClientTools } from "./dhcp-client-tools.js";
import { routeTools } from "./route-tools.js";
import { firewallTools } from "./firewall-tools.js";
import { diagnosticTools } from "./diagnostic-tools.js";
import { systemOpsTools } from "./system-ops-tools.js";
import { bridgeTools } from "./bridge-tools.js";
import { wifiTools } from "./wifi-tools.js";
import { wireguardTools } from "./wireguard-tools.js";
import { dnsTools } from "./dns-tools.js";
import { mangleTools } from "./mangle-tools.js";
import { addressListTools } from "./address-list-tools.js";
import { policyRoutingTools } from "./policy-routing-tools.js";
import { routingProtocolTools } from "./routing-protocol-tools.js";
import { scriptsTools } from "./scripts-tools.js";
import { schedulerTools } from "./scheduler-tools.js";
import { packagesTools } from "./packages-tools.js";
import { filesTools } from "./files-tools.js";
import { containerTools } from "./container-tools.js";
import { createChangeManagementTools } from "./change-management-tools.js";
import { ipsecTools } from "./ipsec-tools.js";
import { certificateTools } from "./certificate-tools.js";
import { userTools } from "./user-tools.js";
import { dhcpServerTools } from "./dhcp-server-tools.js";
import { ipPoolTools } from "./ip-pool-tools.js";
import { queueTools } from "./queue-tools.js";
import { vrrpTools } from "./vrrp-tools.js";
import { networkServicesTools } from "./network-services-tools.js";
import { ipServiceTools } from "./ip-service-tools.js";
import { createFleetTools } from "./fleet-tools.js";

const baseTools: ToolDefinition[] = [
  ...systemTools,
  ...interfaceTools,
  ...vlanTools,
  ...ipTools,
  ...dhcpTools,
  ...dhcpClientTools,
  ...routeTools,
  ...firewallTools,
  ...diagnosticTools,
  ...systemOpsTools,
  ...bridgeTools,
  ...wifiTools,
  ...wireguardTools,
  ...dnsTools,
  ...mangleTools,
  ...addressListTools,
  ...policyRoutingTools,
  ...routingProtocolTools,
  ...scriptsTools,
  ...schedulerTools,
  ...packagesTools,
  ...filesTools,
  ...containerTools,
  ...ipsecTools,
  ...certificateTools,
  ...userTools,
  ...dhcpServerTools,
  ...ipPoolTools,
  ...queueTools,
  ...vrrpTools,
  ...networkServicesTools,
  ...ipServiceTools,
];

export const allTools: ToolDefinition[] = [
  ...baseTools,
  ...createChangeManagementTools(baseTools),
  ...createFleetTools(baseTools),
];

export type {
  ToolDefinition,
  ToolContext,
  ToolResult,
  ToolAnnotations,
} from "./tool-definition.js";
```

- [ ] **Step 2: Run full test suite + typecheck**

```bash
cd /Users/alikarami/Projects/MikroMCP && npm test 2>&1 | tail -30
```

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

- [ ] **Step 3: Fix any failures**

If tests fail, read the error output and fix the specific issue before continuing.

- [ ] **Step 4: Commit**

```bash
cd /Users/alikarami/Projects/MikroMCP && git add src/domain/tools/index.ts
git commit -m "$(cat <<'EOF'
feat: wire v1.2 tools into index (vlan, ip-pool, dhcp-client, ip-service)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
EOF
)"
```

---

## Task 7: Docs + CHANGELOG

**Files:**
- Modify: `CHANGELOG.md`
- Modify: `README.md` (tool count)
- Modify: `ROADMAP.md` (add v1.2 section)

- [ ] **Step 1: Add [Unreleased] entries to `CHANGELOG.md`**

Under the `[Unreleased]` section add:

```markdown
### Added
- `manage_vlan` — add, remove, enable, or disable VLAN interfaces (idempotent by name; replaces `create_vlan`)
- `list_ip_pools`, `manage_ip_pool` — IP address pool tools (extracted from DHCP server tools; renamed from `list_dhcp_pools`/`manage_dhcp_pool`)
- `manage_dhcp_lease` — convert dynamic leases to static or remove leases (idempotent by MAC address)
- `list_dhcp_clients`, `manage_dhcp_client` — DHCP client configuration on router interfaces
- `list_ip_services`, `manage_ip_service` — view and enable/disable RouterOS IP services (api, ssh, telnet, www, etc.)

### Changed
- `list_dhcp_leases` gains `leaseType` filter (`dynamic`, `static`, `all`) to distinguish lease types
- `list_dhcp_servers` gains `offset` parameter for consistent pagination across all list tools
- `list_ip_pools` / `manage_ip_pool` now use semantically accurate names (were `list_dhcp_pools` / `manage_dhcp_pool`)

### Removed
- `create_vlan` — replaced by `manage_vlan` which covers add, remove, enable, and disable
- `list_dhcp_pools`, `manage_dhcp_pool` — renamed to `list_ip_pools`, `manage_ip_pool`
```

- [ ] **Step 2: Update tool count in `README.md`**

Find the tool count reference in README.md (search for a number like "70+ tools" or similar), and update it to reflect the new tool count. The change adds 5 tools (`manage_vlan`, `list_ip_pools`, `manage_ip_pool`, `manage_dhcp_lease`, `list_dhcp_clients`, `manage_dhcp_client`, `list_ip_services`, `manage_ip_service` = 8 new) and removes 3 (`create_vlan`, `list_dhcp_pools`, `manage_dhcp_pool`) = net +5.

```bash
grep -n "tools" /Users/alikarami/Projects/MikroMCP/README.md | grep -i "count\|number\|[0-9]" | head -10
```

- [ ] **Step 3: Add v1.2 section to ROADMAP.md**

After the `## ✅ v1.1` section add:

```markdown
## 🔜 v1.2 — DHCP & Interface Completeness

**Goal:** Fill the remaining DHCP gaps, unify VLAN management under a single tool, and expose IP services for management.

- **Breaking renames:** `create_vlan` → `manage_vlan`; `list_dhcp_pools` → `list_ip_pools`; `manage_dhcp_pool` → `manage_ip_pool`
- **`manage_vlan`** — add, remove, enable, or disable VLAN interfaces (full lifecycle, idempotent by name)
- **`list_ip_pools`, `manage_ip_pool`** — semantically accurate rename; pools serve any purpose, not only DHCP
- **DHCP improvements:** `manage_dhcp_lease` (MAC-keyed; make-static action); `leaseType` filter on `list_dhcp_leases`; `offset` on `list_dhcp_servers`
- **DHCP clients:** `list_dhcp_clients`, `manage_dhcp_client` — configure which interfaces obtain IP via DHCP
- **IP services:** `list_ip_services`, `manage_ip_service` — enable/disable api, api-ssl, ssh, telnet, www, www-ssl, winbox, ftp (port changes excluded to prevent lockout)
```

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

```bash
cd /Users/alikarami/Projects/MikroMCP && git add CHANGELOG.md README.md ROADMAP.md
git commit -m "$(cat <<'EOF'
docs: update CHANGELOG, README, and ROADMAP for v1.2

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
EOF
)"
```

---

## Self-Review

**Spec coverage check:**
- ✅ `manage_vlan` (rename + add remove/enable/disable) — Task 2
- ✅ `list_ip_pools`, `manage_ip_pool` (rename from dhcp-server-tools) — Task 1
- ✅ `manage_dhcp_lease` (MAC key, make-static) — Task 3
- ✅ `leaseType` filter on `list_dhcp_leases` — Task 3
- ✅ `offset` on `list_dhcp_servers` — Task 1
- ✅ `list_dhcp_clients`, `manage_dhcp_client` — Task 4
- ✅ `list_ip_services`, `manage_ip_service` (enable/disable only) — Task 5
- ✅ Wire index.ts — Task 6
- ✅ Docs + CHANGELOG — Task 7

**Placeholder scan:** None — all steps have complete code.

**Type consistency:** All handlers use `Record<string, unknown>` context pattern; all tests use the same `makeContext` function with `appConfig` omitted (cast via `as unknown as ToolContext`). Boolean fields check both `=== true` and `=== "true"`.
