Merge branch 'fix/demo-monitoring-targets' into 'main'

fix(mon): configure demo monitoring target during local-install --demo

Closes #151

See merge request postgres-ai/postgresai!223

Author: Sarumyan

.gitignore

@@ -78,3 +78,9 @@ volumes/
 .pgwatch-config
 
 .env.hypothesis/
+
+# Runtime instances config (created from instances.demo.yml or by CLI)
+instances.yml
+
+# Build artifact: copied from repo root into cli/ during build
+cli/instances.demo.yml

cli/.npmignore

@@ -0,0 +1,7 @@
+package-lock.json
+node_modules/
+coverage/
+
+# Auto-generated at build time (do not commit)
+lib/metrics-embedded.ts
+lib/checkup-dictionary-embedded.ts

cli/bin/postgres-ai.ts

@@ -7,6 +7,7 @@ import * as yaml from "js-yaml";
 import * as fs from "fs";
 import * as path from "path";
 import * as os from "os";
+import { fileURLToPath } from "url";
 import * as crypto from "node:crypto";
 import { Client } from "pg";
 import { startMcpServer } from "../lib/mcp-server";
@@ -503,7 +504,11 @@ async function ensureDefaultMonitoringProject(): Promise<PathResolution> {
     }
   }
 
-  // Ensure instances.yml exists as a FILE (avoid Docker creating a directory)
+  // Ensure instances.yml exists as a FILE (avoid Docker creating a directory).
+  // Docker bind-mounts create missing paths as directories; replace if so.
+  if (fs.existsSync(instancesFile) && fs.lstatSync(instancesFile).isDirectory()) {
+    fs.rmSync(instancesFile, { recursive: true, force: true });
+  }
   if (!fs.existsSync(instancesFile)) {
     const header =
       "# PostgreSQL instances to monitor\n" +
@@ -2302,7 +2307,7 @@ mon
     console.log("This will install, configure, and start the monitoring system\n");
 
     // Ensure we have a project directory with docker-compose.yml even if running from elsewhere
-    const { projectDir } = await resolveOrInitPaths();
+    const { projectDir, instancesFile: instancesPath } = await resolveOrInitPaths();
     console.log(`Project directory: ${projectDir}\n`);
 
     // Save project name to .pgwatch-config if provided (used by reporter container)
@@ -2525,7 +2530,38 @@ mon
         }
       }
     } else {
-      console.log("Step 2: Demo mode enabled - using included demo PostgreSQL database\n");
+      // Demo mode: configure instances.yml from the bundled demo template.
+      //
+      // Side effects:
+      //   - Writes instancesPath (instances.yml next to docker-compose.yml)
+      //   - If Docker previously bind-mounted instances.yml as a directory, removes it first.
+      //
+      // Failure modes:
+      //   - Exits with code 1 if instances.demo.yml is not found in any candidate path.
+      //     This is fatal because starting without a target produces empty dashboards that
+      //     look like a bug rather than a misconfiguration.
+      //
+      // Template search order (import.meta.url is resolved at runtime, not baked in at build):
+      //   1. npm layout:  dist/bin/../../instances.demo.yml  →  package-root/instances.demo.yml
+      //   2. dev layout:  cli/bin/../../../instances.demo.yml →  repo-root/instances.demo.yml
+      console.log("Step 2: Demo mode enabled - using included demo PostgreSQL database");
+      const currentDir = path.dirname(fileURLToPath(import.meta.url));
+      const demoCandidates = [
+        path.resolve(currentDir, "..", "..", "instances.demo.yml"),        // npm: dist/bin -> package root
+        path.resolve(currentDir, "..", "..", "..", "instances.demo.yml"),  // dev: cli/bin -> repo root
+      ];
+      const demoSrc = demoCandidates.find(p => fs.existsSync(p));
+      if (demoSrc) {
+        // Remove directory artifact left by Docker bind-mounts before copying
+        if (fs.existsSync(instancesPath) && fs.lstatSync(instancesPath).isDirectory()) {
+          fs.rmSync(instancesPath, { recursive: true, force: true });
+        }
+        fs.copyFileSync(demoSrc, instancesPath);
+        console.log("✓ Demo monitoring target configured\n");
+      } else {
+        console.error(`Error: instances.demo.yml not found — cannot configure demo target.\nSearched: ${demoCandidates.join(", ")}\n`);
+        process.exit(1);
+      }
     }
 
     // Step 3: Update configuration
@@ -2880,7 +2916,7 @@ mon
     console.log(`Project Directory: ${projectDir}`);
     console.log(`Docker Compose File: ${composeFile}`);
     console.log(`Instances File: ${instancesFile}`);
-    if (fs.existsSync(instancesFile)) {
+    if (fs.existsSync(instancesFile) && !fs.lstatSync(instancesFile).isDirectory()) {
       console.log("\nInstances configuration:\n");
       const text = fs.readFileSync(instancesFile, "utf8");
       process.stdout.write(text);
@@ -3096,7 +3132,7 @@ targets
   .description("list monitoring target databases")
   .action(async () => {
     const { instancesFile: instancesPath, projectDir } = await resolveOrInitPaths();
-    if (!fs.existsSync(instancesPath)) {
+    if (!fs.existsSync(instancesPath) || fs.lstatSync(instancesPath).isDirectory()) {
       console.error(`instances.yml not found in ${projectDir}`);
       process.exitCode = 1;
       return;
@@ -3162,7 +3198,7 @@ targets
 
     // Check if instance already exists
     try {
-      if (fs.existsSync(file)) {
+      if (fs.existsSync(file) && !fs.lstatSync(file).isDirectory()) {
         const content = fs.readFileSync(file, "utf8");
         const instances = yaml.load(content) as Instance[] | null || [];
         if (Array.isArray(instances)) {
@@ -3176,15 +3212,19 @@ targets
       }
     } catch (err) {
       // If YAML parsing fails, fall back to simple check
-      const content = fs.existsSync(file) ? fs.readFileSync(file, "utf8") : "";
+      const isFile = fs.existsSync(file) && !fs.lstatSync(file).isDirectory();
+      const content = isFile ? fs.readFileSync(file, "utf8") : "";
       if (new RegExp(`^- name: ${instanceName}$`, "m").test(content)) {
         console.error(`Monitoring target '${instanceName}' already exists`);
         process.exitCode = 1;
         return;
       }
     }
 
-    // Add new instance
+    // Add new instance — if instances.yml is a directory (Docker artifact), replace it with a file
+    if (fs.existsSync(file) && fs.lstatSync(file).isDirectory()) {
+      fs.rmSync(file, { recursive: true, force: true });
+    }
     const body = `- name: ${instanceName}\n  conn_str: ${connStr}\n  preset_metrics: full\n  custom_metrics:\n  is_enabled: true\n  group: default\n  custom_tags:\n    env: production\n    cluster: default\n    node_name: ${instanceName}\n    sink_type: ~sink_type~\n`;
     const content = fs.existsSync(file) ? fs.readFileSync(file, "utf8") : "";
     fs.appendFileSync(file, (content && !/\n$/.test(content) ? "\n" : "") + body, "utf8");
@@ -3195,7 +3235,7 @@ targets
   .description("remove monitoring target database")
   .action(async (name: string) => {
     const { instancesFile: file } = await resolveOrInitPaths();
-    if (!fs.existsSync(file)) {
+    if (!fs.existsSync(file) || fs.lstatSync(file).isDirectory()) {
       console.error("instances.yml not found");
       process.exitCode = 1;
       return;
@@ -3232,7 +3272,7 @@ targets
   .description("test monitoring target database connectivity")
   .action(async (name: string) => {
     const { instancesFile: instancesPath } = await resolveOrInitPaths();
-    if (!fs.existsSync(instancesPath)) {
+    if (!fs.existsSync(instancesPath) || fs.lstatSync(instancesPath).isDirectory()) {
       console.error("instances.yml not found");
       process.exitCode = 1;
       return;

cli/package.json

@@ -28,7 +28,7 @@
     "embed-metrics": "bun run scripts/embed-metrics.ts",
     "embed-checkup-dictionary": "bun run scripts/embed-checkup-dictionary.ts",
     "embed-all": "bun run embed-metrics && bun run embed-checkup-dictionary",
-    "build": "bun run embed-all && bun build ./bin/postgres-ai.ts --outdir ./dist/bin --target node && node -e \"const fs=require('fs');const f='./dist/bin/postgres-ai.js';fs.writeFileSync(f,fs.readFileSync(f,'utf8').replace('#!/usr/bin/env bun','#!/usr/bin/env node'))\" && cp -r ./sql ./dist/sql",
+    "build": "bun run embed-all && bun build ./bin/postgres-ai.ts --outdir ./dist/bin --target node && node -e \"const fs=require('fs');const f='./dist/bin/postgres-ai.js';fs.writeFileSync(f,fs.readFileSync(f,'utf8').replace('#!/usr/bin/env bun','#!/usr/bin/env node'))\" && cp -r ./sql ./dist/sql && cp ../instances.demo.yml ./instances.demo.yml",
     "prepublishOnly": "npm run build",
     "start": "bun ./bin/postgres-ai.ts --help",
     "start:node": "node ./dist/bin/postgres-ai.js --help",

cli/test/init.test.ts

@@ -1,5 +1,5 @@
 import { describe, test, expect, beforeAll, afterAll } from "bun:test";
-import { resolve } from "path";
+import path, { resolve } from "path";
 import * as fs from "fs";
 import * as os from "os";
 
@@ -1070,6 +1070,69 @@ describe("CLI commands", () => {
     expect(r.stderr).toMatch(/Reports will be generated locally only/);
   });
 
+  test("cli: mon local-install --demo configures demo monitoring target", () => {
+    // --demo should copy instances.demo.yml to instances.yml and print confirmation.
+    // The command will fail later (no Docker), but we verify the demo target step succeeded.
+    // resolvePaths() walks cwd() up to find docker-compose.yml, so instances.yml
+    // is written next to docker-compose.yml in the repo root.
+    const repoRoot = resolve(import.meta.dir, "..", "..");
+    const instancesPath = path.join(repoRoot, "instances.yml");
+    // Remove instances.yml if it exists — use rmSync to handle both files and
+    // directories (the EISDIR test may have left a directory here if it failed).
+    if (fs.existsSync(instancesPath)) fs.rmSync(instancesPath, { recursive: true, force: true });
+    try {
+      const r = runCli(["mon", "local-install", "--demo"]);
+      expect(r.stdout).toMatch(/Demo mode enabled/);
+      expect(r.stdout).toMatch(/Demo monitoring target configured/);
+      // Verify instances.yml was actually written with the demo target
+      expect(fs.existsSync(instancesPath)).toBe(true);
+      const content = fs.readFileSync(instancesPath, "utf8");
+      expect(content).toContain("name: target_database");
+      expect(content).toContain("conn_str: postgresql://monitor:monitor_pass@target-db:5432/target_database");
+    } finally {
+      // Clean up — instances.yml is gitignored so safe to remove
+      if (fs.existsSync(instancesPath)) fs.rmSync(instancesPath, { recursive: true, force: true });
+    }
+  });
+
+  test("cli: mon local-install --demo exits with code 1 when instances.demo.yml is missing", () => {
+    // Regression: if instances.demo.yml cannot be found in any candidate path, the CLI
+    // must exit with a non-zero code and a descriptive error (not silently create empty dashboards).
+    const repoRoot = resolve(import.meta.dir, "..", "..");
+    const demoFile = path.join(repoRoot, "instances.demo.yml");
+    const tempBackup = path.join(os.tmpdir(), `instances.demo.yml.test-backup-${Date.now()}`);
+    // Temporarily move instances.demo.yml so neither candidate path resolves
+    fs.copyFileSync(demoFile, tempBackup);
+    fs.unlinkSync(demoFile);
+    try {
+      const r = runCli(["mon", "local-install", "--demo"]);
+      expect(r.status).not.toBe(0);
+      expect(r.stderr).toContain("instances.demo.yml not found");
+    } finally {
+      // Restore the file — critical to do before any assertion can throw
+      if (!fs.existsSync(demoFile)) fs.copyFileSync(tempBackup, demoFile);
+      fs.rmSync(tempBackup, { force: true });
+    }
+  });
+
+  test("cli: mon local-install --demo with EISDIR recovers instances.yml", () => {
+    // Docker bind-mounts create missing paths as directories; the CLI must handle this.
+    const repoRoot = resolve(import.meta.dir, "..", "..");
+    const instancesPath = path.join(repoRoot, "instances.yml");
+    // Create instances.yml as a directory (simulating Docker artifact)
+    if (fs.existsSync(instancesPath)) fs.rmSync(instancesPath, { recursive: true, force: true });
+    fs.mkdirSync(instancesPath);
+    try {
+      const r = runCli(["mon", "local-install", "--demo"]);
+      expect(r.stdout).toMatch(/Demo monitoring target configured/);
+      expect(fs.statSync(instancesPath).isFile()).toBe(true);
+      const content = fs.readFileSync(instancesPath, "utf8");
+      expect(content).toContain("name: target_database");
+    } finally {
+      if (fs.existsSync(instancesPath)) fs.rmSync(instancesPath, { recursive: true, force: true });
+    }
+  });
+
   test("cli: mon local-install --demo with global --api-key shows error", () => {
     // When --demo is used with global --api-key, it should still be detected and error
     const r = runCli([

cli/test/monitoring.test.ts

@@ -259,3 +259,81 @@ describe("registerMonitoringInstance", () => {
     expect(fetchCalls[0].url).toBe("https://custom.api.com/v2/rpc/monitoring_instance_register");
   });
 });
+
+describe("demo mode instances.demo.yml", () => {
+  const repoRoot = path.resolve(import.meta.dir, "..", "..");
+
+  test("instances.demo.yml exists in repo root", () => {
+    const demoFile = path.join(repoRoot, "instances.demo.yml");
+    expect(fs.existsSync(demoFile)).toBe(true);
+  });
+
+  test("instances.demo.yml contains demo target connection", () => {
+    const demoFile = path.join(repoRoot, "instances.demo.yml");
+    const content = fs.readFileSync(demoFile, "utf8");
+    expect(content).toContain("name: target_database");
+    expect(content).toContain("conn_str: postgresql://monitor:monitor_pass@target-db:5432/target_database");
+    expect(content).toContain("is_enabled: true");
+    expect(content).toContain("preset_metrics: full");
+  });
+
+  test("instances.demo.yml has required YAML structure", () => {
+    const demoFile = path.join(repoRoot, "instances.demo.yml");
+    const content = fs.readFileSync(demoFile, "utf8");
+    // Verify it's a YAML list (starts with "- name:")
+    expect(content).toMatch(/^- name: target_database/m);
+    // Verify required fields are present with correct indentation
+    expect(content).toMatch(/^\s+conn_str:/m);
+    expect(content).toMatch(/^\s+preset_metrics: full/m);
+    expect(content).toMatch(/^\s+is_enabled: true/m);
+    // ~sink_type~ is a sed token substituted by generate-pgwatch-sources.sh; values: postgres, prometheus
+    expect(content).toMatch(/^\s+sink_type: ~sink_type~/m);
+  });
+
+  test("instances.yml is gitignored (not tracked)", () => {
+    const gitignore = fs.readFileSync(path.join(repoRoot, ".gitignore"), "utf8");
+    expect(gitignore).toMatch(/^instances\.yml$/m);
+  });
+
+  test("demo config can be copied to instances.yml in temp dir", () => {
+    const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "demo-install-test-"));
+    try {
+      const demoSrc = path.join(repoRoot, "instances.demo.yml");
+      const instancesDest = path.join(tempDir, "instances.yml");
+
+      fs.copyFileSync(demoSrc, instancesDest);
+
+      expect(fs.existsSync(instancesDest)).toBe(true);
+      const content = fs.readFileSync(instancesDest, "utf8");
+      expect(content).toContain("name: target_database");
+      expect(content).toContain("conn_str: postgresql://monitor:monitor_pass@target-db:5432/target_database");
+    } finally {
+      fs.rmSync(tempDir, { recursive: true, force: true });
+    }
+  });
+
+  test("demo config copy overwrites directory at instances.yml path", () => {
+    // Docker bind-mounts create missing paths as directories; the copy must handle this
+    const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "demo-eisdir-test-"));
+    try {
+      const demoSrc = path.join(repoRoot, "instances.demo.yml");
+      const instancesDest = path.join(tempDir, "instances.yml");
+
+      // Simulate Docker creating a directory at instances.yml path
+      fs.mkdirSync(instancesDest);
+      expect(fs.statSync(instancesDest).isDirectory()).toBe(true);
+
+      // The fix: remove directory then copy
+      if (fs.statSync(instancesDest).isDirectory()) {
+        fs.rmSync(instancesDest, { recursive: true, force: true });
+      }
+      fs.copyFileSync(demoSrc, instancesDest);
+
+      expect(fs.statSync(instancesDest).isFile()).toBe(true);
+      const content = fs.readFileSync(instancesDest, "utf8");
+      expect(content).toContain("name: target_database");
+    } finally {
+      fs.rmSync(tempDir, { recursive: true, force: true });
+    }
+  });
+});

instances.yml instances.demo.ymlinstances.yml renamed to instances.demo.yml

@@ -1,3 +1,6 @@
+# Demo PostgreSQL instance for `pgai mon local-install --demo`
+# This file is copied to instances.yml during demo mode setup.
+
 - name: target_database
   conn_str: postgresql://monitor:monitor_pass@target-db:5432/target_database
   preset_metrics: full
@@ -8,4 +11,4 @@
     env: demo
     cluster: local
     node_name: node-01
-    sink_type: ~sink_type~
+    sink_type: ~sink_type~  # sed token substituted by generate-pgwatch-sources.sh; values: postgres, prometheus

tests/e2e.cli.sh

@@ -7,6 +7,11 @@ set -e
 CLI_CMD="node ./cli/dist/bin/postgres-ai.js"
 MON_CMD="$CLI_CMD mon"
 
+# Ensure instances.yml exists as a file (instances.demo.yml is the tracked template)
+if [[ ! -f instances.yml ]]; then
+  cp instances.demo.yml instances.yml || { echo "ERROR: instances.demo.yml missing — cannot seed instances.yml" >&2; exit 1; }
+fi
+
 echo "=== Testing service commands ==="
 $MON_CMD check || true
 $MON_CMD config || true