fix: stop draining queue on /clear (remove SessionEnd shim) (#2136)

* fix: stop draining queue on /clear (and on every other SessionEnd) The SessionEnd hook was wired to session-complete on Claude Code, Gemini CLI, the transcripts processor, the OpenCode plugin, and OpenClaw. All of those paths called POST /api/sessions/complete, which marked the session completed and abandoned every still-pending observation in the queue. So typing /clear (or logging out, or quitting) wiped in-flight work that the worker was perfectly happy to keep processing on its own. Removed the entire shim: - Deleted SessionEnd hook block in plugin/hooks/hooks.json - Deleted src/cli/handlers/session-complete.ts and its registry entry - Deleted POST /api/sessions/complete route + Zod schema in SessionRoutes - Removed call from transcripts processor handleSessionEnd - Removed call from opencode-plugin session.deleted handler - Removed Gemini SessionEnd → session-complete mapping - Removed openclaw scheduleSessionComplete + completionDelayMs + timer state - Updated tests + comments accordingly Explicit user-initiated deletion (DELETE /api/sessions/:id and POST /api/sessions/:sessionDbId/complete from the viewer UI) still works via SessionCompletionHandler.completeByDbId — that's the only path that should drain the queue. The worker self-completes via its SDK-agent generator's finally-block, so no external completion call is needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: clarify opencode-plugin session.deleted is in-memory cleanup only Greptile P2: file-level header still implied session.deleted called the worker. Now it only cleans up the local contentSessionIdsByOpenCodeSessionId map; worker self-completes via the SDK-agent generator finally-block. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 17:08:35 -07:00
parent 298f5463d9
commit 8e0e3ca109
17 changed files with 540 additions and 695 deletions
@@ -266,12 +266,6 @@ describe("Observation I/O event handlers", () => {
            return;
          }

-          if (req.url === "/api/sessions/complete") {
-            res.writeHead(200, { "Content-Type": "application/json" });
-            res.end(JSON.stringify({ status: "completed" }));
-            return;
-          }
-
          if (req.url?.startsWith("/api/context/inject")) {
            res.writeHead(200, { "Content-Type": "text/plain; charset=utf-8" });
            res.end("# Claude-Mem Context\n\n## Timeline\n- Session 1: Did some work");
@@ -446,8 +440,7 @@ describe("Observation I/O event handlers", () => {
    assert.ok(summarizeRequest!.body.contentSessionId.startsWith("openclaw-summarize-test-"));

    const completeRequest = receivedRequests.find((r) => r.url === "/api/sessions/complete");
-    assert.ok(completeRequest, "should send complete to worker");
-    assert.ok(completeRequest!.body.contentSessionId.startsWith("openclaw-summarize-test-"));
+    assert.ok(!completeRequest, "should not send complete (worker self-completes)");
  });

  it("agent_end extracts text from array content", async () => {
@@ -644,12 +644,7 @@ export default function claudeMemPlugin(api: OpenClawPluginApi): void {
  const sessionIds = new Map<string, string>();
  const canonicalSessionKeys = new Map<string, string>();
  const sessionAliasesByCanonicalKey = new Map<string, Set<string>>();
-  const pendingCompletionTimers = new Map<string, ReturnType<typeof setTimeout>>();
  const recentPromptInits = new Map<string, number>();
-  const completionDelayMs = (() => {
-    const val = Number((userConfig as Record<string, unknown>).completionDelayMs);
-    return Number.isFinite(val) ? Math.max(0, val) : 5000;
-  })();
  const syncMemoryFile = userConfig.syncMemoryFile !== false; // default true
  const syncMemoryFileExclude = new Set(userConfig.syncMemoryFileExclude || []);

@@ -733,18 +728,6 @@ export default function claudeMemPlugin(api: OpenClawPluginApi): void {
    sessionIds.delete(canonicalKey);
  }

-  function scheduleSessionComplete(contentSessionId: string): void {
-    const existingTimer = pendingCompletionTimers.get(contentSessionId);
-    if (existingTimer) clearTimeout(existingTimer);
-    const timer = setTimeout(() => {
-      pendingCompletionTimers.delete(contentSessionId);
-      workerPostFireAndForget(workerPort, "/api/sessions/complete", {
-        contentSessionId,
-      }, api.logger);
-    }, completionDelayMs);
-    pendingCompletionTimers.set(contentSessionId, timer);
-  }
-
  // TTL cache for context injection to avoid re-fetching on every LLM turn.
  // before_prompt_build fires on every turn; caching for 60s keeps the worker
  // load manageable while still picking up new observations reasonably quickly.
@@ -898,7 +881,7 @@ export default function claudeMemPlugin(api: OpenClawPluginApi): void {
  });

  // ------------------------------------------------------------------
-  // Event: agent_end — summarize and complete session
+  // Event: agent_end — summarize session (worker self-completes)
  // ------------------------------------------------------------------
  api.on("agent_end", async (event, ctx) => {
    const { contentSessionId } = rememberSessionContext(ctx);
@@ -922,16 +905,12 @@ export default function claudeMemPlugin(api: OpenClawPluginApi): void {
      }
    }

-    // Await summarize so the worker receives it before complete.
-    // This also gives in-flight tool_result_persist observations time to arrive
-    // (they use fire-and-forget and may still be in transit).
+    // Send summarize. The worker self-completes the session when its SDK-agent
+    // generator drains; no explicit complete call needed.
    await workerPost(workerPort, "/api/sessions/summarize", {
      contentSessionId,
      last_assistant_message: lastAssistantMessage,
    }, api.logger);
-
-    api.logger.info(`[claude-mem] Scheduling session complete in ${completionDelayMs}ms: ${contentSessionId}`);
-    scheduleSessionComplete(contentSessionId);
  });

  // ------------------------------------------------------------------
@@ -952,10 +931,6 @@ export default function claudeMemPlugin(api: OpenClawPluginApi): void {
    recentPromptInits.clear();
    canonicalSessionKeys.clear();
    sessionAliasesByCanonicalKey.clear();
-    for (const timer of pendingCompletionTimers.values()) {
-      clearTimeout(timer);
-    }
-    pendingCompletionTimers.clear();
    api.logger.info("[claude-mem] Gateway started — session tracking reset");
  });

@@ -88,18 +88,6 @@
          }
        ]
      }
-    ],
-    "SessionEnd": [
-      {
-        "hooks": [
-          {
-            "type": "command",
-            "shell": "bash",
-"command": "export PATH=\"$($SHELL -lc 'echo $PATH' 2>/dev/null):$PATH\"; _R=\"${CLAUDE_PLUGIN_ROOT}\"; [ -z \"$_R\" ] && _R=$(ls -dt $HOME/.claude/plugins/cache/thedotmack/claude-mem/[0-9]*/ 2>/dev/null | head -1); _R=\"${_R%/}\"; [ -z \"$_R\" ] && _R=\"$HOME/.claude/plugins/marketplaces/thedotmack/plugin\"; node \"$_R/scripts/bun-runner.js\" \"$_R/scripts/worker-service.cjs\" hook claude-code session-complete",
-            "timeout": 30
-          }
-        ]
-      }
    ]
  }
 }
@@ -5,11 +5,10 @@ import { AdapterRejectedInput, isValidCwd } from './errors.js';
 * Gemini CLI Platform Adapter
 *
 * Normalizes Gemini CLI's hook JSON to NormalizedHookInput.
- * Gemini CLI supports 11 lifecycle hooks; we register 8:
+ * Gemini CLI supports 11 lifecycle hooks; we register 7:
 *
 * Lifecycle:
 *   SessionStart  → context     (inject memory context)
- *   SessionEnd    → session-complete
 *   PreCompress   → summarize
 *   Notification  → observation (system events like ToolPermission)
 *
@@ -28,7 +27,7 @@ import { AdapterRejectedInput, isValidCwd } from './errors.js';
 * Base fields (all events): session_id, transcript_path, cwd, hook_event_name, timestamp
 *
 * Output format: { continue, stopReason, suppressOutput, systemMessage, decision, reason, hookSpecificOutput }
- * Advisory hooks (SessionStart, SessionEnd, PreCompress, Notification) ignore flow-control fields.
+ * Advisory hooks (SessionStart, PreCompress, Notification) ignore flow-control fields.
 */
 export const geminiCliAdapter: PlatformAdapter = {
  normalizeInput(raw) {
@@ -14,14 +14,12 @@ import { summarizeHandler } from './summarize.js';
 import { userMessageHandler } from './user-message.js';
 import { fileEditHandler } from './file-edit.js';
 import { fileContextHandler } from './file-context.js';
-import { sessionCompleteHandler } from './session-complete.js';

 export type EventType =
  | 'context'           // SessionStart - inject context
  | 'session-init'      // UserPromptSubmit - initialize session
  | 'observation'       // PostToolUse - save observation
  | 'summarize'         // Stop - generate summary (phase 1)
-  | 'session-complete'  // Stop - complete session (phase 2) - fixes #842
  | 'user-message'      // SessionStart (parallel) - display to user
  | 'file-edit'         // Cursor afterFileEdit
  | 'file-context';     // PreToolUse - inject file observation history
@@ -31,7 +29,6 @@ const handlers: Record<EventType, EventHandler> = {
  'session-init': sessionInitHandler,
  'observation': observationHandler,
  'summarize': summarizeHandler,
-  'session-complete': sessionCompleteHandler,
  'user-message': userMessageHandler,
  'file-edit': fileEditHandler,
  'file-context': fileContextHandler
@@ -68,4 +65,3 @@ export { summarizeHandler } from './summarize.js';
 export { userMessageHandler } from './user-message.js';
 export { fileEditHandler } from './file-edit.js';
 export { fileContextHandler } from './file-context.js';
-export { sessionCompleteHandler } from './session-complete.js';
@@ -1,52 +0,0 @@
-/**
- * Session Complete Handler - Stop (Phase 2)
- *
- * Completes the session after summarize has been queued.
- * This removes the session from the active sessions map, allowing
- * the orphan reaper to clean up any remaining subprocess.
- *
- * Fixes Issue #842: Orphan reaper starts but never reaps because
- * sessions stay in the active sessions map forever.
- */
-
-import type { EventHandler, NormalizedHookInput, HookResult } from '../types.js';
-import { executeWithWorkerFallback, isWorkerFallback } from '../../shared/worker-utils.js';
-import { logger } from '../../utils/logger.js';
-import { normalizePlatformSource } from '../../shared/platform-source.js';
-import { shouldTrackProject } from '../../shared/should-track-project.js';
-
-export const sessionCompleteHandler: EventHandler = {
-  async execute(input: NormalizedHookInput): Promise<HookResult> {
-    const { sessionId } = input;
-    const platformSource = normalizePlatformSource(input.platform);
-
-    // Same OBSERVER_SESSIONS_DIR exclusion as the rest of the hook surface —
-    // the observer's child Claude Code must never call /api/sessions/complete.
-    if (input.cwd && !shouldTrackProject(input.cwd)) {
-      return { continue: true, suppressOutput: true };
-    }
-
-    if (!sessionId) {
-      logger.warn('HOOK', 'session-complete: Missing sessionId, skipping');
-      return { continue: true, suppressOutput: true };
-    }
-
-    logger.info('HOOK', '→ session-complete: Removing session from active map', {
-      contentSessionId: sessionId,
-    });
-
-    // Plan 05 Phase 2: single helper for ensure-worker-alive → request → fallback.
-    const result = await executeWithWorkerFallback<{ status?: string }>(
-      '/api/sessions/complete',
-      'POST',
-      { contentSessionId: sessionId, platformSource },
-    );
-
-    if (isWorkerFallback(result)) {
-      return { continue: true, suppressOutput: true };
-    }
-
-    logger.info('HOOK', 'Session completed successfully', { contentSessionId: sessionId });
-    return { continue: true, suppressOutput: true };
-  },
-};
@@ -7,7 +7,7 @@
 * Plugin hooks:
 * - tool.execute.after: Captures tool execution observations
 * - Bus events: session.created, message.updated, session.compacted,
- *   file.edited, session.deleted
+ *   file.edited, session.deleted (in-memory cleanup only; worker self-completes)
 *
 * Custom tool:
 * - claude_mem_search: Search memory database from within OpenCode
@@ -299,16 +299,7 @@ export const ClaudeMemPlugin = async (ctx: OpenCodePluginContext) => {

        case "session.deleted": {
          const { event } = payload as SessionDeletedEvent;
-          const contentSessionId = contentSessionIdsByOpenCodeSessionId.get(
-            event.sessionID,
-          );
-
-          if (contentSessionId) {
-            workerPostFireAndForget("/api/sessions/complete", {
-              contentSessionId,
-            });
-            contentSessionIdsByOpenCodeSessionId.delete(event.sessionID);
-          }
+          contentSessionIdsByOpenCodeSessionId.delete(event.sessionID);
          break;
        }
      }
@@ -86,7 +86,6 @@ const GEMINI_EVENT_TO_INTERNAL_EVENT: Record<string, string> = {
  'AfterTool': 'observation',
  'PreCompress': 'summarize',
  'Notification': 'observation',
-  'SessionEnd': 'session-complete',
 };

 // ============================================================================
@@ -1,7 +1,6 @@
 import path from 'path';
 import { sessionInitHandler } from '../../cli/handlers/session-init.js';
 import { fileEditHandler } from '../../cli/handlers/file-edit.js';
-import { sessionCompleteHandler } from '../../cli/handlers/session-complete.js';
 import { ensureWorkerRunning, workerHttpRequest } from '../../shared/worker-utils.js';
 import { DATA_DIR } from '../../shared/paths.js';
 import { logger } from '../../utils/logger.js';
@@ -339,11 +338,6 @@ export class TranscriptEventProcessor {

  private async handleSessionEnd(session: SessionState, watch: WatchTarget): Promise<void> {
    await this.queueSummary(session);
-    await sessionCompleteHandler.execute({
-      sessionId: session.sessionId,
-      cwd: session.cwd ?? process.cwd(),
-      platform: session.platformSource
-    });
    await this.updateContext(session, watch);
    session.pendingTools?.clear();
    const key = this.getSessionKey(watch, session.sessionId);
@@ -806,11 +806,8 @@ export class WorkerService implements WorkerRef {
        } else {
          // Successful completion with no pending work — finalize then drop
          // in-memory state. finalizeSession flips sdk_sessions.status to
-          // 'completed', drains orphaned pendings, broadcasts; idempotent so
-          // the later POST /api/sessions/complete from the Stop hook is a
-          // no-op. Without this, hooks-disabled installs (and any session
-          // whose Stop hook fails before /api/sessions/complete) leave the
-          // DB row permanently 'active'.
+          // 'completed', drains orphaned pendings, broadcasts. This is the
+          // sole completion path now that the SessionEnd hook shim is gone.
          session.restartGuard?.recordSuccess();
          session.consecutiveRestarts = 0;
          this.completionHandler.finalizeSession(session.sessionDbId);
@@ -1225,7 +1222,7 @@ async function main() {
      if (!platform || !event) {
        console.error('Usage: claude-mem hook <platform> <event>');
        console.error('Platforms: claude-code, cursor, gemini-cli, raw');
-        console.error('Events: context, session-init, observation, summarize, session-complete, user-message');
+        console.error('Events: context, session-init, observation, summarize, user-message');
        process.exit(1);
      }

@@ -430,11 +430,6 @@ export class SessionRoutes extends BaseRouteHandler {
      validateBody(SessionRoutes.summarizeByClaudeIdSchema),
      this.handleSummarizeByClaudeId.bind(this)
    );
-    app.post(
-      '/api/sessions/complete',
-      validateBody(SessionRoutes.completeByClaudeIdSchema),
-      this.handleCompleteByClaudeId.bind(this)
-    );
    app.get('/api/sessions/status', this.handleStatusByClaudeId.bind(this));
  }

@@ -490,11 +485,6 @@ export class SessionRoutes extends BaseRouteHandler {
    platformSource: z.string().optional(),
  }).passthrough();

-  private static readonly completeByClaudeIdSchema = z.object({
-    contentSessionId: z.string().min(1),
-    platformSource: z.string().optional(),
-  }).passthrough();
-
  /**
   * Initialize a new session
   */
@@ -793,52 +783,6 @@ export class SessionRoutes extends BaseRouteHandler {
    });
  });

-  /**
-   * Complete session by contentSessionId (session-complete hook uses this)
-   * POST /api/sessions/complete
-   * Body: { contentSessionId }
-   *
-   * Removes session from active sessions map, allowing orphan reaper to
-   * clean up any remaining subprocesses.
-   *
-   * Fixes Issue #842: Sessions stay in map forever, reaper thinks all active.
-   */
-  private handleCompleteByClaudeId = this.wrapHandler(async (req: Request, res: Response): Promise<void> => {
-    const { contentSessionId } = req.body;
-    const platformSource = normalizePlatformSource(req.body.platformSource);
-
-    logger.info('HTTP', '→ POST /api/sessions/complete', { contentSessionId });
-
-    const store = this.dbManager.getSessionStore();
-
-    // Look up sessionDbId from contentSessionId (createSDKSession is idempotent)
-    // Pass empty strings - we only need the ID lookup, not to create a new session
-    const sessionDbId = store.createSDKSession(contentSessionId, '', '', undefined, platformSource);
-
-    // Check if session is in the active sessions map
-    const activeSession = this.sessionManager.getSession(sessionDbId);
-    if (!activeSession) {
-      // Session may not be in memory (already completed or never initialized)
-      // Still proceed with DB-backed completion so the row gets marked completed
-      logger.debug('SESSION', 'session-complete: Session not in active map; continuing with DB-backed completion', {
-        contentSessionId,
-        sessionDbId
-      });
-    }
-
-    // Complete the session (removes from active sessions map if present)
-    // Note: The Stop hook (summarize handler) waits for pending work before calling
-    // this endpoint. No polling here — that's the hook's responsibility.
-    await this.completionHandler.completeByDbId(sessionDbId);
-
-    logger.info('SESSION', 'Session completed via API', {
-      contentSessionId,
-      sessionDbId
-    });
-
-    res.json({ status: activeSession ? 'completed' : 'completed_db_only', sessionDbId });
-  });
-
  /**
   * Initialize session by contentSessionId (new-hook uses this)
   * POST /api/sessions/init
@@ -25,15 +25,14 @@ export class SessionCompletionHandler {
   * Finalize a session's persistent + broadcast state.
   *
   * Idempotent — safe to call twice. The worker calls this from the SDK-agent
-   * generator's finally-block (primary path), and the HTTP route
-   * POST /api/sessions/complete also calls it as a backward-compat shim.
-   * If the session is already marked completed in the DB, this is a no-op.
+   * generator's finally-block when work naturally completes. If the session
+   * is already marked completed in the DB, this is a no-op.
   *
   * This method intentionally does NOT touch the in-memory SessionManager map.
   * The generator's finally-block handles in-memory removal via
   * `removeSessionImmediate` (which cannot `await` the generator it's running
-   * inside); the HTTP route layers `deleteSession` on top for the case where
-   * the generator is still running and needs to be aborted.
+   * inside); explicit-delete callers layer `deleteSession` on top for the case
+   * where the generator is still running and needs to be aborted.
   */
  finalizeSession(sessionDbId: number): void {
    const sessionStore = this.dbManager.getSessionStore();
@@ -76,13 +75,11 @@ export class SessionCompletionHandler {
  }

  /**
-   * Complete session by database ID
-   * Used by DELETE /api/sessions/:id and POST /api/sessions/:id/complete
+   * Complete session by database ID. Used by explicit user-initiated deletion
+   * via DELETE /api/sessions/:id and POST /api/sessions/:id/complete (viewer UI).
   *
   * Calls `finalizeSession` (DB mark + drain + broadcast, idempotent) and then
-   * aborts any running SDK agent via `sessionManager.deleteSession`. The
-   * HTTP route wraps this so older callers that still POST to
-   * /api/sessions/complete keep working even after the worker self-cleans.
+   * aborts any running SDK agent via `sessionManager.deleteSession`.
   */
  async completeByDbId(sessionDbId: number): Promise<void> {
    // Finalize first so the DB and broadcast state are consistent even if
@@ -39,10 +39,10 @@ describe('GeminiCliHooksInstaller - event mapping', () => {
    expect(src).toContain("'SessionStart': 'context'");
  });

-  it('should map SessionEnd to session-complete (unchanged)', async () => {
+  it('should not map SessionEnd (worker self-completes; /clear must not drain queue)', async () => {
    const { readFileSync } = await import('fs');
    const src = readFileSync('src/services/integrations/GeminiCliHooksInstaller.ts', 'utf-8');
-    expect(src).toContain("'SessionEnd': 'session-complete'");
+    expect(src).not.toContain("'SessionEnd':");
  });
 });

@@ -18,7 +18,7 @@ describe('Hook Lifecycle - Event Handlers', () => {
      const { getEventHandler } = await import('../src/cli/handlers/index.js');
      const recognizedTypes = [
        'context', 'session-init', 'observation',
-        'summarize', 'session-complete', 'user-message', 'file-edit'
+        'summarize', 'user-message', 'file-edit'
      ];
      for (const type of recognizedTypes) {
        const handler = getEventHandler(type);
@@ -42,15 +42,6 @@ describe('Hook Lifecycle - Event Handlers', () => {
      expect(result.exitCode).toBe(0);
    });

-    it('should include session-complete as a recognized event type (#984)', async () => {
-      const { getEventHandler } = await import('../src/cli/handlers/index.js');
-      const handler = getEventHandler('session-complete');
-      // session-complete should NOT be the no-op handler
-      // We can verify this by checking it's not the same as an unknown type handler
-      expect(handler).toBeDefined();
-      // The real handler has different behavior than the no-op
-      // (it tries to call the worker, while no-op just returns immediately)
-    });
  });
 });