Fix logic bugs found in third multi-agent review

Medium:
- create_ticket_api.php: environment tags were parsed with explode('][') which
  left brackets on the first/last tag so the whitelist never matched, dropping
  the env tag from the dedup hash — a [production] and [staging] issue with
  otherwise-identical components could collide onto one ticket. Use a
  bracket-aware regex.
- CommentModel::getThreadedCommentsPaged only fetched DIRECT children of root
  comments, so when pagination is active, nested replies at depth 2-3 vanished
  from the thread. Expand replies level-by-level (bounded to depth 3).
- StatsModel::getTicketsByAssignee ignored the visibility filter the rest of the
  stats apply, so a non-admin's "by assignee" widget counted (leaked) confidential
  tickets. Thread the same filter through.
- watch_ticket.php GET path returned watch state / watcher names / count for any
  ticket with no access check (the POST path checks it) — added canUserAccessTicket.
- dashboard.js kanban: every card rendered as P4 because the [class*="lt-p"]
  selector never matched the lt-badge-p1 class and the fallback didn't strip "P".
  Extract the digit directly.

Low:
- audit_log.php CSV: "Log ID" column was always blank ($log['log_id'] vs the real
  audit_id column). Use audit_id.
- check_duplicates.php: the graceful-degradation try/catch only covered the throw
  path; guard the false-return (non-exception mysqli) path too.
- notifications.php: owner-who-is-also-@mentioned got two notifications for one
  comment; drop the duplicate comment row when a mention covers the same comment.
- dashboard.js hover preview rendered "PP1" (doubled prefix); strip the leading P.
- markdown.js: code/inline-code restore used string replace, so $&, $$, $`, $' in
  user code were treated as replacement patterns; use a function replacer. Also
  removed an unused loop var.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

api/audit_log.php

@@ -70,7 +70,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
             }
 
             fputcsv($output, [
-                $log['log_id'],
+                $log['audit_id'] ?? ($log['log_id'] ?? ''),
                 $log['created_at'],
                 $log['display_name'] ?? $log['username'] ?? 'N/A',
                 $log['action_type'],

api/check_duplicates.php

@@ -64,6 +64,11 @@ try {
     }
     $stmt->execute();
     $result = $stmt->get_result();
+    if ($result === false) {
+        // Non-exception mysqli mode: execute/get_result return false instead of
+        // throwing. Treat as a query failure so we don't fatal on $result below.
+        throw new RuntimeException('query failed: ' . $conn->error);
+    }
 } catch (Throwable $e) {
     error_log('check_duplicates: ' . $e->getMessage());
     ResponseHelper::success(['duplicates' => []]);

api/notifications.php

@@ -175,6 +175,23 @@ $stmt->execute();
 $mentionRows = $stmt->get_result()->fetch_all(MYSQLI_ASSOC);
 $stmt->close();
 
+// If the user owns/watches a ticket AND was @mentioned in the same comment, the
+// comment query and the mention query both produce a row for it. Prefer the more
+// specific mention and drop the duplicate comment notification for that comment.
+$mentionCommentIds = [];
+foreach ($mentionRows as $mr) {
+    $md = json_decode($mr['details'] ?? '{}', true) ?? [];
+    if (!empty($md['comment_id'])) {
+        $mentionCommentIds[(int)$md['comment_id']] = true;
+    }
+}
+if (!empty($mentionCommentIds)) {
+    $commentRows = array_filter(
+        $commentRows,
+        fn($cr) => !isset($mentionCommentIds[(int)($cr['entity_id'] ?? 0)])
+    );
+}
+
 // Merge, deduplicate by log_id, sort by created_at desc
 $all = [];
 $seen = [];

api/watch_ticket.php

@@ -78,6 +78,17 @@ if ($ticketId <= 0) {
     exit;
 }
 
+// Enforce ticket visibility before returning watch state / watcher names, so a
+// restricted ticket's watcher list and count aren't disclosed (the POST path
+// already checks this).
+$ticketModel = new TicketModel($conn);
+$ticket = $ticketModel->getTicketById($ticketId);
+if (!$ticket || !$ticketModel->canUserAccessTicket($ticket, $currentUser)) {
+    http_response_code(404);
+    echo json_encode(['success' => false, 'error' => 'Ticket not found']);
+    exit;
+}
+
 $watchingStmt = $conn->prepare(
     "SELECT COUNT(*) as cnt FROM ticket_watchers WHERE ticket_id = ? AND user_id = ?"
 );

assets/js/dashboard.js

@@ -1142,8 +1142,10 @@ function populateKanbanCards() {
         if (cells.length < 6) return;
 
         const ticketId   = cells[0 + offset]?.querySelector('.ticket-link')?.textContent.trim() || '';
-        const priorityEl = cells[1 + offset]?.querySelector('[class*="lt-p"]');
-        const priority   = priorityEl ? priorityEl.textContent.trim().replace('P','') : cells[1 + offset]?.textContent.trim() || '4';
+        // The priority cell renders a "P1".."P5" badge; extract just the digit.
+        // (The old [class*="lt-p"] selector never matched the lt-badge-p1 class, so
+        // every card fell back to P4 regardless of real priority.)
+        const priority   = (cells[1 + offset]?.textContent.trim() || '').replace(/[^0-9]/g, '') || '4';
         const title      = cells[2 + offset]?.textContent.trim() || '';
         const category   = cells[3 + offset]?.textContent.trim() || '';
         const statusEl   = cells[5 + offset]?.querySelector('.lt-status');
@@ -1315,7 +1317,9 @@ function showTicketPreview(event) {
         const offset = isAdmin ? 1 : 0;
 
         const ticketId = link.textContent.trim();
-        const priority = cells[1 + offset]?.textContent.trim() || '';
+        // Cell text is already "P1".."P5"; strip the leading P so the template's
+        // `P${priority}` doesn't render "PP1".
+        const priority = (cells[1 + offset]?.textContent.trim() || '').replace(/^P/i, '');
         const title = cells[2 + offset]?.textContent.trim() || '';
         const category = cells[3 + offset]?.textContent.trim() || '';
         const type = cells[4 + offset]?.textContent.trim() || '';

assets/js/markdown.js

@@ -142,12 +142,14 @@ function parseMarkdown(markdown) {
     html = html.replace(/  \n/g, '<br>');
     html = html.replace(/\n\n/g, '</p><p>');
 
-    // Restore code blocks and inline code
+    // Restore code blocks and inline code. Use a function replacer so '$'
+    // sequences in user code (e.g. $&, $$, $`, $') are inserted literally rather
+    // than interpreted as String.replace replacement patterns.
     codeBlocks.forEach((block, i) => {
-        html = html.replace('%%CODEBLOCK' + i + '%%', block);
+        html = html.replace('%%CODEBLOCK' + i + '%%', () => block);
     });
     inlineCodes.forEach((code, i) => {
-        html = html.replace('%%INLINECODE' + i + '%%', code);
+        html = html.replace('%%INLINECODE' + i + '%%', () => code);
     });
 
     // Restore footnote reference placeholders
@@ -164,7 +166,7 @@ function parseMarkdown(markdown) {
     // Append footnote definitions block
     if (footnoteOrder.length) {
         html += '<hr class="fn-hr"><ol class="fn-list">';
-        footnoteOrder.forEach(function(label, i) {
+        footnoteOrder.forEach(function(label) {
             html += '<li id="fn-' + fnSlug(label) + '" class="fn-item">' +
                 parseMarkdown(footnotes[label]).replace(/<\/?p>/g, '') +
                 ' <a href="#fnref-' + fnSlug(label) + '" class="fn-back">&#x21A9;</a></li>';

create_ticket_api.php

@@ -195,10 +195,17 @@ function generateTicketHash($data)
         'source_type'    => $sourceType,
         'issue_category' => $issueCategory,
         'issue_subtype'  => $issueSubtype,
-        'environment_tags' => array_values(array_filter(
-            explode('][', $title),
-            fn($tag) => in_array($tag, ['production', 'development', 'staging', 'single-node', 'cluster-wide'])
-        )),
+        'environment_tags' => (function () use ($title) {
+            // Extract each [bracketed] tag, then keep the known environment ones.
+            // (explode('][') leaves brackets stuck to the first/last tag, so e.g.
+            // "[production] ..." never matched and the env tag was dropped from the
+            // dedup hash — letting prod and staging issues collide onto one ticket.)
+            preg_match_all('/\[([^\]]+)\]/', $title, $m);
+            return array_values(array_filter(
+                $m[1],
+                fn($tag) => in_array($tag, ['production', 'development', 'staging', 'single-node', 'cluster-wide'], true)
+            ));
+        })(),
     ];
 
     // Manual tickets should be unique by title (so different software installs don't collide)

models/CommentModel.php

@@ -176,27 +176,41 @@ class CommentModel
             return [];
         }
 
-        // All replies for these root comments (up to 3 levels deep)
-        $placeholders = implode(',', array_fill(0, count($rootIds), '?'));
-        $replySql = "SELECT tc.*, u.display_name, u.username
-                     FROM ticket_comments tc
-                     LEFT JOIN users u ON tc.user_id = u.user_id
-                     WHERE tc.ticket_id = ?
-                       AND tc.parent_comment_id IN ($placeholders)
-                       AND tc.parent_comment_id IS NOT NULL
-                     ORDER BY tc.created_at ASC";
-        $replyStmt = $this->conn->prepare($replySql);
-        $types = 'i' . str_repeat('i', count($rootIds));
-        $replyStmt->bind_param($types, $ticketId, ...$rootIds);
-        $replyStmt->execute();
-        $replyResult = $replyStmt->get_result();
-        $replyStmt->close();
+        // Load replies level-by-level under this page's roots. A single
+        // "parent_comment_id IN (rootIds)" only fetches DIRECT children, so
+        // grandchildren/great-grandchildren (addComment allows up to depth 3)
+        // would be missing from the map and dropped by buildCommentThread.
+        // Expand iteratively until no new replies (bounded by max depth 3).
+        $parentIds = $rootIds;
+        $depth = 0;
+        while (!empty($parentIds) && $depth < 3) {
+            $placeholders = implode(',', array_fill(0, count($parentIds), '?'));
+            $replySql = "SELECT tc.*, u.display_name, u.username
+                         FROM ticket_comments tc
+                         LEFT JOIN users u ON tc.user_id = u.user_id
+                         WHERE tc.ticket_id = ?
+                           AND tc.parent_comment_id IN ($placeholders)
+                         ORDER BY tc.created_at ASC";
+            $replyStmt = $this->conn->prepare($replySql);
+            $types = 'i' . str_repeat('i', count($parentIds));
+            $replyStmt->bind_param($types, $ticketId, ...$parentIds);
+            $replyStmt->execute();
+            $replyResult = $replyStmt->get_result();
+            $replyStmt->close();
 
-        while ($row = $replyResult->fetch_assoc()) {
-            $row['display_name_formatted'] = $row['display_name'] ?: ($row['user_name'] ?? 'Unknown User');
-            $row['replies']      = [];
-            $row['thread_depth'] = $row['thread_depth'] ?? 1;
-            $commentMap[$row['comment_id']] = $row;
+            $nextParentIds = [];
+            while ($row = $replyResult->fetch_assoc()) {
+                if (isset($commentMap[$row['comment_id']])) {
+                    continue; // guard against cycles / duplicates
+                }
+                $row['display_name_formatted'] = $row['display_name'] ?: ($row['user_name'] ?? 'Unknown User');
+                $row['replies'] = [];
+                $row['thread_depth'] = $depth + 1;
+                $commentMap[$row['comment_id']] = $row;
+                $nextParentIds[] = $row['comment_id'];
+            }
+            $parentIds = $nextParentIds;
+            $depth++;
         }
 
         $rootComments = [];

models/StatsModel.php

@@ -28,8 +28,14 @@ class StatsModel
     /**
      * Get tickets by assignee (top 5)
      */
-    public function getTicketsByAssignee(int $limit = 8): array
+    public function getTicketsByAssignee(int $limit = 8, array $visFilter = []): array
     {
+        // Apply the same visibility filter as the rest of the stats so a non-admin's
+        // assignee widget doesn't count (and thereby leak) confidential tickets.
+        $visSQL    = $visFilter['sql']    ?? '';
+        $visParams = $visFilter['params'] ?? [];
+        $visTypes  = $visFilter['types']  ?? '';
+
         $sql = "SELECT
                     u.user_id,
                     u.display_name,
@@ -37,12 +43,20 @@ class StatsModel
                     COUNT(t.ticket_id) as open_count
                 FROM tickets t
                 LEFT JOIN users u ON t.assigned_to = u.user_id
-                WHERE t.status != 'Closed' AND t.assigned_to IS NOT NULL
-                GROUP BY t.assigned_to
-                ORDER BY open_count DESC
-                LIMIT ?";
+                WHERE t.status != 'Closed' AND t.assigned_to IS NOT NULL";
+        if ($visSQL !== '') {
+            $sql .= " AND ($visSQL)";
+        }
+        $sql .= " GROUP BY t.assigned_to
+                  ORDER BY open_count DESC
+                  LIMIT ?";
+
+        $params   = $visParams;
+        $params[] = $limit;
+        $types    = $visTypes . 'i';
+
         $stmt = $this->conn->prepare($sql);
-        $stmt->bind_param('i', $limit);
+        $stmt->bind_param($types, ...$params);
         $stmt->execute();
         $result = $stmt->get_result();
         $data = [];
@@ -173,8 +187,9 @@ class StatsModel
         // Sort priority keys
         ksort($byPriority);
 
-        // Query 3: Get assignee stats (requires JOIN, kept separate)
-        $byAssignee = $this->getTicketsByAssignee();
+        // Query 3: Get assignee stats (requires JOIN, kept separate). Pass the same
+        // visibility filter so confidential tickets aren't counted for non-admins.
+        $byAssignee = $this->getTicketsByAssignee(8, $visFilter);
 
         return [
             'open_tickets' => (int)($counts['open_tickets'] ?? 0),