perf: stop v2 host-service from scaling linearly with worktree count#3999
Merged
Conversation
…ling Both maps previously had no eviction and grew monotonically with worktree count (searchIndexCache) and file event count (pathTypes). Adds LRU(12) + 30-min idle TTL to searchIndexCache and LRU(10k) cap to per-watcher pathTypes. Eviction in pathTypes loses only the directory-type hint; the next event for that path falls back to stat() (existing slow path). Measured (cache-and-paths-memory.bench.test.ts): - searchIndexCache @ 130 worktrees: 6.87 MB → 2.02 MB (-71%) - pathTypes @ 20k unique paths: 8.69 MB → 2.54 MB (-71%) - searchIndexCache cap holds at 12 entries, pathTypes at 10000 Adds findings audit + fix plan + reproduction tests + benchmarks for the broader v2 worktree-perf investigation. Notably, the host-service syncWorkspaceBranches 30s polling (1542 ms/tick at N=20 worktrees, real git subprocesses) is documented and reproduced but not fixed in this commit; follow-up PR will subscribe the pull-requests runtime to GitWatcher.onChanged. See plans/v2-paths-worktree-perf-findings.md for the full audit and plans/v2-paths-worktree-perf-fix-plan.md for the remaining work.
…notes Adds concrete pickup steps, app.ts wiring order, concurrency notes, and a mapping of which existing tests/benchmarks change vs stay. The next session (or fresh agent) implementing Fix #1 should be able to read the plan top-to-bottom and execute without re-deriving context.
Replaces the unconditional 30s `syncWorkspaceBranches` polling timer with a `GitWatcher.onChanged` subscription so idle worktrees cost ~0 git subprocesses regardless of N. Branch / HEAD / upstream changes are picked up at ~430 ms p50 (was up to 30 s). Lift `GitWatcher` to a standalone instance in `app.ts` so both `EventBus` (broadcasts to clients) and `PullRequestRuntimeManager` (event-driven branch sync) share one watcher. `syncWorkspaceBranches` becomes the safety-net sweep: still O(N) per call, but cadence drops from 30 s → 5 min. Project-level PR refresh interval also drops from 20 s → 5 min — branch changes drive their own `refreshProject` via `syncOneWorkspace`, so the polling is only there to catch external PR opens. Concurrency stays safe via the existing `inFlightProjects` guard. Workspace deletion races no-op cleanly via a fresh row lookup in `syncOneWorkspace`. Tests updated: - Existing scaling unit + integration tests now describe the safety-net sweep (still pin the O(N) per-call shape). - New integration test wires a real `GitWatcher` + `WorkspaceFilesystemManager` and asserts a `git commit` in 1/5 worktrees triggers exactly 4 git ops on 1 worktree. - Bench replaces "ms per polling tick" with event-to-DB-update latency (427 ms measured) plus the long-cadence safety-net sweep cost. Closes Fix #1 + Fix #4 in plans/v2-paths-worktree-perf-fix-plan.md.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughcreateApp now starts a single ChangesWorktree Performance Scaling — Event-Driven Sync & Memory Caps
Sequence DiagramsequenceDiagram
participant App
participant GitWatcher
participant PullReqMgr as PullRequestRuntimeManager
participant EventBus
participant DB
participant Git
App->>GitWatcher: start()
App->>PullReqMgr: start(gitWatcher)
App->>EventBus: start(gitWatcher)
GitWatcher->>PullReqMgr: onChanged(workspaceId)
PullReqMgr->>DB: select workspace row
PullReqMgr->>Git: rev-parse / symbolic-ref / remote
PullReqMgr->>DB: update workspace branch/head/upstream (if changed)
PullReqMgr->>PullReqMgr: refreshProject(projectId)
Note over PullReqMgr: periodic safety-net (5m) runs syncWorkspaceBranches()
PullReqMgr->>Git: git ops for all workspaces (sweep)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR eliminates three O(N-worktrees) steady-state costs in v2 host-service: the 30 s branch-sync polling loop is replaced by a
Confidence Score: 3/5Production code changes are sound; two P1 test bugs mean the CI signal for the critical event-driven path is weaker than it appears. The core performance fixes (GitWatcher injection, LRU caps) are well-implemented with correct lifecycle management. However, two P1 findings in the integration test silently use a 5 s timeout instead of the intended 10 s, which can cause the headline event-driven steady-state test to give false positives or flaky failures on slow runners, weakening confidence in the test coverage that justifies this change. packages/host-service/test/integration/pull-requests-scaling.integration.test.ts — two waitFor calls use wrong option key
|
| Filename | Overview |
|---|---|
| packages/host-service/src/app.ts | Lifts GitWatcher out of EventBus into a shared singleton; correct disposal order (pullRequestRuntime.stop → eventBus.close → gitWatcher.close → db.close). |
| packages/host-service/src/runtime/pull-requests/pull-requests.ts | Replaces 30 s polling timer with GitWatcher subscription; safety-net sweep at 5 min; extracts syncWorkspaceRow helper; stop() correctly clears all three handles. |
| packages/host-service/src/events/event-bus.ts | Accepts injected GitWatcher instead of constructing its own; removes redundant start/close calls; no issues found. |
| packages/workspace-fs/src/search.ts | Adds LRU+TTL cap (12 entries, 30 min) to searchIndexCache; patchSearchIndexesForRoot correctly bumps MRU; in-flight waiters skip the LRU bump (minor P2). |
| packages/workspace-fs/src/watch.ts | Caps pathTypes Map at 10 k entries with LRU eviction in normalizeEvent; delete events still shrink the map; logic is correct. |
| packages/host-service/test/integration/pull-requests-scaling.integration.test.ts | Two waitFor calls pass { timeout } instead of { timeoutMs }, silently using the 5 s default; can cause spurious failures in the event-driven steady-state test on slow CI. |
| packages/host-service/test/integration/pull-requests-scaling.bench.test.ts | New benchmark measuring event-to-DB latency and safety-net sweep cost; correctly uses { timeoutMs } throughout; no issues. |
| packages/workspace-fs/src/search-cache-eviction.test.ts | New LRU eviction tests covering cap, LRU bump, and explicit invalidation; well-structured. |
| packages/workspace-fs/src/watch-pathtypes-growth.test.ts | Integration tests reproducing pathTypes leak and verifying the 10 k cap; extensive real-fs coverage. |
Sequence Diagram
sequenceDiagram
participant FS as .git/ filesystem
participant GW as GitWatcher
participant EB as EventBus
participant PRM as PullRequestRuntimeManager
participant DB as SQLite DB
Note over GW: start() — watches all workspace .git/ dirs
GW-->>EB: onChanged subscription (broadcast git:changed to clients)
GW-->>PRM: onChanged subscription (event-driven branch sync)
FS->>GW: fs.watch event (commit / branch switch)
GW->>GW: debounce 300ms per workspace
GW->>EB: GitChangedEvent {workspaceId}
EB->>EB: broadcast git:changed to WebSocket clients
GW->>PRM: GitChangedEvent {workspaceId}
PRM->>DB: SELECT workspace WHERE id=workspaceId
PRM->>PRM: syncWorkspaceRow (git subprocesses)
PRM->>DB: UPDATE workspaces SET branch/headSha/upstream
PRM->>PRM: refreshProject (deduplicated by inFlightProjects)
Note over PRM: Safety-net sweep every 5 min
PRM->>DB: SELECT all workspaces
loop each workspace
PRM->>PRM: syncWorkspaceRow
end
Comments Outside Diff (1)
-
packages/workspace-fs/src/search.ts, line 322-325 (link)In-flight waiters don't refresh
lastAccessedAtWhen a second
getSearchIndexcall hits a key already insearchIndexBuilds, it returnsawait inFlightand never callsbumpAndReturnCachedIndex. The cache entry was just written withlastAccessedAt = Date.now()by the first caller's.then(), so in practice the timestamp is fresh. But conceptually the second caller did access the entry and its access time goes unrecorded, which could cause a spuriously early eviction under a full cache with tight TTL pressure.Consider fetching from cache and bumping after the
await inFlightresolves, similar to the cache-hit path.Prompt To Fix With AI
This is a comment left during a code review. Path: packages/workspace-fs/src/search.ts Line: 322-325 Comment: **In-flight waiters don't refresh `lastAccessedAt`** When a second `getSearchIndex` call hits a key already in `searchIndexBuilds`, it returns `await inFlight` and never calls `bumpAndReturnCachedIndex`. The cache entry was just written with `lastAccessedAt = Date.now()` by the first caller's `.then()`, so in practice the timestamp is fresh. But conceptually the second caller did access the entry and its access time goes unrecorded, which could cause a spuriously early eviction under a full cache with tight TTL pressure. Consider fetching from cache and bumping after the `await inFlight` resolves, similar to the cache-hit path. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/host-service/test/integration/pull-requests-scaling.integration.test.ts:326
**Wrong option key silently drops the custom timeout**
Both `waitFor` calls pass `{ timeout: 10_000 }`, but the locally-defined `waitFor` destructs `{ timeoutMs, pollMs }` — so `timeout` is an unknown key, `timeoutMs` falls back to its 5 s default, and the intended 10 s window is silently ignored. On a slow CI runner the predicate can easily exceed 5 s, causing a spurious failure in the critical event-driven steady-state test.
```suggestion
await waitFor(() => scenario.gitOpLog.length > 0, { timeoutMs: 10_000 });
```
### Issue 2 of 3
packages/host-service/test/integration/pull-requests-scaling.integration.test.ts:342-344
**Same `timeout` vs `timeoutMs` mismatch on the second call**
Same issue as the call above — `{ timeout: 10_000 }` is ignored; the effective timeout is the 5 s default.
```suggestion
await waitFor(() => scenario.gitOpLog.length > baselineLogLength, {
timeoutMs: 10_000,
});
```
### Issue 3 of 3
packages/workspace-fs/src/search.ts:322-325
**In-flight waiters don't refresh `lastAccessedAt`**
When a second `getSearchIndex` call hits a key already in `searchIndexBuilds`, it returns `await inFlight` and never calls `bumpAndReturnCachedIndex`. The cache entry was just written with `lastAccessedAt = Date.now()` by the first caller's `.then()`, so in practice the timestamp is fresh. But conceptually the second caller did access the entry and its access time goes unrecorded, which could cause a spuriously early eviction under a full cache with tight TTL pressure.
Consider fetching from cache and bumping after the `await inFlight` resolves, similar to the cache-hit path.
Reviews (1): Last reviewed commit: "perf(host-service): event-driven pull-re..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Wait until the initial sweep AND any startup-related GitWatcher | ||
| // events have fully drained — otherwise we'd snapshot mid-flush and | ||
| // see leftover ops from another workspace counted as "event-driven". | ||
| await waitFor(() => scenario.gitOpLog.length > 0, { timeout: 10_000 }); |
There was a problem hiding this comment.
Both waitFor calls pass { timeout: 10_000 }, but the locally-defined waitFor destructs { timeoutMs, pollMs } — so timeout is an unknown key, timeoutMs falls back to its 5 s default, and the intended 10 s window is silently ignored. On a slow CI runner the predicate can easily exceed 5 s, causing a spurious failure in the critical event-driven steady-state test.
Suggested change
| await waitFor(() => scenario.gitOpLog.length > 0, { timeout: 10_000 }); | |
| await waitFor(() => scenario.gitOpLog.length > 0, { timeoutMs: 10_000 }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/test/integration/pull-requests-scaling.integration.test.ts
Line: 326
Comment:
**Wrong option key silently drops the custom timeout**
Both `waitFor` calls pass `{ timeout: 10_000 }`, but the locally-defined `waitFor` destructs `{ timeoutMs, pollMs }` — so `timeout` is an unknown key, `timeoutMs` falls back to its 5 s default, and the intended 10 s window is silently ignored. On a slow CI runner the predicate can easily exceed 5 s, causing a spurious failure in the critical event-driven steady-state test.
```suggestion
await waitFor(() => scenario.gitOpLog.length > 0, { timeoutMs: 10_000 });
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+342
to
+344
| await waitFor(() => scenario.gitOpLog.length > baselineLogLength, { | ||
| timeout: 10_000, | ||
| }); |
There was a problem hiding this comment.
timeout vs timeoutMs mismatch on the second call
Same issue as the call above — { timeout: 10_000 } is ignored; the effective timeout is the 5 s default.
Suggested change
| await waitFor(() => scenario.gitOpLog.length > baselineLogLength, { | |
| timeout: 10_000, | |
| }); | |
| await waitFor(() => scenario.gitOpLog.length > baselineLogLength, { | |
| timeoutMs: 10_000, | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/test/integration/pull-requests-scaling.integration.test.ts
Line: 342-344
Comment:
**Same `timeout` vs `timeoutMs` mismatch on the second call**
Same issue as the call above — `{ timeout: 10_000 }` is ignored; the effective timeout is the 5 s default.
```suggestion
await waitFor(() => scenario.gitOpLog.length > baselineLogLength, {
timeoutMs: 10_000,
});
```
How can I resolve this? If you propose a fix, please make it concise.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/workspace-fs/src/watch.ts (1)
478-493:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftEvicting directory hints can leave stale descendant search entries.
When a directory ages out of
pathTypes, a later delete for that path falls back toisDirectory = false. Downstream,patchSearchIndexesForRoot()only deletes the exact path in that case, so files under the deleted directory can remain searchable until a full rebuild. Please keep directory hints out of the LRU, or add a conservative “unknown type” path that invalidates the root on delete/rename cache misses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/workspace-fs/src/watch.ts` around lines 478 - 493, The LRU currently evicts directory hints (state.pathTypes) which causes deletes to fall back to isDirectory = false and leaves descendant search entries; update the logic in the watch handler around stat/state.pathTypes/PATH_TYPES_MAX so directories are not tracked in the LRU (e.g., keep directories in a separate non-evicting map or never delete entries where isDirectory === true) or implement a three-state value (true/false/unknown) in state.pathTypes and on a cache miss or unknown treat the path as “unknown type” and call patchSearchIndexesForRoot() to conservatively invalidate the whole root on delete/rename; adjust the stat/eviction code that currently does state.pathTypes.delete(...), size check against PATH_TYPES_MAX, and state.pathTypes.set(...) to follow one of these approaches and ensure downstream callers of state.pathTypes read the new semantic (unknown -> invalidate root).
🧹 Nitpick comments (2)
packages/host-service/test/integration/pull-requests-scaling.bench.test.ts (1)
36-49: ⚡ Quick winPromote this proxy helper and remove the
anyescape hatch.This
instrumentGit()proxy is duplicated inpackages/host-service/test/integration/pull-requests-scaling.integration.test.ts, and both copies suppressnoExplicitAny. Moving it into a shared test helper with a typed wrapper overraw/revparse/remotewill keep the suites in sync and make signature drift easier to catch.As per coding guidelines "Avoid using
anytype; prefer explicit type safety in TypeScript." and "If a utility is used 2+ times, promote it to the highest shared parent's directory or top-levelcomponents/."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/test/integration/pull-requests-scaling.bench.test.ts` around lines 36 - 49, Move the duplicated instrumentGit proxy into a shared test helper and replace the any escape hatch by explicitly typing the wrapped methods: keep the function name instrumentGit(realGit: SimpleGit, counter: OpCounter) and implement a typed wrapper for the three targeted members (raw, revparse, remote) so the proxy returns the exact SimpleGit method signatures (e.g., raw(...args: string[]): Promise<string> or the actual return types from SimpleGit) instead of using (target as any); export this helper from a common test utilities module so both integration and bench tests import it, and remove the biome-ignore/noExplicitAny comment.packages/host-service/src/events/event-bus.test.ts (1)
15-17: ⚡ Quick winMake the GitWatcher unsubscribe observable in this test.
EventBus.close()now owns listener cleanup for the shared watcher, but this stub always returns an anonymous noop, so the new contract is never asserted. Return a namedmock(() => {})here and expect it aftereventBus.close().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/events/event-bus.test.ts` around lines 15 - 17, Replace the anonymous noop returned by the GitWatcher stub with a named mock so the test can assert it was unsubscribed; specifically, change the gitWatcher.onChanged stub to return a named jest mock (e.g., unsubscribe = jest.fn()) and wire gitWatcher.onChanged to return that mock, then call eventBus.close() and add an assertion that the mock unsubscribe was invoked (expect(unsubscribe).toHaveBeenCalled() or similar) to verify EventBus.close() cleans up the shared watcher. Reference: GitWatcher, gitWatcher.onChanged, EventBus.close / eventBus.close.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/host-service/src/runtime/pull-requests/pull-requests.ts`:
- Around line 248-250: The git-change handler can start concurrent
syncOneWorkspace() calls for the same workspace; fix by serializing
per-workspace work using a Map (e.g., workspaceSyncs: Map<string,
Promise<void>>) or a simple per-workspace lock. In the gitWatcher.onChanged
callback (the unsubscribeFromGitWatcher registration) look up the current
promise for event.workspaceId (or use Promise.resolve()), then replace it with
prev.finally(() => syncOneWorkspace(event.workspaceId).catch(err => { /* log */
})) so each call runs after the prior finishes; ensure you store the new promise
back into the map and remove the entry on completion (or on error) to avoid
leaks. Apply the same pattern to the other handler referenced (the block around
syncOneWorkspace at lines 355-367) so all workspace syncs are ordered.
In `@packages/workspace-fs/src/search.ts`:
- Around line 317-319: The cache hit path is returning cached entries
unconditionally; update the logic around searchIndexCache.get(cacheKey) and
bumpAndReturnCachedIndex(cacheKey, cached) to first verify the entry's
TTL/expiry (e.g., stored timestamp or expiry field on the cached object) and
treat expired entries as a miss so the index is rebuilt and re-cached; apply the
same TTL check to the other hit path referenced at lines 327-334 so no stale
index is served past the TTL.
In `@plans/v2-paths-worktree-perf-fix-plan.md`:
- Around line 352-354: Update the acceptance criteria in the plan to match the
actual tests that landed: replace the misleading "idle tick" wording with a
description that
packages/host-service/test/integration/pull-requests-scaling.integration.test.ts
retains the safety-net sweep assertions and adds a single-workspace event-driven
sync case; also clarify that pull-requests-scaling.bench.test.ts now reports "ms
per real change" instead of ~0 ms steady-state cost and that
cache-and-paths-memory.bench.test.ts shows plateau heap deltas (~0.6 MB cache
cap, ~4.3 MB pathTypes cap) independent of input size. Edit the bullet list in
plans/v2-paths-worktree-perf-fix-plan.md to use these exact test file names and
the corrected descriptions so the checklist aligns with the current suite.
---
Outside diff comments:
In `@packages/workspace-fs/src/watch.ts`:
- Around line 478-493: The LRU currently evicts directory hints
(state.pathTypes) which causes deletes to fall back to isDirectory = false and
leaves descendant search entries; update the logic in the watch handler around
stat/state.pathTypes/PATH_TYPES_MAX so directories are not tracked in the LRU
(e.g., keep directories in a separate non-evicting map or never delete entries
where isDirectory === true) or implement a three-state value
(true/false/unknown) in state.pathTypes and on a cache miss or unknown treat the
path as “unknown type” and call patchSearchIndexesForRoot() to conservatively
invalidate the whole root on delete/rename; adjust the stat/eviction code that
currently does state.pathTypes.delete(...), size check against PATH_TYPES_MAX,
and state.pathTypes.set(...) to follow one of these approaches and ensure
downstream callers of state.pathTypes read the new semantic (unknown ->
invalidate root).
---
Nitpick comments:
In `@packages/host-service/src/events/event-bus.test.ts`:
- Around line 15-17: Replace the anonymous noop returned by the GitWatcher stub
with a named mock so the test can assert it was unsubscribed; specifically,
change the gitWatcher.onChanged stub to return a named jest mock (e.g.,
unsubscribe = jest.fn()) and wire gitWatcher.onChanged to return that mock, then
call eventBus.close() and add an assertion that the mock unsubscribe was invoked
(expect(unsubscribe).toHaveBeenCalled() or similar) to verify EventBus.close()
cleans up the shared watcher. Reference: GitWatcher, gitWatcher.onChanged,
EventBus.close / eventBus.close.
In `@packages/host-service/test/integration/pull-requests-scaling.bench.test.ts`:
- Around line 36-49: Move the duplicated instrumentGit proxy into a shared test
helper and replace the any escape hatch by explicitly typing the wrapped
methods: keep the function name instrumentGit(realGit: SimpleGit, counter:
OpCounter) and implement a typed wrapper for the three targeted members (raw,
revparse, remote) so the proxy returns the exact SimpleGit method signatures
(e.g., raw(...args: string[]): Promise<string> or the actual return types from
SimpleGit) instead of using (target as any); export this helper from a common
test utilities module so both integration and bench tests import it, and remove
the biome-ignore/noExplicitAny comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71013585-671a-4a99-a433-244f45ec1272
📒 Files selected for processing (16)
packages/host-service/src/app.tspackages/host-service/src/events/event-bus.test.tspackages/host-service/src/events/event-bus.tspackages/host-service/src/events/index.tspackages/host-service/src/runtime/pull-requests/pull-requests.tspackages/host-service/test/integration/pull-requests-scaling.bench.test.tspackages/host-service/test/integration/pull-requests-scaling.integration.test.tspackages/host-service/test/pull-requests-scaling.test.tspackages/host-service/test/pull-requests.test.tspackages/workspace-fs/src/cache-and-paths-memory.bench.test.tspackages/workspace-fs/src/search-cache-eviction.test.tspackages/workspace-fs/src/search.tspackages/workspace-fs/src/watch-pathtypes-growth.test.tspackages/workspace-fs/src/watch.tsplans/v2-paths-worktree-perf-findings.mdplans/v2-paths-worktree-perf-fix-plan.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 16 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/workspace-fs/src/watch-pathtypes-growth.test.ts">
<violation number="1" location="packages/workspace-fs/src/watch-pathtypes-growth.test.ts:234">
P2: The cap/eviction test can assert too early: waiting for 95% create events does not guarantee `cap-0.tmp` has actually been evicted yet, so this test can fail nondeterministically under slower/coalesced watcher delivery.</violation>
</file>
<file name="packages/workspace-fs/src/search.ts">
<violation number="1" location="packages/workspace-fs/src/search.ts:319">
P2: Idle TTL is bypassed on cache hits, so entries older than 30 minutes are still reused instead of expiring.</violation>
</file>
<file name="packages/host-service/test/integration/pull-requests-scaling.integration.test.ts">
<violation number="1" location="packages/host-service/test/integration/pull-requests-scaling.integration.test.ts:326">
P2: `waitFor` options use `timeout` instead of `timeoutMs`, so this call silently falls back to the 5s default timeout and can make the integration test flaky under slower CI runs.</violation>
<violation number="2" location="packages/host-service/test/integration/pull-requests-scaling.integration.test.ts:343">
P2: This `waitFor` call also uses `timeout` instead of `timeoutMs`, so the intended 10s wait is not applied.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…test timeout - search.ts: enforce idle TTL on cache hits — previously a hot key was bumped forever and only sibling-key misses ran the TTL sweep, so a 30+ min idle entry would still be served stale on next access. Now the hit path checks freshness and rebuilds when expired. - pull-requests.ts: serialize syncOneWorkspace per workspaceId via Map<workspaceId, Promise>. GitWatcher only debounces 300 ms; two bursts far enough apart could run concurrent git reads and let the slower write clobber the newer snapshot. - pull-requests-scaling.integration.test.ts: fix `timeout` → `timeoutMs` in two waitFor calls. The wrong key was silently dropped, falling back to the 5 s default and risking flake on slow CI.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/host-service/src/runtime/pull-requests/pull-requests.ts`:
- Around line 243-257: The startup sweep and safety-net timer currently call
syncWorkspaceBranches()/syncWorkspaceRow() directly which can race with
watcher-triggered syncs; instead, route those operations through the existing
workspace sync queue by calling enqueueWorkspaceSync(workspaceId) for each
workspace rather than invoking syncWorkspaceBranches or syncWorkspaceRow
directly. Update the initial call site that uses this.syncWorkspaceBranches and
the safetyNetTimer callback to iterate workspaces and call
this.enqueueWorkspaceSync(...) (and replace any direct calls to
this.syncWorkspaceRow(...) at the noted spot) so all syncs are serialized by the
workspaceSyncQueue and deduplicated by inFlightProjects.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 50c76720-7367-40b4-a793-a23c71fde220
📒 Files selected for processing (3)
packages/host-service/src/runtime/pull-requests/pull-requests.tspackages/host-service/test/integration/pull-requests-scaling.integration.test.tspackages/workspace-fs/src/search.ts
✅ Files skipped from review due to trivial changes (1)
- packages/host-service/test/integration/pull-requests-scaling.integration.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/workspace-fs/src/search.ts
Replaces the linear promise chain with a "running + rerun pending" flag so N events for the same workspace collapse into at most one running sync + one queued rerun. Since each sync reads fresh state, queuing additional redundant syncs adds no value — it just wastes git subprocesses. Bounded under sustained watcher noise (long interactive rebase, bulk ref churn), where the previous chain could pile up dozens of sequential no-op syncs.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/host-service/src/runtime/pull-requests/pull-requests.ts (1)
343-358:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRoute the safety-net/startup sweep through the workspace queue.
Line 348 calls
syncWorkspaceRow()directly, which bypassesenqueueWorkspaceSync()ordering. That allows sweep writes to interleave with watcher-triggered syncs for the same workspace and can persist an older snapshot last.Suggested fix
private async syncWorkspaceBranches(): Promise<void> { - const allWorkspaces = this.db.select().from(workspaces).all(); - const changedProjectIds = new Set<string>(); - - for (const workspace of allWorkspaces) { - const projectId = await this.syncWorkspaceRow(workspace); - if (projectId) changedProjectIds.add(projectId); - } - - // Branch changes use the shared 60s cache rather than bypassing it. - // The next refreshEligibleProjects tick will pick up newly-opened PRs; - // up to TTL_MS lag on attaching a brand-new external PR is acceptable - // and keeps high-churn workspaces from multiplying GraphQL traffic. - await Promise.all( - [...changedProjectIds].map((projectId) => this.refreshProject(projectId)), - ); + const allWorkspaces = this.db + .select({ id: workspaces.id }) + .from(workspaces) + .all(); + + for (const workspace of allWorkspaces) { + await this.enqueueWorkspaceSync(workspace.id); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/runtime/pull-requests/pull-requests.ts` around lines 343 - 358, The startup sweep currently calls syncWorkspaceRow() directly inside syncWorkspaceBranches(), bypassing enqueueWorkspaceSync() and its ordering; change the loop to route each workspace through this.enqueueWorkspaceSync(workspace.id) (awaiting/enqueuing its returned promise) and collect any returned projectId into changedProjectIds so enqueueWorkspaceSync's ordering and deduping are respected; keep the later Promise.all([...changedProjectIds].map(id => this.refreshProject(id))) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/host-service/src/runtime/pull-requests/pull-requests.ts`:
- Around line 343-358: The startup sweep currently calls syncWorkspaceRow()
directly inside syncWorkspaceBranches(), bypassing enqueueWorkspaceSync() and
its ordering; change the loop to route each workspace through
this.enqueueWorkspaceSync(workspace.id) (awaiting/enqueuing its returned
promise) and collect any returned projectId into changedProjectIds so
enqueueWorkspaceSync's ordering and deduping are respected; keep the later
Promise.all([...changedProjectIds].map(id => this.refreshProject(id)))
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 007ce433-9da2-44d4-8c4c-c77f5c51d0b1
📒 Files selected for processing (1)
packages/host-service/src/runtime/pull-requests/pull-requests.ts
…ap test Splits WatcherState.pathTypes into filePaths (LRU-capped at 10k) and directoryPaths (uncapped Set). Pre-fix, the unified Map could LRU-evict a directory hint, after which a delete event for that directory fell back to isDirectory=false and patchSearchIndexesForRoot only pruned the exact path — leaving descendant search-index entries stale until the next full rebuild. Directory count per worktree is bounded by repo structure (O(100s) even for huge repos), so tracking them uncapped is fine; only the file-path stream grows unboundedly. Also fixes the cap-eviction test which was polling on a 95% event-count threshold (10_000 cap × 95% = 9,690 events, which can land before eviction triggers and stall under coalesced delivery). Now polls on the actual eviction outcome — `pathTypes.has(cap-0.tmp) === false` — and asserts cap on filePaths.size directly via a new getFilePathsSize helper. Bench predicate is similarly capped at min(target, FILE_PATHS_MAX) to avoid spinning the deadline once size plateaus.
The serialization queue added previously only covered the watcher-driven path; `syncWorkspaceBranches` (initial startup sweep + 5-min safety net) still called `syncWorkspaceRow` directly, so it could race a concurrent watcher-triggered sync for the same workspace and clobber newer state. The sweep now iterates ids and routes each through enqueueWorkspaceSync, which coalesces — if a watcher sync is already running for a workspace, the sweep just flips rerunPending and awaits the running promise. Sequential per-workspace iteration matches the original sweep's git-subprocess concurrency profile. Test mocks override syncOneWorkspace to bypass the drizzle .where() chain, since the sweep now performs a per-workspace row lookup that doesn't compose cleanly with the existing chained mock structure.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/workspace-fs/src/watch-pathtypes-growth.test.ts (2)
232-233: ⚡ Quick winAvoid hard-coding the cap in the test.
PATH_TYPES_MAX = 10_000duplicates the production knob fromwatch.ts. If that cap changes, this test will drift silently. Export the cap, or expose a test-only accessor, so the assertion stays coupled to the real limit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/workspace-fs/src/watch-pathtypes-growth.test.ts` around lines 232 - 233, The test hard-codes PATH_TYPES_MAX (10_000) causing it to diverge from production; instead, expose the real cap from the module that enforces it (watch.ts) and import it in the test (e.g., export a constant like PATH_TYPES_MAX or a test-only accessor such as getPathTypesLimit) so the test uses that exported symbol rather than a literal; update the test to reference the exported name (e.g., PATH_TYPES_MAX from watch.ts) and compute total = exportedLimit + 200 to keep the assertion coupled to the real limit.
110-110: Replace event-count polling with state-based waits for test stability.These tests poll event counts (
events.length,events.filter().length,createCount), but@parcel/watcherdoes not guarantee consistent event delivery across platforms—backends coalesce, batch, and throttle differently. The cap test in this same file (lines 243–246) already demonstrates the better pattern: polling on actual state viagetPathTypes()/getFilePathsSize(). Use the same approach at lines 110, 142, 150–152, 182–184, 193–195, 208–210, and 290 to eliminate backend sensitivity and improve reliability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/workspace-fs/src/watch-pathtypes-growth.test.ts` at line 110, Tests currently poll event arrays (waitForCondition(() => events.length >= ...), events.filter(...), createCount) which is flaky across watcher backends; replace those waits with state-based waits against the filesystem helpers used elsewhere. Locate uses of waitForCondition that reference events, events.filter, or createCount and instead call the existing getPathTypes() or getFilePathsSize() helpers inside the wait predicate (e.g., waitForCondition(() => getPathTypes(root).length >= expected) or waitForCondition(() => getFilePathsSize(root) >= expected)) so the tests assert actual FS state rather than event delivery. Also remove/stop relying on the events array for test success assertions and adapt assertions to read from getPathTypes()/getFilePathsSize() where needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/workspace-fs/src/watch-pathtypes-growth.test.ts`:
- Around line 34-40: The afterEach cleanup currently only removes tempRoots and
can leave live watchers running if a test throws; modify the tests so watcher
subscriptions/manager handles are always cleaned up on failure by either
tracking watcher unsubscribe functions (or manager instances) in a shared array
and calling their cleanup inside afterEach alongside the tempRoots fs.rm logic,
or by wrapping each test body that calls waitForCondition() / startWatcher() /
createWatcherManager() in try/finally and calling the corresponding
unsubscribe/close method in the finally block; update references to tempRoots,
afterEach, waitForCondition, and any watcher-related symbols (e.g.,
watcher.unsubscribe, manager.close, stopWatching) across the test blocks
mentioned (lines ~34-297) so no live watcher remains when fs.rm runs.
In `@packages/workspace-fs/src/watch.ts`:
- Around line 492-505: normalizeEvent() currently awaits stat(absolutePath)
before updating the LRU Maps, so concurrent calls from flushPendingEvents()
(Promise.all) can reorder mutations and break LRU eviction; fix by moving the
state mutations to occur before the async stat (or otherwise serializing
mutations), i.e., reserve/update state.filePaths and state.directoryPaths and
perform the LRU eviction (using FILE_PATHS_MAX and deleting oldestKey)
synchronously in normalizeEvent() prior to awaiting stat(absolutePath), or
implement a simple mutation queue/lock so state mutations for normalizeEvent()
run in watcher-event order instead of stat-completion order.
---
Nitpick comments:
In `@packages/workspace-fs/src/watch-pathtypes-growth.test.ts`:
- Around line 232-233: The test hard-codes PATH_TYPES_MAX (10_000) causing it to
diverge from production; instead, expose the real cap from the module that
enforces it (watch.ts) and import it in the test (e.g., export a constant like
PATH_TYPES_MAX or a test-only accessor such as getPathTypesLimit) so the test
uses that exported symbol rather than a literal; update the test to reference
the exported name (e.g., PATH_TYPES_MAX from watch.ts) and compute total =
exportedLimit + 200 to keep the assertion coupled to the real limit.
- Line 110: Tests currently poll event arrays (waitForCondition(() =>
events.length >= ...), events.filter(...), createCount) which is flaky across
watcher backends; replace those waits with state-based waits against the
filesystem helpers used elsewhere. Locate uses of waitForCondition that
reference events, events.filter, or createCount and instead call the existing
getPathTypes() or getFilePathsSize() helpers inside the wait predicate (e.g.,
waitForCondition(() => getPathTypes(root).length >= expected) or
waitForCondition(() => getFilePathsSize(root) >= expected)) so the tests assert
actual FS state rather than event delivery. Also remove/stop relying on the
events array for test success assertions and adapt assertions to read from
getPathTypes()/getFilePathsSize() where needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 466d0053-26cf-4f27-8fa3-a56f38ee3081
📒 Files selected for processing (3)
packages/workspace-fs/src/cache-and-paths-memory.bench.test.tspackages/workspace-fs/src/watch-pathtypes-growth.test.tspackages/workspace-fs/src/watch.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/workspace-fs/src/cache-and-paths-memory.bench.test.ts
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/host-service/test/pull-requests.test.ts (1)
154-162:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the warning payload, not just that a warning happened.
toHaveBeenCalled()will pass for anyconsole.warn, so an unrelated warning could satisfy the test even if the HEAD-failure path regressed. Check the message or error object to pin the assertion to the expected failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/test/pull-requests.test.ts` around lines 154 - 162, The test currently only checks that console.warn was called which can be triggered by unrelated warnings; update the assertion to verify the actual warning payload from the HEAD-failure path. After calling manager.syncWorkspaceBranches(), assert that the warnSpy (mocking console.warn) was called with the expected message or error object (e.g., using warnSpy.toHaveBeenCalledWith(...) or inspecting warnSpy.mock.calls[0][0]) that indicates the HEAD failure, while keeping the existing checks for setMock and refreshProjectMock unchanged; reference the syncWorkspaceBranches invocation and warnSpy/console.warn to locate where to update the assertion.
🧹 Nitpick comments (1)
packages/host-service/test/pull-requests.test.ts (1)
60-75: ⚡ Quick winKeep this test on the production sync path.
Overriding
syncOneWorkspacehere hardcodes the row and bypasses the new queue/lookup layer, so the test no longer catches regressions in workspace selection, coalescing, or serialization. Prefer a stub that still lets the realsyncOneWorkspacepath run, or at least assert the workspace ID passed into it.Also applies to: 138-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/test/pull-requests.test.ts` around lines 60 - 75, The test currently replaces syncOneWorkspace with a hardcoded implementation that calls syncWorkspaceRow directly and bypasses the real queue/lookup/selection logic; instead, capture the original syncOneWorkspace, replace it with a thin wrapper that asserts the workspace ID passed in (or records it) and then calls the original implementation so the production sync path still runs; reference syncOneWorkspace and syncWorkspaceRow (and the alternative occurrence around the other block) and apply the same wrapper pattern there rather than hardcoding the row.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/host-service/test/pull-requests.test.ts`:
- Around line 154-162: The test currently only checks that console.warn was
called which can be triggered by unrelated warnings; update the assertion to
verify the actual warning payload from the HEAD-failure path. After calling
manager.syncWorkspaceBranches(), assert that the warnSpy (mocking console.warn)
was called with the expected message or error object (e.g., using
warnSpy.toHaveBeenCalledWith(...) or inspecting warnSpy.mock.calls[0][0]) that
indicates the HEAD failure, while keeping the existing checks for setMock and
refreshProjectMock unchanged; reference the syncWorkspaceBranches invocation and
warnSpy/console.warn to locate where to update the assertion.
---
Nitpick comments:
In `@packages/host-service/test/pull-requests.test.ts`:
- Around line 60-75: The test currently replaces syncOneWorkspace with a
hardcoded implementation that calls syncWorkspaceRow directly and bypasses the
real queue/lookup/selection logic; instead, capture the original
syncOneWorkspace, replace it with a thin wrapper that asserts the workspace ID
passed in (or records it) and then calls the original implementation so the
production sync path still runs; reference syncOneWorkspace and syncWorkspaceRow
(and the alternative occurrence around the other block) and apply the same
wrapper pattern there rather than hardcoding the row.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b254526-e5db-4560-8dab-9d867a1dcab3
📒 Files selected for processing (3)
packages/host-service/src/runtime/pull-requests/pull-requests.tspackages/host-service/test/pull-requests-scaling.test.tspackages/host-service/test/pull-requests.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/host-service/src/runtime/pull-requests/pull-requests.ts
- watch.ts: serialize normalizeEvent calls in flushPendingEvents — replaces Promise.all with a sequential for-of loop so LRU mutations land in event order, not stat-completion order. Net code roughly unchanged but removes the concurrency hazard coderabbit flagged. - watch-pathtypes-growth.test.ts: consolidate manager cleanup into afterEach. Tests register managers via createManager() and stop calling unsubscribe() + manager.close() inline. afterEach closes them all even if a test throws. Net code reduction (-16 lines). - pull-requests.test.ts: tighten warn assertion to match the actual "Failed to sync workspace" prefix instead of accepting any console.warn. - v2-paths-worktree-perf-fix-plan.md: align acceptance criteria wording with the tests that actually landed.
…lper `evictStaleSearchIndexEntries` is redundant: per-hit TTL check on getSearchIndex line 299-307 already discards stale entries on access, and the hard LRU cap of 12 bounds memory regardless of TTL behavior. The build-path sweep over all entries was duplicated work that did nothing the LRU eviction wasn't already doing. `bumpAndReturnCachedIndex` had one caller and was 4 lines of body — inlined directly into the hit path. Net -23 lines.
# Conflicts: # packages/host-service/src/runtime/pull-requests/pull-requests.ts
CI typecheck failed because workspace-fs had a hand-rolled src/bun-test.d.ts shim with a minimal `expect` (only `toContain` / `toEqual` / `toHaveLength` / `toBeNull` / `toBeTruthy`) that shadowed the real bun-types definitions. Adding bun-types as a devDependency and dropping the shim restores the full matcher surface. CI tests OOM'd on @superset/workspace-fs#test (exit 137). The cache-and-paths-memory bench creates 130 worktrees × 200 files + heap snapshots and was being picked up by default `bun test` because of its `.bench.test.ts` suffix. Renamed both bench files to `.bench.ts` (off the auto-discovery pattern) and added explicit `bun run bench` scripts so they're still runnable on demand. Also tightened search-cache-eviction.test.ts array typings: previous `unknown[]` was fine under the shim's permissive `expect` but doesn't typecheck against the real signature. Now uses `Awaited<ReturnType<typeof getSearchIndex>>[]` with explicit guards for noUncheckedIndexedAccess.
…nits CI host-service#test was OOMing (exit 137) because the integration test created 4 scenarios with simple-git + WorkspaceFilesystemManager + GitWatcher per scenario (15 worktrees + 15 parcel-watcher subscriptions total). Two of those scenarios just re-asserted what the mock-based unit test in test/pull-requests-scaling.test.ts already pins — linearity of git-subprocess count and "safety-net walks all N". Removes the duplicative integration scaling cases. Keeps only the event-driven scenario, which is the unique integration coverage (verifies a real `git commit` in one workspace triggers exactly one single-workspace sync, with the others staying quiet). Reduced from 5 to 3 worktrees — enough to prove "only the target was touched". Net: -142 lines, ~80% fewer worktrees spawned per test file run.
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
1 participant
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.
Summary
Three steady-state costs in v2 host-service grew linearly with worktree count, even when worktrees were idle. After this branch, host-service idle CPU and JS heap stay roughly flat as N grows.
syncWorkspaceBranchespolling timer with aGitWatcher.onChangedsubscription. Idle worktrees now do 0 git subprocesses; branch changes propagate at ~430 ms p50 (was up to 30 s).syncWorkspaceBranchessurvives as a 5-min safety-net sweep forGitWatcheroverflow / error paths.PROJECT_REFRESH_INTERVAL_MSalso drops from 20 s → 5 min (Fix Bump tailwind-merge from 2.6.0 to 3.3.1 #4 bundled in).searchIndexCache. Hard-caps cached indexes at 12 (was unbounded across all worktrees ever opened).pathTypes. Hard-caps at 10 k entries (was unbounded — log rotation / hashed build artifacts leaked entries forever).GitWatcherwas lifted out ofEventBusinto a standalone instance inapp.tsso bothEventBus(broadcasts to clients) andPullRequestRuntimeManagershare one watcher.Companion docs in
plans/v2-paths-worktree-perf-findings.mdandplans/v2-paths-worktree-perf-fix-plan.mdhave the full audit, methodology, and per-fix targets.Measured impact
Workspace-fs caps (
cache-and-paths-memory.bench.test.ts):pathTypespathTypes.size@ 20 k unique pathssearchIndexCacheretained @ 130 worktreesPull-requests runtime (
pull-requests-scaling.bench.test.ts, real DB + real git):Test plan
bun testfrompackages/host-service— 460/460 passing, including the new event-driven steady-state integration test (commits in 1/5 worktrees → exactly 4 git ops on 1 worktree)bun testfrompackages/workspace-fs— 43/43 passingbun run lintcleanbun run typecheckclean forhost-service(theworkspace-fstest typecheck errors flagged by turbo are pre-existing —bun:testtypes in test-only files, unrelated to this branch)workspaces.headShaupdates within ~1 s in the rendererSummary by cubic
Stop
v2host-service from scaling with worktree count. Pull‑requests sync is now event‑driven viaGitWatcher; caches are capped with TTL, directory watch hints are preserved, per‑workspace syncs serialize/coalesce, and watcher normalization is serialized. Idle CPU and heap stay flat, and branch changes update in ~430 ms.Refactors
packages/host-service: drive branch sync fromGitWatcher.onChanged; replace the 30s poll with a 5‑min safety‑net sweep; create a singleGitWatcherinapp.tsand inject intoEventBusand the PR runtime; addsyncOneWorkspace; serialize and coalesce per‑workspace syncs via a queue; route the safety‑net sweep through the same queue; drop project refresh to 5 min; slim the integration test to only the event‑driven scenario to avoid CI OOM (scaling covered by unit tests).packages/workspace-fs: add LRU + 30‑min idle TTL tosearchIndexCache(cap 12; TTL enforced on hits); remove the redundant TTL sweep; split path tracking into cappedfilePaths(10k) and uncappeddirectoryPathsto preserve delete semantics; serialize watcher event normalization; add tests/benches and deflake where needed.Dependencies
bun-typesdevDependency inhost-serviceandworkspace-fs; drop the localbun-test.d.tsshim; move heavy benches behind explicitbun run benchscripts to keepbun testlean.Written for commit 6a6fac2. Summary will update on new commits.
Summary by CodeRabbit
New Features
Performance
Tests
Documentation