refactor(usage): split compact/reset buttons, fix 5 blockers

[BCDel89]

Summary

• Split the single action button into Compact (Discord sessions only) + Reset (all sessions) with separate confirm modals
• Added POST /api/usage/:sessionId/compact endpoint; reset no longer triggers compact first
• Bumped version to 1.0.39

Fixes from Terry's review

• Version bump — plugin.json + marketplace.json both at 1.0.39
• Cancel greyed out during in-flight ops — disabled = true on start, restored in cleanup
• Compact failure propagated to client — compactSessionById returns {success, message}; client alerts on failure
• Compact restricted to Discord sessions — web sessions only show Reset button
• Per-session concurrency lock — inFlightSessions Set prevents double-submit on same session

[TerrysPOV]

Thanks for taking this on — the JSONL path fix and the resetSessionById cache-coherence fix are both real bugs worth squashing, and the modal is a nice UX upgrade on window.confirm().

I'd like a few things addressed before I approve. I ran the in-tree 5-agent review plus an independent Codex review and they triangulated on the same set of concerns, so I'm fairly confident these are real.

Blockers

1. Version bumps not run. .claude-plugin/plugin.json and .claude-plugin/marketplace.json are both still at 1.0.38; master is at 1.1.2. Both plugin-version-guard and marketplace-version-guard will fail. CLAUDE.md has the note — run:

bun run bump:plugin-version
bun run bump:marketplace-version

2. Cancel doesn't actually cancel the server-side work. activeController.abort() aborts the client fetch, but src/ui/server.ts keeps awaiting resetSessionById, which still calls compactCurrentThreadSession → removeThreadSession. So if a user clicks Cancel mid-compact, the server can still finish compacting and delete the session — the UI just doesn't see the result. Either plumb an AbortSignal all the way through (accept it in resetSessionById, kill the spawned compact process in runCompact, skip removeThreadSession after abort), or remove the cancellation promise and grey out Cancel once the request is in flight. As written the UI lies about what Cancel does.

3. Compact failures silently become destructive reset success. compactCurrentThreadSession(session.threadId).catch(() => {}) swallows everything, then removeThreadSession() runs unconditionally. runCompact() in src/runner.ts:994 returns {success: false, message: ...} for failed compacts rather than throwing — so even a clean failure path leads straight to deletion with no signal to the user. The modal title says "Compact & Reset" and the status messages say "Compacting conversation history…" but if compact silently fails the conversation history is just dropped. Either bubble the compact result back to the client (let the user choose "compact failed — proceed with reset anyway?") or refuse to reset if compact failed.

4. Web session path skips compact while the modal says "Compact & Reset". For the global/web session (no threadId) the compact step is skipped entirely, but the modal title and status messages claim compacting. Either compact the web session too, or branch the modal copy so it reads "Reset" (not "Compact & Reset") when there's no threadId.

5. Concurrent resets bypass the per-thread enqueue. runCompact() calls runClaudeOnce(...) directly, not through the enqueue(fn, threadId) serial queue at src/runner.ts:262. Two browser tabs hitting reset on the same session can both pass listThreadSessions before either removes the row, then run concurrent /compact --resume against one Claude session. Worth routing through the existing queue so the existing concurrent-resume protection covers this path.

Secondary asks (would like in this PR)

6. Listener accumulation on modal re-open. Each click on a Reset button calls addEventListener('click', onOk/onCancel) without first removing any prior listeners. cleanup() removes them at the end, but if a user opens the modal from row A, then opens it from row B without clicking anything in between, you'll stack listeners. Track them on the elements and clean before re-binding.

7. Cache-Control: no-store is only on the success response. The error path goes through the generic json() helper without the header — a caching proxy could serve a stale error. Apply consistently.

8. findJSONLPath should filter to directories. readdir (no withFileTypes) plus existsSync will follow symlinks. If a symlink in ~/.claude/projects/ points to a directory containing <sessionId>.jsonl, you could pick up the wrong project's data for that UUID. Use readdir(..., { withFileTypes: true }) and dirent.isDirectory().

9. Unbounded fan-out scan. On cold /api/usage the worst case is active_sessions × all ~/.claude/projects subdirs + full JSONL reads. The 60s cache mitigates but degrades with project history size. Worth either bounding by name prefix (only dirs starting with -Users-…-claudeclaw-) or memoising the sessionId → dir map across requests.

10. No tests. Reset destroys data — at least a failure-path test (compact fails → does reset still happen / not happen?) would catch the silent-destruction concern in #3 if you change the behaviour.

Context

• The cache-coherence bug you're fixing is the same class flagged P1 in PR #147 for fallback sessions; routing through removeThreadSession() is exactly the pattern that review recommended. Good precedent following.
• PR #222 (auto-compact by turn count, currently CHANGES_REQUESTED) doesn't modify removeThreadSession or compactCurrentThreadSession signatures, so there's no hard merge conflict — just a soft semantic risk that an auto-compact-by-turns could fire on the same turn as a manual compact triggered by reset. Worth a thought once #222 lands.

Vision: aligned. Complexity: moderate (5 files, +298 lines, frontend + service layer + endpoint). Once 1–5 land I'll do another pass.

[TerrysPOV]

Thanks for the thorough rework — all 5 prior blockers cleared. Re-running the 5-agent review against a8faff6 plus an independent Codex pass surfaced 2 new blockers introduced by the refactor, plus the version bump is no longer ahead of master (PR #226 just landed and brought master to 1.0.39).

Code review

Found 3 issues:

1. onCompactCancel guard is inverted — the early-return fires precisely when the operation is in-flight, which is the only time a user would click Cancel. The comment above it says "close the modal and unblock the button", but the guard returns before cleanupCompact() runs, so the modal stays open and the Cancel button is effectively inert during the long-running compact. Either drop the guard or invert it (e.g. if (!inFlightSessions.has(sessionId)) would be the no-op path).

   claudeclaw/src/ui/page/script.ts

   Lines 1696 to 1710 in a8faff6

   	btn.textContent = "Compact";
   	};
   	var onCompactCancel = function() {
   	// Server-side compact continues; we just close the modal and unblock the button.
   	// Note: the operation cannot be truly cancelled once started.
   	if (inFlightSessions.has(sessionId)) return;
   	cleanupCompact();
   	};
   	var onCompactOk = function() {
   	inFlightSessions.add(sessionId);
   	compactOk.disabled = true;
   	compactOk.textContent = "Working…";
   	compactCancel.disabled = true;
   	if (compactProgress) compactProgress.classList.add("active");
   	startStatusCycle();

2. Web-session reset in resetSessionById only renames session.json to a backup but doesn't clear the in-memory session cache (current in src/sessions.ts). The runner reads getSession() and resumes from the cached current unless resetSession() is called — so after a web-session reset, a still-running runner process keeps using the old sessionId. The Discord branch correctly routes through removeThreadSession. Calling resetSession() (or an equivalent in-memory invalidation) in the web branch would mirror that.

   claudeclaw/src/ui/services/usage.ts

   Lines 166 to 184 in a8faff6

   	export async function resetSessionById(sessionId: string): Promise<void> {
   	if (!UUID_RE.test(sessionId)) throw new Error("invalid sessionId");
   	const cwd = process.cwd();
   	// Global web session
   	const sessionFile = join(cwd, ".claude", "claudeclaw", "session.json");
   	if (existsSync(sessionFile)) {
   	const data = JSON.parse(await readFile(sessionFile, "utf-8"));
   	if (data.sessionId === sessionId) {
   	const backup = join(cwd, ".claude", "claudeclaw", `session_backup_${Date.now()}.json`);
   	await rename(sessionFile, backup);
   	invalidateUsageCache();
   	return;
   	}
   	}
   	// Discord/job thread session — go through sessionManager to keep in-memory cache consistent
   	const threadSessions = await listThreadSessions();
   	const session = threadSessions.find((s) => s.sessionId === sessionId);

3. Version conflict — both plugin.json and marketplace.json bump to 1.0.39, but master is already at 1.0.39 after PR #226 merged this morning. Re-run bun run bump:plugin-version + bun run bump:marketplace-version so the bump lands ahead of master (1.0.40) and the guard CI checks pass.

   claudeclaw/.claude-plugin/plugin.json

   Lines 1 to 5 in a8faff6

   	{
   	"name": "claudeclaw",
   	"version": "1.0.39",
   	"description": "Cron-like daemon that runs Claude prompts on a schedule"
   	}

Smaller follow-ups (non-blocking, mention if convenient):

• .usage-compact-btn class is referenced in the rendered HTML but has no rule in styles.ts — the button will inherit defaults rather than match the .usage-reset-btn styling.
• compactSessionById has no server-side concurrency guard; the client-side inFlightSessions set is the only thing preventing concurrent compact subprocess spawn on the same session. A logged-in attacker (or runaway client loop) with a valid bearer token can bypass it.
• findJSONLPath uses sync existsSync inside an async loop. Minor consistency nit with the rest of the file.

Once (1), (2), and (3) are addressed I'll approve and merge.

[TerrysPOV]

Code review

Big improvement — all prior blockers from both earlier rounds are resolved in code. Verified the two round-2 ones directly: the inverted onCompactCancel guard is gone (cancel now just runs cleanupCompact()), and web reset now clears the in-memory cache (resetSession() + invalidateUsageCache(), with thread sessions routed through removeThreadSession). The inFlightSessions lock is balanced across success/error/network-failure paths, and the findSessionJsonlPath integration is correct.

Four items remain before this can merge:

1. Version is stale again — this branch bumps to 1.0.41, but master is now 1.0.43, and that's the only thing making the PR conflict (the usage/session code merges clean). Rebase on master and re-bump both .claude-plugin/*.json to 1.0.44.

2. compactSessionById doesn't invalidate the usage cache, unlike resetSessionById. After a successful compact, the 60s in-memory usageCache serves pre-compact data, so the compact looks like a no-op in the dashboard for up to a minute. One-line fix mirroring the reset path:

claudeclaw/src/ui/services/usage.ts

Lines 145 to 153 in fdf7162

	export async function compactSessionById(sessionId: string): Promise<{ success: boolean; message: string }> {
	if (!UUID_RE.test(sessionId)) throw new Error("invalid sessionId");
	const threadSessions = await listThreadSessions();
	const session = threadSessions.find((s) => s.sessionId === sessionId);
	if (!session) throw new Error("session not found or not a Discord session");
	return compactCurrentThreadSession(session.threadId);
	}

Compare the reset path, which invalidates on every branch:

claudeclaw/src/ui/services/usage.ts

Lines 164 to 177 in fdf7162

	const backup = join(cwd, ".claude", "claudeclaw", `session_backup_${Date.now()}.json`);
	await rename(sessionFile, backup);
	await resetSession(); // clear in-memory `current` cache so runner doesn't resume the stale sessionId
	invalidateUsageCache();
	return;
	}
	}
	// Discord/job thread session — go through sessionManager to keep in-memory cache consistent
	const threadSessions = await listThreadSessions();
	const session = threadSessions.find((s) => s.sessionId === sessionId);
	if (session) {
	await removeThreadSession(session.threadId);
	invalidateUsageCache();

3. POST /api/usage/:id/compact has no server-side concurrency guard. It spawns claude -p /compact --resume <sessionId> with no isMainBusy/session-level lock, so an HTTP compact can fire while a Discord turn is actively resuming the same sessionId — two processes resuming one session id, risking JSONL corruption or a forked session. The client inFlightSessions lock is per-tab and can't see the runner. Partly inherent to the existing compactCurrentThreadSession, but this PR makes it triggerable on demand — worth a guard (or an explicit acknowledgement of the race).

4. No tests. Reset renames/deletes session state, so at minimum a resetSessionById test (global + thread branches, and the deny-on-missing case) would lock in the destructive path. This was raised in earlier rounds and is still open across the 7 changed files.

Non-blocking nits: modal handlers addEventListener on each open without pre-removal, so opening one row's modal then another stacks listeners (cleanup only runs on click); and Cache-Control: no-store is set only on the /api/usage success path, not the error path.

[BCDel89]

Addressed all 4 + both nits:

1. Version — rebased onto current master (1.0.44), re-bumped to 1.0.45.

2. compactSessionById cache invalidation — now calls invalidateUsageCache() after compacting, mirroring resetSessionById.

3. The compact/message race — fixed directly rather than just acknowledged. runner.ts already has a per-thread promise queue (enqueue(fn, threadId)) that serializes every Discord/Telegram message run on a given thread. compactCurrentThreadSession() was calling runCompact() outside that queue entirely — that was the actual hole. It's now wrapped in the same enqueue(..., threadId) call, so an HTTP-triggered compact takes its turn in the same queue as live messages instead of racing them. This also closes the same race for the existing Discord /compact slash command and Telegram's compact path, since they share this function.

4. Tests — added src/__tests__/usage.test.ts (sandboxed subprocess pattern, same as jobs.test.ts, since usage.ts/sessionManager.ts/sessions.ts all resolve paths off process.cwd() at module load). Covers: global session backup+clear, thread session removal, not-found rejection, and invalid-sessionId rejection.

Nits:

• Modal stacking: clicking "Compact" on session A then session B before confirming/canceling A used to stack a second set of listeners on the shared confirm buttons. Both compact and reset flows now bail out if their modal is already open.
• /api/usage's error response now sets Cache-Control: no-store like the success path.

bun test: 138 pass, 3 pre-existing/unrelated failures (same env-dependent session-files.test.ts cases flagged in earlier rounds).