perf(cli): cache GitHub PR list in the daemon route with a 60s TTL#7705
Merged
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed and measured — the PR panel from #7683 takes ~8s to open because every open re-runs Direction: aligned. The Web Shell PR panel is a glanceable surface that gets opened repeatedly while working; caching the slow Size: not applicable — no core paths touched. Production logic is 51 lines (+50/-1 in Approach: the scope feels right. Closure-scoped per-workspace cache, single-flight coalescing, only-cache-successes, injectable TTL — each piece is needed and nothing extra is carried. The design doc update documents the caching rationale alongside the existing feature doc. No unrelated changes or drive-by refactors. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测并有实测数据——#7683 的 PR 面板每次打开都重跑 方向：对齐。Web Shell PR 面板是"瞟一眼"性质的界面，工作中会反复打开；用短 TTL 缓存慢速 规模：不适用——未触及核心路径。生产逻辑 51 行（+50/-1），测试 101 行，文档 17 行。 方案：范围合理。闭包级按 workspace 分键缓存、单飞合并、仅缓存成功结果、TTL 可注入——每一部分都有必要，没有多余的东西。设计文档更新记录了缓存的设计理由。无无关改动或顺手重构。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
Code ReviewIndependent proposal: for a slow Comparison: the PR matches this proposal almost exactly. The implementation is a clean adaptation of the No critical blockers found. Specific observations:
TestingThis is an unattended CI run — PR code is not executed here. Evidence comes from the PR's own CI checks on Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The main unit test suite ( The change is a latency optimization to an existing daemon route — response shape is unchanged, only the caching layer is new. A maintainer can trigger the isolated Not verified: first-open latency improvement (requires real 中文说明代码审查独立方案： 对于一个慢速 对比： PR 与这个方案几乎完全一致。实现是 未发现关键阻塞问题。具体观察：
测试这是无人值守 CI 运行——不在此执行 PR 代码。证据来自 PR 自身在 该变更是对现有 daemon 路由的延迟优化——响应形状不变，只是新增了缓存层。如需真实场景验证 PR 面板，维护者可触发隔离的 未验证：首次打开延迟改善（需真实 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 5/5 — Clean across every stage; would merge without hesitation. This is a textbook small perf PR: one measured problem (8s The design doc addition is a nice touch — it documents the caching rationale right where the feature is described, so the next person who wonders "why is this cached?" finds the answer in-place. Approval deferred until CI lands green on 中文说明置信度：5/5 —— 每个阶段都很干净；毫不犹豫地合并。 这是一个教科书式的小型 perf PR：一个有实测数据的问题（每次打开面板都要 8s 的 设计文档的补充也很好——在功能描述的地方记录了缓存的设计理由，下一个问"为什么要缓存？"的人能就地找到答案。 审批延迟至 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
added
category/performance
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
qwen-code-ci-bot
approved these changes
Jul 25, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
approved these changes
Jul 25, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
doudouOUC
reviewed
Jul 25, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
Comment on lines
+72
to
+74
| () => { | ||
| if (cache.get(workspaceCwd) === entry) cache.delete(workspaceCwd); | ||
| }, |
Collaborator
There was a problem hiding this comment.
[Suggestion] The promise rejection handler (this .then() callback) has no retry-after-rejection test. The pre-existing "falls back to the bridge error mapper on unexpected throws" test uses mockRejectedValue with a single request, and the new "does not cache a failed result" test covers the kind: 'failed' resolution path — but neither verifies that after fetchGitHubPullRequests rejects, the cache entry is cleared and a subsequent request re-invokes gh. If this cache.delete were missing, every request within the daemon's lifetime would replay the cached rejection.
Concrete cost: a regression in the rejection handler (e.g., a refactor that drops the cache.delete) would silently break recovery from transient gh crashes with no test to catch it.
Consider adding a test analogous to "does not cache a failed result" but using mockRejectedValueOnce(new Error('boom')) followed by mockResolvedValueOnce({ kind: 'ok', pullRequests: [PR] }), asserting the second request succeeds and fetchGitHubPullRequests was called twice.
— qwen3.7-max via Qwen Code /review
doudouOUC
reviewed
Jul 25, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| promise: Promise<FetchGitHubPullRequestsResult>; | ||
| settledAt: number | null; | ||
| } | ||
| const cache = new Map<string, PrsCacheEntry>(); |
Collaborator
There was a problem hiding this comment.
[Suggestion] Per-workspace PR cache has no eviction for removed workspaces — Failure scenario: workspace removal via removeWorkspace() does not propagate to this closure-scoped Map; orphaned entries (~few KB each) accumulate over months of add/remove cycles until daemon restart.
The WorkspaceRegistry has no removal event mechanism, so the simplest fix is a lazy sweep: in getPullRequests, delete entries whose settledAt is older than a generous max-age (e.g. 1 hour) before the freshness check. Alternatively, accept the leak as negligible and document it.
— qwen3.7-max via Qwen Code /review
Collaborator
🔍 Code Review3 files reviewed · 0 findings · LGTM Clean closure-scoped TTL cache following the same pattern as the git-status cache in #7680 (Map + in-flight dedup + cache-only-on-ok). Verified: TTL expiry, concurrent coalescing, stale-entry guard, non-ok eviction, and the Reviewed with Open Code Review + manual audit |
ytahdn
approved these changes
Jul 25, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. Clean 60s TTL cache with in-flight dedup, only-ok caching, and 4 solid tests. No findings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Adds a short-lived (60s) daemon-side cache for the Web Shell GitHub pull-request list. Opening the PR panel shells out to
gh pr listwith CI rollup, which makes multi-second GitHub round-trips — measured at ~8s cold for this repository, almost entirely network I/O. The panel is a glanceable surface, so a minute-old snapshot is perfectly acceptable. The cache is closure-scoped per workspace, coalesces concurrent opens onto a single in-flightghspawn, and only caches successful results:gh-missing / failure / not-a-repo responses clear the entry so the next open retries. The TTL is dependency-injected, following the existing usage-dashboard route's caching convention.Why it's needed
The PR panel from #7683 was slow to open: every open re-ran
gh pr list --json …,statusCheckRollup, fetching CI status for 30 PRs over the network (~8s). With the cache, repeat opens within the TTL are effectively instant, which matches how the panel is actually used (opened repeatedly while working).Reviewer Test Plan
How to verify
Start the daemon on a GitHub-backed repository and open the Web Shell PR panel (
/prs). The first open takes a few seconds (theghcall); close and reopen within a minute — the second open should be near-instant. Wait past 60s (or restart) and the next open re-fetches. Ifghis missing or fails, the panel still shows the appropriate error and the next open retries rather than serving a cached failure.Evidence (Before & After)
Measured against this repository via the daemon route:
gh)Tested on
Environment (optional)
Real daemon + real
ghagainst this repository for the timing evidence; four new route unit tests cover cache-hit, TTL-0 reload, no-cache-on-failure, and concurrent-coalescing.Risk & Scope
gh's own network time). A two-phase load (render the list fast, enrich CI icons asynchronously) would improve first open and is left as a follow-up.Linked Issues
N/A
中文说明
本 PR 做了什么
为 Web Shell 的 GitHub 拉取请求列表加一个短时效（60s）的 daemon 侧缓存。打开 PR 面板会 shell 出
gh pr list并带 CI rollup，需要多秒级 GitHub 往返——本仓库实测冷启动约 8s，几乎全是网络 I/O。面板是"瞟一眼"性质，一分钟内的快照完全可接受。缓存按 workspace 闭包级存放，并发打开合并到一次在飞的gh拉起，且只缓存成功结果：gh 缺失/失败/非仓库的响应会清掉条目，下次打开重试。TTL 依赖注入，对齐现有 usage-dashboard 路由的缓存约定。为什么需要
#7683 的 PR 面板打开慢：每次打开都重跑
gh pr list --json …,statusCheckRollup，为 30 个 PR 拉 CI 状态（约 8s）。加缓存后，TTL 内的重复打开几乎瞬时，符合面板的实际使用方式（工作中反复打开）。评审验证计划
如何验证
在 GitHub 仓库上启动 daemon，打开 Web Shell PR 面板（
/prs）。首次打开需几秒（gh 调用）；一分钟内关闭再打开——第二次应几乎瞬时。等过 60s（或重启）后下次打开会重新拉取。若 gh 缺失或失败，面板仍显示对应错误，且下次打开会重试而非返回缓存的失败。前后对照
本仓库经 daemon 路由实测：第 1 次（冷，走 gh）约 8.0s（前后相同）；第 2 次（缓存）从约 8.0s 降到约 0.003s；第 3 次约 0.001s。
测试平台
macOS 已测试；Windows、Linux 未单独验证（由 CI 覆盖）。
环境（可选）
真实 daemon + 真实 gh 指向本仓库做计时证据；新增 4 个路由单测覆盖缓存命中、TTL=0 重载、失败不缓存、并发合并。
风险与范围
关联 Issue
无