Repository navigation
agentHost: stop recomputing and running git for folders only archived chats use - #343275
Sandeep Somavarapu (sandy081) merged 5 commits into
Conversation
getRepositoryRoot spawned 'git rev-parse --show-toplevel' with cwd set to the working directory even when that folder had been deleted. The resulting 'spawn git ENOENT' has empty stderr, so it was never negatively cached and was re-probed and re-logged on every refresh. Check that the directory exists before spawning git and return undefined quietly without caching, so a worktree recreated at the same path is picked up by the next lookup. A spawn ENOENT for a directory that vanished after the check is handled the same way.
The Branch summary computed and watched every folder any chat of a session ever had, including archived chats and the folders subagents are pinned to (often deleted worktrees). Each git-state refresh re-armed every one of those owners through refreshBranchChangeset, which clears what is known about an unavailable owner, so a session with many archived chats and subagents kept recomputing and re-broadcasting changeset status. Move the 'only chats in use count' rule (#341782, #341885) out of AgentService into getChatsInUse and apply it to the Branch summary owners, the summary working directories and the coordinator's branch summary subscriptions. AgentService behavior is unchanged. The default chat is always kept. Also stop two ways a round restarts itself: do not dispatch a changeset status that is already current, and do not recompute an owner whose computation already ended in an error from the summary update; an explicit refresh still retries it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…archived The branch summary subscribes to the folders of the chats in use only, but nothing re-evaluated that set when a chat's archived state changed. Archiving left the chat's changeset and file watcher subscribed, and unarchiving left it unobserved until an unrelated refresh. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lder and in-use fixes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open issues
What changed in this PR
This PR reduces agent host hangs by avoiding repeated git probes and changeset recomputation for folders that are only referenced by archived chats (including archived subagents / deleted worktrees), and by reconciling branch-summary subscriptions when archive state changes.
Changes:
- Skip
gitspawning for missing/non-directory working directories and reduce noisy warnings/log churn. - Centralize “chats in use” resolution and apply it to branch summary owners, session summary working directories, and branch summary reconciliation.
- Add/adjust unit + integration tests covering missing dirs, recreated worktrees, and archive/unarchive effects on subscriptions.
| File | Description |
|---|---|
| src/vs/platform/agentHost/node/agentHostGitService.ts | Avoid git spawn for missing working directories; add helper + ENOENT handling. |
| src/vs/platform/agentHost/node/agentHostChatsInUse.ts | New shared logic to compute chats “in use” and session summary sources. |
| src/vs/platform/agentHost/node/agentService.ts | Delegate in-use folder computation to new shared helper. |
| src/vs/platform/agentHost/node/agentHostChangesetService.ts | Restrict branch-summary owners to in-use sources; reduce redundant status dispatches. |
| src/vs/platform/agentHost/node/agentHostChangesetCoordinator.ts | Reconcile branch summary resources on chat archive/unarchive and removals. |
| src/vs/platform/agentHost/node/agentConfigurationService.ts | Limit session-summary working directories to in-use sources. |
| src/vs/platform/agentHost/test/node/agentHostGitService.integrationTest.ts | Add integration tests for missing dirs, recreated worktrees, and contribution probing behavior. |
| src/vs/platform/agentHost/test/node/agentHostChatsInUse.test.ts | New unit tests for chat-in-use resolution rules. |
| src/vs/platform/agentHost/test/node/agentHostChangesetService.test.ts | Add tests ensuring archived chats/subagents don’t trigger branch summary computation; reduce recomputation loops. |
| src/vs/platform/agentHost/test/node/agentHostChangesetCoordinator.test.ts | Add tests for branch summary refresh scoped to in-use chats and for archive/unarchive subscription behavior. |
| src/vs/platform/agentHost/test/node/agentConfigurationService.test.ts | Add tests for session summary directory selection using in-use logic. |
Review effort: Lite
…t they detect Review feedback: _isDirectory returned true for stat failures other than ENOENT/ENOTDIR, and isSpawnNotFoundError also matched a missing working directory. Rename them to _isMissingDirectory (true only when the path is definitely not a directory) and isSpawnEnoentError. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Connor Peet (connor4312)
left a comment
There was a problem hiding this comment.
Reviewed the security and correctness of the changes and relevant remote context. No actionable P1 or P2 findings identified.
Written by Connor's agent running GPT-6.1 Sol


No tracking issue: this comes from an observed hang of the Insiders agent host (details below).
Contract
The agent host must not repeatedly recompute, re-broadcast, or run git for folders that only archived chats, or subagents/side chats of archived chats, use (deleted worktrees included), and it must stay responsive while such a session is loaded.
workingDirectorieslists from before agentHost: detach an archived chat's folders from its session #341782.refreshBranchChangesetstill retries; a worktree deleted and recreated at the same path is picked up (no permanent negative cache for missing directories).Observed problem
On a real Insiders agent host with a session that had ~150 archived chats and ~77 subagent chats (each pinned to 3-24 folders, some of them deleted worktrees), the host sent nothing to the window for ~110 s and a ping took 58 s. During ~109 s, for that one session: ~10,600
changeset/statusChangedbroadcasts (~97/s), ~2,000git rev-parse --show-toplevel failed: spawn git ENOENT, 147git status5 s timeouts, and a flip-flopping pull-request operation.Root cause
AgentHostChangesetService._getBranchSummaryOwners,getSessionSummaryWorkingDirectories(also used by Session Changes discard) andAgentHostChangesetCoordinator._reconcileBranchSummaryResourceswalked all chats (archived ones, and subagents with pinned folders) while the "only chats in use count" rule from agentHost: detach an archived chat's folders from its session #341782/agentHost: only real chats' folders count for archive/delete detaching and Git state #341885 existed only for archive/delete detaching and git-state refresh.refreshBranchChangeset()for every owner. That call clears_unavailableBranchOwners/_failedBranchOwnersand recomputes the owner; every owner that ends "not a git repository" calls_updateBranchSummary, which refreshes the next owner. The cost is roughly quadratic in the number of owners and is re-armed by every git-state refresh.AgentHostGitService.getRepositoryRootnever quieted a missing working directory:spawn git ENOENThas empty stderr, so it was not matched by the "not a git repository" cache and was re-spawned and re-logged on every probe.Provenance: LIKELY regression
_getBranchSummaryOwners, the coordinator's_reconcileBranchSummaryResourcesandgetSessionSummaryWorkingDirectories. Verified withgit log -Sandgit show.getSessionSummaryWorkingDirectoriesfor discard.What changed (one commit each)
8154354f1f2skip git spawn for missing working directories:getRepositoryRootstats the directory first and returnsundefinedat trace level without spawning git. Nothing is cached for a missing path, so a recreated worktree is found by the next lookup. Aspawn git ENOENTfor a directory that vanished in between is also trace-level; "not a git repository" caching is unchanged.3c168d0198elimit session changes to the folders of chats in use:node/agentHostChatsInUse.ts:getChatsInUseis the agentHost: detach an archived chat's folders from its session #341782/agentHost: only real chats' folders count for archive/delete detaching and Git state #341885 rule moved out ofAgentService._getWorkingDirectoriesInUse(which now delegates, behavior unchanged);getSessionSummarySourcesis the default chat plus the folder-providing chat of every chat in use.getSessionSummaryWorkingDirectoriesand the coordinator's reconcile.AgentHostChangesetService:_updateBranchSummaryno longer refreshes an owner whose changeset is already inError; the status-restore helpers skip aChangesetStatusChangeddispatch when status and error are already current.b10877f5dd4reconcile the branch summary when a chat is archived or unarchived: nothing re-evaluated the in-use set when a chat's archived state changed (found while replaying "unarchive").7212ebd81addrop explanatory inline comments added by the previous commits.Tests (Node 24.22.0)
New or extended:
agentHostChatsInUse.test.ts(new),agentConfigurationService.test.ts,agentHostChangesetService.test.ts,agentHostChangesetCoordinator.test.ts,agentHostGitService.integrationTest.ts../scripts/test.sh --run src/vs/platform/agentHost/test/node/{agentHostChatsInUse,agentConfigurationService,agentHostChangesetService,agentHostChangesetCoordinator,agentHostChangesetStrategy,agentHostDiscardChangesOperationHandler,agentService,agentHostGitService,agentHostSessionRepositories}.test.ts./scripts/test-integration.sh --run src/vs/platform/agentHost/test/node/agentHostGitService.integrationTest.ts --grep "getRepositoryRoot"npm run typecheck-client,npm run valid-layers-check,npm run eslint -- <changed files>, husky hygiene on every commitRed without the fix (source changes stashed, new tests kept):
getRepositoryRootintegration tests fail on the base.agentHostChangesetServicetests, the newagentConfigurationServicesummary test and the new coordinator test fail on the base. The fifth new test ("the default chat is kept") passes either way by design.Live before/after
Real agent host (not mocks) from the base revision
e0a4363d21aand from this branch, through the repo's agent-host E2E harness: isolated home and user-data directory, scratch git repos, one recorded model exchange set (a few tiny prompts, including a realexploresubagent) replayed identically on both builds. Scenario: a worktree-isolation session; a peer chat with its own host-created worktree running a subagent (subagent chat has origintooland is pinned to 2 session folders); 11 more peer chats each with their own worktree; archive all 12 and delete their worktrees; keep the session subscribed for 3 min.Per 5 s timeline after archiving and deleting (same fixture, same window):
e0a4363d21a)changeset/statusChangedbroadcasts, first 5 s / next 5 s / restspawn git ENOENTlinesEarlier whole-window (3 min) trials: base 297-428 broadcasts and 774-1,276 git spawns (4 trials); this branch 18-19 broadcasts and 79-96 git spawns (3 trials).
Correction / scale caveat.
git statuscalls timing out at 5 s and git-state refreshes re-arming the cascade. Why it lasted minutes there is an inference from the mechanism, not something I reproduced.Neighbors replayed on this branch: PASS.
readywithin ~1 s.Performance / lifecycle assessment
getChatsInUseis O(chats) per call plus an ancestor walk per chat (bounded by visited-set); it replaces a walk that already existed inAgentService.onDidEmitEnvelopelistener. Subscription release goes through the existing_releaseBranchSummaryResourcepath (covered by a unit test); file-watcher start/stop counts per worktree are identical on base and this branch in the live run. Heap/listener-count measurement was not run.Partially verified, remaining
e0a4363d21a..7212ebd81ad, all 11 changed files plus surrounding ownership, state-envelope, changeset-scope, refresh/retry, watcher, discard and git-cache paths, full review matrix. Findings: none; no repairs requested. The reviewer independently ran the four targeted unit files (263 passing, 30 pending) and the 26 real-gitgetRepositoryRootintegration tests, and confirmedgit diff --checkis clean and all four commits are signed. It noted the two follow-ups below as reasonable and out of scope.git statustimeouts, ping under load): NOT RUN.AgentHostGitService._runGitstill spawns git for other callers (rev-parse HEAD,reviewedref lookups) in a missing working directory; trace-level only.workingDirectories; retry/backoff for 5 sgit statustimeouts; file watcher exclusions.