refactor(vscode): share terminal directory blocking - #13644
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Previous Review Summary (commit fb0781d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit fb0781d)Status: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Reviewed by grok-4.6 · Input: 122.1K · Output: 6.2K · Cached: 154.2K Review guidance: REVIEW.md from base branch |
| if (!count || count === 1) this.blocked.delete(target) | ||
| else this.blocked.set(target, count - 1) | ||
| } | ||
| return block(target, this.blocked, this.creates.get(target)) |
There was a problem hiding this comment.
a bit weird an async without any await in it, I think in this case it's TS dictating the async nature, maybe worth awaiting or removing the async declaration? I'd be good with it's OK as it is as answer though
WebReflection
left a comment
WebReflection
left a comment
There was a problem hiding this comment.
Minor comment but maybe it's already readable / clean enough as it is, I usually prefer not having async if there is no await in the body but technically peaking I think the JS engine might optimize that anyway: approved!
2 participants
What Problem This Solves
The terminal and script-terminal managers duplicate the same reference-counted directory barrier used during worktree cleanup.
Why This Change Was Made
Share only that barrier through the existing PTY cleanup module. Keep each manager’s maps, directory normalization, SDK calls, and error handling separate. Remove only duplication exception
83aaa96c32fb3894.User Impact
No behavior change is intended. Blocking starts synchronously, waits for the current creation snapshot, and releases each lease at most once. The complete PR removes 17 net lines across four files, including 5 net production lines. No test code is added.
Evidence
electronApplication.evaluate: Resulting promise was garbage collected.The isolated profile and harness were cleaned up.No changeset is needed for this internal behavior-preserving refactor.