Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions server/routes.migration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,12 @@ describe("error_logs dedup column migration at startup", () => {
const app = makeMockApp();
await registerRoutes(app as any, app as any);

// db.execute should have been called 11 times:
// db.execute should have been called 16 times:
// 3 for the ALTER TABLE error_logs statements (first_occurrence, occurrence_count, deleted_at)
// 2 for the api_keys table creation (CREATE TABLE + CREATE INDEX)
// 6 for notification channel tables (3 CREATE TABLE + 2 indexes + 1 unique index)
expect(mockDbExecute).toHaveBeenCalledTimes(11);
// 5 for tag tables (2 CREATE TABLE + 2 indexes + 1 unique index)
expect(mockDbExecute).toHaveBeenCalledTimes(16);

// Verify specific DDL statements were issued (drizzle sql`` produces SQL objects)
const callStrings = mockDbExecute.mock.calls.map((c: any[]) => {
Expand All @@ -184,6 +185,8 @@ describe("error_logs dedup column migration at startup", () => {
expect(callStrings.some((s: string) => s.includes("notification_channels"))).toBe(true);
expect(callStrings.some((s: string) => s.includes("delivery_log"))).toBe(true);
expect(callStrings.some((s: string) => s.includes("slack_connections"))).toBe(true);
expect(callStrings.some((s: string) => s.includes("CREATE TABLE IF NOT EXISTS tags"))).toBe(true);
expect(callStrings.some((s: string) => s.includes("monitor_tags"))).toBe(true);
});

it("still registers all route groups when migration succeeds", async () => {
Expand Down Expand Up @@ -346,6 +349,45 @@ describe("error_logs dedup column migration at startup", () => {
expect(scCreate).not.toContain("CHECK");
});

it("issues DDL for tags and monitor_tags with correct columns and indexes", async () => {
vi.clearAllMocks();
mockDbExecute.mockResolvedValue({ rows: [] });
process.env.APP_OWNER_ID = "owner-123";

const { registerRoutes } = await import("./routes");
const app = makeMockApp();
await registerRoutes(app as any, app as any);

const callStrings = mockDbExecute.mock.calls.map((c: any[]) => {
try { return JSON.stringify(c[0]); } catch { return String(c[0]); }
});

// tags CREATE TABLE includes required columns
const tagsCreate = callStrings.find((s: string) =>
s.includes("CREATE TABLE IF NOT EXISTS tags")
);
expect(tagsCreate).toBeDefined();
expect(tagsCreate).toContain("user_id");
expect(tagsCreate).toContain("name_lower");
expect(tagsCreate).toContain("colour");

// tags indexes
expect(callStrings.some((s: string) => s.includes("tags_user_idx"))).toBe(true);
expect(callStrings.some((s: string) => s.includes("tags_user_name_lower_uniq"))).toBe(true);

// monitor_tags CREATE TABLE includes required columns and constraints
const mtCreate = callStrings.find((s: string) =>
s.includes("CREATE TABLE IF NOT EXISTS monitor_tags")
);
expect(mtCreate).toBeDefined();
expect(mtCreate).toContain("monitor_id");
expect(mtCreate).toContain("tag_id");
expect(mtCreate).toContain("ON DELETE CASCADE");

// monitor_tags unique index
expect(callStrings.some((s: string) => s.includes("monitor_tags_monitor_tag_uniq"))).toBe(true);
});

it("logs error and continues when notification channel table creation fails", async () => {
vi.clearAllMocks();
const channelError = new Error("permission denied for schema public");
Expand Down
3 changes: 2 additions & 1 deletion server/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { encryptToken, decryptToken, isValidEncryptedToken } from "./utils/encry
import { validateHost } from "./utils/hostValidation";
import { createHmac } from "node:crypto";
import rateLimit from "express-rate-limit";
import { ensureErrorLogColumns, ensureApiKeysTable, ensureChannelTables } from "./services/ensureTables";
import { ensureErrorLogColumns, ensureApiKeysTable, ensureChannelTables, ensureTagTables } from "./services/ensureTables";


// ------------------------------------------------------------------
Expand Down Expand Up @@ -62,6 +62,7 @@ export async function registerRoutes(
await ensureErrorLogColumns();
const apiKeysReady = await ensureApiKeysTable();
await ensureChannelTables();
await ensureTagTables();

// Setup Auth (must be before rate limiter so req.user is populated)
await setupAuth(app);
Expand Down
33 changes: 33 additions & 0 deletions server/services/ensureTables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,36 @@ export async function ensureChannelTables(): Promise<void> {
console.error("Could not ensure notification channel tables:", e);
}
}

/**
* Ensures tags and monitor_tags tables exist (added in PR #86).
* Without this, getMonitorsWithTags() fails when the tables have not been created yet.
*/
export async function ensureTagTables(): Promise<void> {
try {
await db.execute(sql`
CREATE TABLE IF NOT EXISTS tags (
id SERIAL PRIMARY KEY,
user_id TEXT NOT NULL REFERENCES users(id),
name TEXT NOT NULL,
name_lower TEXT NOT NULL,
colour TEXT NOT NULL,
created_at TIMESTAMP NOT NULL DEFAULT NOW()
)
`);
await db.execute(sql`CREATE INDEX IF NOT EXISTS tags_user_idx ON tags(user_id)`);
await db.execute(sql`CREATE UNIQUE INDEX IF NOT EXISTS tags_user_name_lower_uniq ON tags(user_id, name_lower)`);

await db.execute(sql`
CREATE TABLE IF NOT EXISTS monitor_tags (
id SERIAL PRIMARY KEY,
monitor_id INTEGER NOT NULL REFERENCES monitors(id) ON DELETE CASCADE,
tag_id INTEGER NOT NULL REFERENCES tags(id) ON DELETE CASCADE,
created_at TIMESTAMP NOT NULL DEFAULT NOW()
)
`);
await db.execute(sql`CREATE UNIQUE INDEX IF NOT EXISTS monitor_tags_monitor_tag_uniq ON monitor_tags(monitor_id, tag_id)`);
} catch (e) {
console.error("Could not ensure tag tables:", e);
}
Comment on lines +104 to +130
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Propagate tag-table bootstrap failure instead of only logging it.

This helper always resolves successfully, so server/routes.ts continues mounting /api/monitors and the tag routes even when the DDL fails. In read-only or permission-constrained deployments, that leaves the original missing-table 500 path intact.

Suggested fix
-export async function ensureTagTables(): Promise<void> {
+export async function ensureTagTables(): Promise<boolean> {
   try {
     await db.execute(sql`
       CREATE TABLE IF NOT EXISTS tags (
         id SERIAL PRIMARY KEY,
         user_id TEXT NOT NULL REFERENCES users(id),
         name TEXT NOT NULL,
         name_lower TEXT NOT NULL,
         colour TEXT NOT NULL,
         created_at TIMESTAMP NOT NULL DEFAULT NOW()
       )
     `);
     await db.execute(sql`CREATE INDEX IF NOT EXISTS tags_user_idx ON tags(user_id)`);
     await db.execute(sql`CREATE UNIQUE INDEX IF NOT EXISTS tags_user_name_lower_uniq ON tags(user_id, name_lower)`);

     await db.execute(sql`
       CREATE TABLE IF NOT EXISTS monitor_tags (
         id SERIAL PRIMARY KEY,
         monitor_id INTEGER NOT NULL REFERENCES monitors(id) ON DELETE CASCADE,
         tag_id INTEGER NOT NULL REFERENCES tags(id) ON DELETE CASCADE,
         created_at TIMESTAMP NOT NULL DEFAULT NOW()
       )
     `);
     await db.execute(sql`CREATE UNIQUE INDEX IF NOT EXISTS monitor_tags_monitor_tag_uniq ON monitor_tags(monitor_id, tag_id)`);
+    return true;
   } catch (e) {
     console.error("Could not ensure tag tables:", e);
+    return false;
   }
 }
-  await ensureTagTables();
+  const tagTablesReady = await ensureTagTables();
+  if (!tagTablesReady) {
+    throw new Error("Tag tables are unavailable");
+  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/ensureTables.ts` around lines 104 - 130, ensureTagTables
currently catches errors and only logs them, allowing startup to proceed despite
failed DDL; change ensureTagTables (the function in
server/services/ensureTables.ts) to rethrow the caught error (or remove the
try/catch) so failures propagate to the caller (e.g., server/routes.ts) and
prevent mounting tag/monitor routes when the tables couldn't be created; ensure
you preserve the logging but then throw the original error (or a wrapped Error)
after logging.

}
Loading