fix: address PR #65 review comments

- Fix MessagesPage timestamp types (number not string) and cap messages at 500
- Fix broker resubscribeAll to check boolean returns and revert to file polling
- Fix redis-bus subscribe to only add channels after successful Redis subscribe
- Fix file-store polling to initialize timestamp at 0 for replay
- Fix system restore to default backupIndex to 1
- Fix state-manager listBackups to sort by recency descending
- Fix state-manager restoreFromBackup to use persistState queue
- Fix chocolatier notification config to load from repo .cocopilot/config.json
- Remove unused generateFixupSummary import from temperer
- Fix chocolatier.test.ts to use Partial<CocoConfig> and Partial<RepoState>
- Fix workers routes to use const for non-reassigned variables
- Implement per-repo template loading in templates/workers routes

Co-authored-by: patrick-knight <7575792+patrick-knight@users.noreply.github.com>

Author: Copilot

src/agents/chocolatier.test.ts

@@ -10,6 +10,7 @@ import { ContainerStatus, ContainerType } from "../docker/index.js";
 import type { MessageBroker } from "../messaging/index.js";
 import { MessageType } from "../messaging/index.js";
 import type { CocoMessage } from "../messaging/index.js";
+import type { CocoConfig, RepoState } from "../types/index.js";
 
 // ---------------------------------------------------------------------------
 // Mocks
@@ -1032,7 +1033,7 @@ describe("Chocolatier", () => {
 
       stateManager.getConfig.mockReturnValue({
         workerTimeout: "4h",
-      } as any);
+      } as Partial<CocoConfig>);
 
       stateManager.getRepo.mockReturnValue({
         id: "repo-1",
@@ -1050,12 +1051,12 @@ describe("Chocolatier", () => {
           },
         },
         agents: {},
-      } as any);
+      } as Partial<RepoState>);
 
       containerManager.list.mockResolvedValue([]);
       containerManager.stop.mockResolvedValue(undefined);
 
-      const report = await chocolatier.runHealthCheck();
+      await chocolatier.runHealthCheck();
 
       expect(stateManager.updateWorkerStatus).toHaveBeenCalledWith(
         "test-repo",
@@ -1081,7 +1082,7 @@ describe("Chocolatier", () => {
 
       stateManager.getConfig.mockReturnValue({
         workerTimeout: "4h",
-      } as any);
+      } as Partial<CocoConfig>);
 
       stateManager.getRepo.mockReturnValue({
         id: "repo-1",
@@ -1099,7 +1100,7 @@ describe("Chocolatier", () => {
           },
         },
         agents: {},
-      } as any);
+      } as Partial<RepoState>);
 
       containerManager.list.mockResolvedValue([
         {
@@ -1129,7 +1130,7 @@ describe("Chocolatier", () => {
 
       stateManager.getConfig.mockReturnValue({
         workerTimeout: "4h",
-      } as any);
+      } as Partial<CocoConfig>);
 
       stateManager.getRepo.mockReturnValue({
         id: "repo-1",
@@ -1147,7 +1148,7 @@ describe("Chocolatier", () => {
           },
         },
         agents: {},
-      } as any);
+      } as Partial<RepoState>);
 
       containerManager.list.mockResolvedValue([]);
       containerManager.stop.mockResolvedValue(undefined);

src/agents/chocolatier.ts

@@ -13,9 +13,12 @@
 
 import { EventEmitter } from "node:events";
 import { randomUUID } from "node:crypto";
+import * as fs from "node:fs";
+import * as path from "node:path";
 
 import { StateManager } from "../state/index.js";
 import type { WorkerState } from "../state/index.js";
+import type { RepoConfig } from "../state/schemas.js";
 import {
   ContainerManager,
   ContainerType,
@@ -543,6 +546,21 @@ export class Chocolatier extends EventEmitter {
     });
   }
 
+  /**
+   * Load per-repo configuration from .cocopilot/config.json
+   */
+  private loadRepoConfig(localPath: string): RepoConfig {
+    try {
+      const configPath = path.join(localPath, ".cocopilot", "config.json");
+      if (fs.existsSync(configPath)) {
+        return JSON.parse(fs.readFileSync(configPath, "utf-8")) as RepoConfig;
+      }
+    } catch {
+      // Ignore read errors
+    }
+    return {};
+  }
+
   /**
    * Fire-and-forget: create a GitHub notification issue if configured.
    * Failures are logged but never block agent operations.
@@ -551,9 +569,8 @@ export class Chocolatier extends EventEmitter {
     const repo = this.stateManager.getRepo(this.config.repoName);
     if (!repo) return;
 
-    const notifConfig: NotificationConfig =
-      (repo as unknown as { config?: { notifications?: NotificationConfig } }).config?.notifications
-        ?? DEFAULT_NOTIFICATION_CONFIG;
+    const repoConfig = this.loadRepoConfig(repo.localPath);
+    const notifConfig = repoConfig.notifications ?? DEFAULT_NOTIFICATION_CONFIG;
 
     if (!shouldNotify(notifConfig, event.type)) return;
 

src/agents/temperer.ts

@@ -20,7 +20,6 @@ import {
 } from "../messaging/index.js";
 import {
   getCIStatus,
-  generateFixupSummary,
   type CIExecFn,
   checkMergeability,
   shouldNotify,

src/api/v1/system.ts

@@ -64,12 +64,14 @@ export async function restoreBackup(
     return;
   }
 
+  const index = backupIndex ?? 1;
+
   try {
-    const ok = await deps.stateManager.restoreFromBackup(backupIndex);
+    const ok = await deps.stateManager.restoreFromBackup(index);
     if (ok) {
       res.json({
         success: true,
-        message: `State restored from backup ${backupIndex ?? 1}`,
+        message: `State restored from backup ${index}`,
         repositories: Object.keys(deps.stateManager.getRepos()),
       });
     } else {

src/api/v1/templates.ts

@@ -5,9 +5,11 @@
  * GET /api/v1/templates/:id     — Get a specific template by ID
  */
 
+import * as fs from "node:fs";
+import * as path from "node:path";
 import { Router } from "express";
 import type { StateManager } from "../../state/index.js";
-import type { TaskTemplate } from "../../state/schemas.js";
+import type { TaskTemplate, RepoConfig } from "../../state/schemas.js";
 import { createApiError } from "../../server/middleware/error-handler.js";
 
 // ---------------------------------------------------------------------------
@@ -90,6 +92,21 @@ export function resolveTemplate(
   return BUILTIN_TEMPLATES.find((t) => t.id === templateId);
 }
 
+/**
+ * Load per-repo configuration from .cocopilot/config.json
+ */
+function loadRepoConfig(localPath: string): RepoConfig {
+  try {
+    const configPath = path.join(localPath, ".cocopilot", "config.json");
+    if (fs.existsSync(configPath)) {
+      return JSON.parse(fs.readFileSync(configPath, "utf-8")) as RepoConfig;
+    }
+  } catch {
+    // Ignore read errors
+  }
+  return {};
+}
+
 // ---------------------------------------------------------------------------
 // Routes
 // ---------------------------------------------------------------------------
@@ -110,10 +127,8 @@ export function templatesRoutes(deps: TemplatesDeps): Router {
     if (repoName) {
       const repo = stateManager.getRepo(repoName);
       if (repo) {
-        // Repo-specific templates would be stored in repo config
-        // For now we look at a hypothetical templates field loaded from config
-        // This is a placeholder for when per-repo config loading is wired up
-        repoTemplates = [];
+        const repoConfig = loadRepoConfig(repo.localPath);
+        repoTemplates = repoConfig.templates ?? [];
       }
     }
 

src/api/v1/workers.ts

@@ -12,14 +12,32 @@
  * POST   /api/v1/workers/:name/resume  -- Resume a paused worker
  */
 
+import * as fs from "node:fs";
+import * as path from "node:path";
 import { Router } from "express";
 import type { StateManager } from "../../state/index.js";
 import type { MessageBroker } from "../../messaging/index.js";
 import { MessageType } from "../../messaging/index.js";
 import { chocolatierAgentName } from "../../agents/chocolatier.js";
 import { createApiError } from "../../server/middleware/error-handler.js";
+import type { RepoConfig, TaskTemplate } from "../../state/schemas.js";
 import { resolveTemplate, BUILTIN_TEMPLATES } from "./templates.js";
 
+/**
+ * Load per-repo configuration from .cocopilot/config.json
+ */
+function loadRepoConfig(localPath: string): RepoConfig {
+  try {
+    const configPath = path.join(localPath, ".cocopilot", "config.json");
+    if (fs.existsSync(configPath)) {
+      return JSON.parse(fs.readFileSync(configPath, "utf-8")) as RepoConfig;
+    }
+  } catch {
+    // Ignore read errors
+  }
+  return {};
+}
+
 export function extWorkerRoutes(
   stateManager: StateManager,
   broker: MessageBroker,
@@ -29,11 +47,32 @@ export function extWorkerRoutes(
   // POST / -- Spawn a worker in a specific repository
   // Sends SPAWN_WORKER message to Chocolatier which actually spawns the container
   router.post("/", async (req, res, next) => {
-    let { task, repoName, branch, name, model, pushTo, templateId } = req.body ?? {};
+    const body = req.body ?? {};
+    let { task } = body;
+    const { repoName, branch, name, model, pushTo, templateId } = body;
+
+    if (!repoName) {
+      next(createApiError(400, "Missing required field: repoName"));
+      return;
+    }
+
+    // Verify repository exists
+    const repo = stateManager.getRepo(repoName);
+    if (!repo) {
+      next(createApiError(404, `Repository "${repoName}" not tracked`));
+      return;
+    }
+
+    // Get repo-specific templates if resolving from templateId
+    let repoTemplates: TaskTemplate[] | undefined;
+    if (templateId) {
+      const repoConfig = loadRepoConfig(repo.localPath);
+      repoTemplates = repoConfig.templates;
+    }
 
     // Resolve task from template if templateId is provided
     if (templateId && !task) {
-      const template = resolveTemplate(templateId);
+      const template = resolveTemplate(templateId, repoTemplates);
       if (!template) {
         next(createApiError(404, `Template "${templateId}" not found`));
         return;
@@ -45,17 +84,6 @@ export function extWorkerRoutes(
       next(createApiError(400, "Missing required field: task"));
       return;
     }
-    if (!repoName) {
-      next(createApiError(400, "Missing required field: repoName"));
-      return;
-    }
-
-    // Verify repository exists
-    const repo = stateManager.getRepo(repoName);
-    if (!repo) {
-      next(createApiError(404, `Repository "${repoName}" not tracked`));
-      return;
-    }
 
     try {
       // Send SPAWN_WORKER message to Chocolatier

src/messaging/broker.ts

@@ -224,11 +224,23 @@ export class MessageBroker {
         `Use scopedAgentName() or scopedWorkerName() to avoid cross-repo collisions.`,
       );
     }
-    this.agentHandlers.set(agentName, handler);
-    this.lastPollTimestamps.set(agentName, Date.now());
+    
+    // Wrap handler to update timestamp watermark on every delivery
+    const wrappedHandler: MessageHandler = async (msg) => {
+      await handler(msg);
+      // Update watermark to prevent re-delivery during Redis→file fallback
+      const currentTs = this.lastPollTimestamps.get(agentName) ?? 0;
+      if (msg.timestamp > currentTs) {
+        this.lastPollTimestamps.set(agentName, msg.timestamp);
+      }
+    };
+    
+    this.agentHandlers.set(agentName, wrappedHandler);
+    // Initialize to 0 to replay pending messages when Redis is unavailable
+    this.lastPollTimestamps.set(agentName, 0);
 
     if (this.redisAvailable) {
-      const ok = await this.bus.subscribe(agentName, handler);
+      const ok = await this.bus.subscribe(agentName, wrappedHandler);
       if (!ok) {
         console.warn("[MessageBroker] Redis subscribe failed, falling back to file store polling");
         this.redisAvailable = false;
@@ -339,10 +351,10 @@ export class MessageBroker {
   /** Re-subscribe all registered agents to Redis after reconnection. */
   private async resubscribeAll(): Promise<void> {
     for (const [agentName, handler] of this.agentHandlers) {
-      try {
-        await this.bus.subscribe(agentName, handler);
-      } catch {
+      const ok = await this.bus.subscribe(agentName, handler);
+      if (!ok) {
         // If resubscribe fails, remain in file-store-only mode
+        console.warn("[MessageBroker] Resubscribe failed, reverting to file-store polling");
         this.redisAvailable = false;
         this.startFilePolling();
         return;

src/messaging/redis-bus.ts

@@ -151,17 +151,28 @@ export class RedisMessageBus {
     const toSubscribe: string[] = [];
     if (!this.subscribedChannels.has(channel)) {
       toSubscribe.push(channel);
-      this.subscribedChannels.add(channel);
     }
     if (!this.subscribedChannels.has(BROADCAST_CHANNEL)) {
       toSubscribe.push(BROADCAST_CHANNEL);
-      this.subscribedChannels.add(BROADCAST_CHANNEL);
     }
 
     if (toSubscribe.length > 0) {
       try {
         await this.sub.subscribe(...toSubscribe);
+        // Only add to subscribedChannels after successful Redis subscribe
+        for (const ch of toSubscribe) {
+          this.subscribedChannels.add(ch);
+        }
       } catch {
+        // Roll back handlers on failure
+        this.handlers.get(channel)?.delete(handler);
+        if (this.handlers.get(channel)?.size === 0) {
+          this.handlers.delete(channel);
+        }
+        this.handlers.get(BROADCAST_CHANNEL)?.delete(handler);
+        if (this.handlers.get(BROADCAST_CHANNEL)?.size === 0) {
+          this.handlers.delete(BROADCAST_CHANNEL);
+        }
         return false;
       }
     }