_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Code Review

This PR bundles 8 bug fixes. Overall the approach is solid, but there are a few issues worth addressing before merge.

Overview

• RestartGuard replaces a fragile flat counter with a time-windowed approach — good design
• SSEBroadcaster adds proactive dead-client cleanup — correct and well-structured
• FTS5 fallback when Chroma is disabled is a meaningful UX improvement
• OpenRouter URL and zh-TW mode are clean, minimal additions
• The viewer model dropdown and alias path fixes are needed but have bugs noted below

Issues

🔴 Bug — ContextSettingsModal.tsx: Custom model option corrupts settings

The custom model <option> has value="custom", not the actual model ID:

return <option value="custom">custom: {model}</option>;

If a user with a custom model ID (e.g. my-custom-model) triggers onChange, it saves the string "custom" to CLAUDE_MEM_MODEL, permanently overwriting their real model ID. The value should be the model ID itself:

return <option value={model}>custom: {model}</option>;

🟡 Dead code — RestartGuard.ts: applyRestartDecay is never called

applyRestartDecay and RESTART_DECAY_MS are defined but wired up nowhere. Either hook it into the worker's processing loop or remove it — as-is it's misleading documentation that suggests decay is active when it isn't.

🟡 Dynamic imports in hot path — worker-service.ts / SessionRoutes.ts

const { recordRestartAndCheckAllowed, resetRestartCounter } = await import('./worker/RestartGuard.js');

These imports are inside functions called on every session completion/crash. Bun/Node caches them after the first load so there's no real perf hit, but the pattern is unconventional for a module that's always needed. A top-level import would be cleaner and immediately shows the dependency.

🟡 SSEBroadcaster.dispose() not wired to shutdown

The new dispose() method is well-written but nothing in this PR calls it. If WorkerService doesn't call it on SIGTERM/SIGINT, the setInterval will prevent clean shutdown and accumulate on restarts. Worth verifying the shutdown path calls this.sseBroadcaster.dispose().

🟡 Shell alias quoting in smart-install.js

The generated alias embeds $() subshell expansion inside single quotes:

alias claude-mem='bun "$(ls -d ~/.claude/plugins/.../worker-service.cjs ... | sort -V | tail -1 || echo "...")"'

This works in bash (single quotes in an alias body are re-evaluated on execution), but it's fragile — zsh and fish behave differently, and the nested double-quoting inside $() is hard to read. A named function in .bashrc/.zshrc would be more portable and easier to update:

function claude-mem() { bun "$(ls ...)" "$@"; }

Minor Notes

• ChromaMcpManager.ts: httpx is added alongside httpcore but isn't mentioned in the PR description or issue [BUG] chroma-mcp: No module named 'httpcore' on all query/add operations #2046. Intentional? Worth noting.
• ContextSettingsModal.tsx: The knownModels array is duplicated in two IIFEs. Extracting it to a const above the JSX would reduce duplication and improve readability.
• RestartGuard.ts: consecutiveRestarts is still incremented alongside restartTimestamps for "legacy logging." If it's not driving any logic, it adds noise. Consider removing or clarifying in a comment why both are maintained.
• worker-types.ts: The consecutiveRestarts field JSDoc still says "Track consecutive restart attempts" without acknowledging that the real guard is now restartTimestamps. Minor docs drift.

What's Good

• RestartGuard.ts is well-factored — the 60s window + 10-cap is much more robust than a flat counter for long-running legitimate sessions
• SSEBroadcaster cleanup logic is thorough: handles close, error, periodic sweep, and inline broadcast cleanup
• SearchManager.ts FTS5 fallback correctly only shows the "install Chroma" message when FTS5 also returns nothing — good UX
• OpenRouterAgent.ts configurable URL follows the existing env → settings → default priority pattern consistently
• zh-TW mode mirrors the structure of other mode files cleanly

The critical custom-model-dropdown bug (#🔴 above) should be fixed before merge. The other items are improvements but not blockers.

Greptile Summary

This PR bundles eight bug fixes: precise model IDs in the viewer dropdown, SSE dead-client cleanup, configurable OpenRouter base URL, Traditional Chinese mode, version-agnostic CLI alias, time-windowed restart guard, FTS5 fallback when Chroma is disabled, and httpcore/httpx added to chroma-mcp uvx deps. Most changes are clean and well-scoped.

• P1 – ContextSettingsModal: The <option value=\"custom\"> has the literal string \"custom\" as its value; selecting it via onChange overwrites a user's custom model ID (e.g. an OpenRouter model string) with the word \"custom\", breaking API calls. The option value should be the actual model ID.

Confidence Score: 4/5

Safe to merge after fixing the custom-model ID overwrite in ContextSettingsModal

One P1 defect in ContextSettingsModal would silently corrupt a user's custom model setting when they interact with the dropdown. All other findings are P2 style/cleanup items. Everything else in the PR looks correct.

src/ui/viewer/components/ContextSettingsModal.tsx (custom option value bug)

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Session Generator Exit] --> B{pendingCount > 0?}
    B -- Yes --> C[recordRestartAndCheckAllowed]
    C -- allowed --> D[Restart Generator]
    C -- blocked --> E[terminateSession / abort]
    B -- No --> F[resetRestartCounter]
    F --> G[Remove Session]

    H[SSEBroadcaster] --> I{Client connects}
    I --> J[Add to sseClients Set]
    J --> K[Listen: close / error → removeClient]
    H --> L[setInterval 30s → cleanupDeadClients]
    H --> M[broadcast → inline dead-client prune]

    N[SearchManager text query] --> O{ChromaDB initialised?}
    O -- Yes --> P[Semantic search]
    O -- No --> Q[FTS5 fallback]
    Q --> R{Results found?}
    R -- Yes --> S[Return FTS5 results]
    R -- No --> T[chromaFailed = true → show install hint]

_{Reviews (1): Last reviewed commit: "fix: viewer model dropdown, SSE cleanup,..." | Re-trigger Greptile}

Code Review — PR #2069

This is a solid multi-fix PR. The individual changes are focused and well-scoped. A few issues worth addressing before merge, plus some minor suggestions.

Overview

Eight independent fixes bundled together: SSE leak cleanup, configurable OpenRouter URL, zh-TW language mode, version-agnostic alias, time-windowed restart guard, FTS5 fallback, httpcore dependency, and viewer model dropdown.

Issues

1. ContextSettingsModal.tsx — custom model option is selectable and will corrupt settings

<option value="custom">custom: {model}</option>

value="custom" means if a user clicks this dropdown item, updateSetting('CLAUDE_MEM_MODEL', 'custom') fires and saves the literal string "custom" as the model ID. The intent is read-only display of the current unknown model, but nothing prevents selection. Fix: add disabled to the option, or restructure the select to only show the live model value as an input field when it's custom.

2. RestartGuard.ts — off-by-one in the cap check

if (restartsInWindow > MAX_WINDOWED_RESTARTS) {  // blocks at 11, not 10

The PR description says "cap 10" but > 10 allows 11 restarts before blocking. Should be >= MAX_WINDOWED_RESTARTS to match the stated intent.

3. smart-install.js — PowerShell update regex is fragile

const updated = existingContent.replace(/function claude-mem \{[^\n]*\}\n?/, functionDef);

[^\n]* only matches single-line function bodies. If a user ever manually expanded the function to multi-line, the regex silently fails to update it. The old behavior (skip-if-exists) was at least safe. Consider matching function claude-mem as an anchor and replacing through the closing } with a multiline match, or logging a warning when the regex produces no substitution (updated === existingContent).

Minor Suggestions

4. RestartGuard.ts — redundant legacy counter

session.consecutiveRestarts is still incremented inside recordRestartAndCheckAllowed but is never used for guard logic anymore — only the timestamp array is. This creates two sources of truth. Either remove the legacy increment or add a note that it's kept only for logging.

5. worker-service.ts / SessionRoutes.ts — repeated dynamic imports

const { recordRestartAndCheckAllowed, resetRestartCounter } = await import('./worker/RestartGuard.js');

This pattern appears 4+ times in hot paths (session exit handlers). There's no meaningful code-splitting benefit here; these should be static imports at the top of each file. Dynamic imports inside callbacks also obscure the module graph.

6. ContextSettingsModal.tsx — IIFE inside JSX attribute

The value={(() => { ... })()} pattern is non-idiomatic React and harder to read. Extract the mapping logic to a named function or a const selectedModelValue = ... in the render body above the return.

What Looks Good

• SSEBroadcaster.ts: The error event handler + periodic cleanupDeadClients + inline cleanup during broadcast + dispose() is thorough. The layered defense is appropriate for a long-lived process.
• RestartGuard.ts: Extracting restart logic into its own module is the right call — eliminates the duplicated MAX_CONSECUTIVE_RESTARTS = 3 constants that lived in two separate route handlers.
• SearchManager.ts: Graceful FTS5 fallback when Chroma is absent is a good UX improvement. The condition to still set chromaFailed = true when FTS5 also returns nothing correctly preserves the install-suggestion message.
• OpenRouterAgent.ts: env > settings > default precedence for the base URL is the correct pattern.
• ChromaMcpManager.ts: Adding both httpcore and httpx to both the HTTP and persistent client args is correct.
• zh-TW mode: Placeholder strings are consistent with the existing locale pattern.

Summary

The SSE, FTS5 fallback, RestartGuard, and OpenRouter fixes are all clean. The three issues above — the selectable custom model option (data corruption risk), the off-by-one in RestartGuard, and the fragile PowerShell regex — are worth fixing before merge. The rest are style/cleanup suggestions.

Closing to start fresh from main — will redo fixes isolated in Docker container.