Skip to content

perf+fix: 大历史量下仪表盘不再卡顿;setup 不再吞掉你的 Claude 配置#43

Merged
moose-lab merged 4 commits into
mainfrom
claude/review-phase4-performance
Jun 12, 2026
Merged

perf+fix: 大历史量下仪表盘不再卡顿;setup 不再吞掉你的 Claude 配置#43
moose-lab merged 4 commits into
mainfrom
claude/review-phase4-performance

Conversation

@moose-lab

Copy link
Copy Markdown
Owner

用户视角:这次更新你会感受到什么

⚡ 会话历史越多,提速越明显

之前每次仪表盘轮询、每条 CLI 命令都把 ~/.claude/projects 下的所有 JSONL 从头到尾重新解析一遍(串行)——历史到几千个会话时,每次刷新都是数秒级 CPU+IO。现在扫描结果按文件(mtime+大小)缓存,只重读真正变化的文件,并以 8 路并发扫描;删除的文件自动清出缓存。日常使用下,仪表盘刷新从"每次全量解析"变成"几乎零开销"。

🛟 devlog setup-statusline / setup-tmux 不再破坏你的 Claude Code 配置

  • settings.json 损坏时不再被静默清空。 之前解析失败会回退成空对象,然后把只含 devlog 配置的文件写回去——你的 hooks、模型配置、权限设置全部丢失。现在会中止并提示修复,不动你的文件。
  • 每次写入前自动保留 .bak 备份。
  • 不再因为标准格式的 hooks({matcher, hooks:[...]})直接崩溃,这类条目原样保留。

🔑 旧式后端环境变量鉴权的会话不再"第二回合失灵"

agent-api-key(后端环境变量)模式启动的会话,首回合正常、续聊时却报 "API key is required"——因为持久化时丢失了 legacy 标记,被误当成浏览器 BYOK。现在标记随会话持久化,续聊正常。

范围与验证

  • review 条目:IM-23(增量发现缓存)、IM-7(legacy 鉴权模式持久化)、IM-16/IM-18(setup 安全)。
  • 3 个独立提交,各带回归测试(缓存命中/失效/剪枝、持久化往返、损坏文件中止 + .bak);每次提交经 pre-commit 全量门禁(typecheck + 全部测试)。
  • 后续跟进(review Phase 4 剩余):hooks 写入 schema(IM-17)、VCC/db-pool 缓存上限(IM-25/26)、前端共享数据层(IM-13/27/28/30)、看板乐观更新(IM-11)。

🤖 Generated with Claude Code

moose-lab and others added 3 commits June 12, 2026 11:13
IM-23: every dashboard poll (and every CLI command) re-parsed every
JSONL under ~/.claude/projects serially — multi-second CPU+IO per
request once history reaches a few thousand sessions. SessionMeta is
now cached per file and invalidated by mtime+size, unchanged files
skip parsing entirely, scans run with bounded concurrency (8), and
entries for deleted files are pruned each pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…auth mode

IM-7: legacy agent-api-key sessions resolve to mode anthropic-api-key
with usesLegacyEnvVar set, but the bare mode was what got persisted —
the next stored-config resolution treated the session as browser BYOK
and failed with 'API key is required' although preflight passed.
Sessions now persist 'agent-api-key' for legacy env-var runtimes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dard-format hooks

IM-18: a settings.json that failed to parse fell back to {} and the
setup commands overwrote the user's file with a devlog-only object —
silently destroying hooks, model config, and permissions. Corrupt
files now abort with an actionable message and every write keeps a
.bak of the previous content.

IM-16: setup-tmux crashed on standard-format hook entries
({matcher, hooks:[...]}) because h.command.includes assumed the flat
shape; those entries are now preserved untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown

Claude finished @moose-lab's task in 3m 38s —— View job


Code Review

  • Gather context
  • Read changed files
  • Analyze correctness, safety, types, and tests
  • Post findings

Findings

  • [should-fix] src/cli/lib/claude-settings.ts:38-43writeClaudeSettingsWithBackup is non-atomic. The sequence is: copyFileSync (backup) → writeFileSync (overwrite). A process kill between the two leaves settings.json partially written; the .bak is recovery evidence but requires manual cp settings.json.bak settings.json. Fix: write to ${path}.tmp then renameSync(tmp, path)rename(2) is atomic on POSIX when src/dst are on the same filesystem. Example:

    const tmp = `${path}.tmp`;
    writeFileSync(tmp, JSON.stringify(settings, null, 2) + "\n", "utf-8");
    renameSync(tmp, path);

    Fix this →

  • [nit] src/cli/commands/setup-statusline.ts:100 — The local Hook interface (type: string; command: string, both required) diverges from setup-tmux.ts's more permissive version (type?: string; command?: string; [key: string]: unknown). The ?. on h.command?.includes(...) (line 103) silently compensates at runtime, but passing a {matcher, hooks:[...]} entry through the Hook type is still technically unsound. Align both files to use the open-typed version.

  • [nit] src/core/discovery.ts:27scanCache is a module-level singleton. GET /api/devlog?command=stats calls discoverProjects() on every request. Two concurrent requests share the cache and both call pruneScanCache at the end. The first prune can evict an entry just inserted by the concurrent scan (because it wasn't in the first scan's seenFiles). This causes a benign spurious cache miss on the next request—no correctness bug, but it silently degrades the cache hit rate under bursty dashboard traffic. Worth a short comment noting this is expected/intentional.

  • [nit] src/core/discovery.ts:176-179Session.meta is a direct reference to CachedScan.meta. The code comment warns callers to treat it as read-only, but nothing structural enforces it. A future mutation in any consumer would corrupt the cache silently. Object.freeze(meta) at cache insertion time (scanCache.set(...)) would make the contract structural at zero runtime cost for this object shape.

Verdict: APPROVE

All three targeted bugs are real, correctly diagnosed, and fixed:

  • IM-23 cache: The (mtime, size) key is the standard make-style invalidation heuristic; the mapWithConcurrency worker pattern is correct under Node's cooperative scheduler (the next++ pre-await is safe). Pruning on deleted files works correctly.
  • IM-7 legacy auth: getPersistedSessionAuthMode cleanly separates persistence from resolution; the resolveStoredSessionRuntimeAuthConfig round-trip test covers the exact failure mode.
  • IM-16/18 settings safety: loadClaudeSettings returning a discriminated union instead of silently falling back to {} is the right API shape. Both setup commands abort early with a clear message rather than overwriting.

The should-fix (atomic write) is a real latent risk but extremely unlikely in practice, and the .bak provides recovery. The nits are minor. Nothing here warrants blocking the merge.

Comment thread src/cli/lib/claude-settings.ts Fixed
CodeQL flagged a TOCTOU between existsSync and copyFileSync in the
settings backup path; both load and backup now handle ENOENT directly
instead of pre-checking existence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown

Claude finished @moose-lab's task in 1m 24s —— View job


Code Review (follow-up — commit c90875e)

  • Gather context
  • Check what changed since last review
  • Read updated files
  • Analyze findings
  • Post review

What this push changed

c90875e touches only src/cli/lib/claude-settings.ts. It replaces the existsSync pre-checks with direct try/catch + ENOENT handling in both loadClaudeSettings and writeClaudeSettingsWithBackup. This correctly closes the CodeQL TOCTOU finding — the previous existsSync → copyFileSync sequence had a window where another process could create/remove the file between the check and the copy.


Findings

  • [should-fix] src/cli/lib/claude-settings.ts:58 — The non-atomic write from the previous review is still present. writeFileSync writes directly to settings.json; a process kill mid-write leaves a truncated/corrupt file (the .bak is recovery evidence but requires manual cp settings.json.bak settings.json). The TOCTOU fix in this commit is correct and distinct from this issue — they are independent risks. Recommend the write-to-temp + renameSync pattern as previously noted:

    const tmp = `${path}.tmp`;
    writeFileSync(tmp, JSON.stringify(settings, null, 2) + "\n", "utf-8");
    renameSync(tmp, path);

    Fix this →

  • [nit] src/cli/commands/setup-statusline.ts:100 — Local Hook interface (type: string; command: string, both required) still diverges from setup-tmux.ts's open-typed version. Unchanged since last review.

  • [nit] src/core/discovery.ts:46-50 — Concurrent pruneScanCache calls can evict entries just inserted by a parallel scan. Unchanged; a short comment noting the expected benign miss would help future readers.

What the TOCTOU fix gets right: both the load and backup paths now handle ENOENT inline instead of pre-checking with existsSync. The isFileMissing helper is clean and reusable. The non-ENOENT error path in writeClaudeSettingsWithBackup correctly re-throws. No new issues introduced.

Verdict: APPROVE

The TOCTOU (CodeQL finding) is correctly resolved. The remaining should-fix (atomic write) is a latent risk that's very unlikely to manifest in practice given the .bak safety net, and it was already noted in the previous review. Nothing here blocks the merge.

@moose-lab
moose-lab merged commit 9719ee8 into main Jun 12, 2026
4 checks passed
@moose-lab
moose-lab deleted the claude/review-phase4-performance branch June 12, 2026 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants