Discharge7443/brave-core
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[brockit] Auto-detect vscode terminal (#35934)

Browse Source 
This change drops the `--vscode` from `brockit lift` and introduces an
auto-detect approach that checks for the presence of a vscode terminal
session socket, and uses that socket to open files if detected as
opening sources that require conflict attention is most likely always
desired.

This change also abandons the use of `code` command call to open
sources, and uses the socket for that. This fixes the problem that code
would send the command to open the sources to whatever was the
foreground vscode window the user had, which would be the wrong one when
multitasking.

Resolves https://github.com/brave/brave-browser/issues/54989
This commit is contained in:
cdesouza-chromium
2026-04-27 23:46:37 +01:00
committed by GitHub
parent 2d5f19b071
commit 55bfeb4872
4 changed files with 368 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tools/cr/brockit.py
+17 -35
View File
@@ -86,7 +86,7 @@ The following steps will take place:
expected to provide separate commits for deleted patches, explaining the
reason.
6. Having resolved all conflicts. Restart *🚀Brockit!* with `--continue` and
other similar arguments you may want to keep (e.g. `--vscode`).
other similar arguments you may want to keep.
7. *🚀Brockit!* will pick up from where it stopped, possibly running
`npm run update_patches`, staging all patches, and committing the under
*Conflict-resolved patches from Chromium [from] to [to].*
@@ -218,6 +218,7 @@ from repository import Repository, CHROMIUM_SRC_PATH
from terminal import console, terminal
import versioning
from versioning import Version
from vscode import VsCodeIpcConnection
# This file is updated whenever the version number is updated in package.json
PINSLIST_TIMESTAMP_FILE = (
@@ -939,8 +940,7 @@ class Upgrade(Versioned):
def status_message(self):
return "Upgrading Chromium base version"
def apply_patches_3way(self,
launch_vscode: bool = False) -> ApplyPatchesRecord:
def apply_patches_3way(self) -> ApplyPatchesRecord:
"""Applies patches that have failed using the --3way option to allow for
manual conflict resolution.
@@ -949,6 +949,9 @@ class Upgrade(Versioned):
are waiting for conflict resolution.
A list of the patches applied will be produced as well.
When running brockit in a vscode terminal, this method will open any
files that need attention in the editor session.
"""
# A dictionary that holds a list for all patch files affected, by
# repository.
@@ -996,7 +999,7 @@ class Upgrade(Versioned):
for patch in patch_list
]))
vscode_args = ['code']
vscode_files = []
for repo, patches in patch_files.items():
for patch in patches:
apply_result = patch.apply()
@@ -1046,7 +1049,7 @@ class Upgrade(Versioned):
console.log(
Padding(f'✘ {patch.source()} [red bold](deleted)',
(0, 4)))
vscode_args.append(patch.path)
vscode_files.append(patch.path)
elif status.status == 'R':
renamed_to = patch.repository.from_brave(
) / status.renamed_to
@@ -1055,7 +1058,7 @@ class Upgrade(Versioned):
f'✘ {patch.source_from_brave()}\n '
f'([yellow bold]renamed to[/] {renamed_to})',
(0, 4)))
vscode_args += [patch.path, renamed_to]
vscode_files += [patch.path, renamed_to]
# Printing the commmit message for the grouped changes.
console.log(
@@ -1071,22 +1074,16 @@ class Upgrade(Versioned):
for patch in broken_patches:
source = patch.source_from_brave()
console.log(Padding(f'✘ {patch.path} ➜ {source}', (0, 4)))
vscode_args += [patch.path, source]
vscode_files += [patch.path, source]
if files_with_conflicts:
vscode_args += files_with_conflicts
vscode_files += files_with_conflicts
file_list = '\n'.join(f' ✘ {file}'
for file in files_with_conflicts)
terminal.log_task(f'[bold]Manually resolve conflicts for '
f'{ACTION_NEEDED_DECORATOR}:[/]\n{file_list}')
if launch_vscode and len(vscode_args) > 1:
try:
terminal.run(vscode_args)
except subprocess.CalledProcessError as e:
logging.error(
'Failed to launch VSCode with the following args: %s\n%s',
' '.join(vscode_args), e.stderr)
VsCodeIpcConnection().open_file(vscode_files)
# The continuation file is updated at the end of the process, in case
# the process has to be continued later.
@@ -1502,7 +1499,7 @@ class Upgrade(Versioned):
# continuation file around.
ContinuationFile.clear()
def _start(self, launch_vscode: bool, ack_advisory: bool):
def _start(self, ack_advisory: bool):
"""Starts the upgrade process.
This function is responsible for starting the upgrade process. It will
@@ -1513,11 +1510,6 @@ class Upgrade(Versioned):
For cases where no conflict resolution is required, the process will
will continue, concluding the whole four steps of the upgrade process.
Args:
launch_vscode:
Indicates if the user wants to launch vscode with the patches that
require manual conflict resolution.
Return:
Returns True if the process was successful, and False otherwise.
"""
@@ -1576,8 +1568,7 @@ class Upgrade(Versioned):
if (e.returncode != 0
and 'Exiting as not all patches were successful!'
in e.stderr.splitlines()[-1]):
apply_record = self.apply_patches_3way(
launch_vscode=launch_vscode)
apply_record = self.apply_patches_3way()
if apply_record.requires_conflict_resolution():
# Manual resolution required.
raise ActionNeededException(
@@ -1602,8 +1593,8 @@ class Upgrade(Versioned):
terminal.run_npm_command('chromium_rebase_l10n')
self._save_rebased_l10n()
def execute(self, no_conflict_continuation: bool, launch_vscode: bool,
with_github: bool, ack_advisory: bool):
def execute(self, no_conflict_continuation: bool, with_github: bool,
ack_advisory: bool):
"""Executes the upgrade process.
Keep in this function all code that is common to both start and continue.
@@ -1612,9 +1603,6 @@ class Upgrade(Versioned):
no_conflict_continuation:
Indicates that a continuation does not produce a conflict-resolved
change.
launch_vscode:
Indicates the user wants to launch vscode with the patches that
require manual conflict resolution.
with_github:
Indicates the user wants to create or update the github issue for
the upgrade.
@@ -1665,7 +1653,7 @@ class Upgrade(Versioned):
self._continue(no_conflict_continuation=no_conflict_continuation)
else:
self._start(launch_vscode=launch_vscode, ack_advisory=ack_advisory)
self._start(ack_advisory=ack_advisory)
if with_github:
GitHubIssue(base_version=self.base_version,
@@ -2271,11 +2259,6 @@ def main():
action='store_true',
help='Creates or updates the github for this branch.',
dest='with_github')
lift_parser.add_argument(
'--vscode',
action='store_true',
help=
'Launches vscode for manual conflict resolution and similar issues.')
lift_parser.add_argument(
'--no-conflict-change',
action='store_true',
@@ -2397,7 +2380,6 @@ def main():
args.is_continuation)
upgrade.run(no_conflict_continuation=args.no_conflict,
launch_vscode=args.vscode,
with_github=args.with_github,
ack_advisory=args.ack_advisory)
if args.command == 'rebase':
tools/cr/brockit_test.py
+2
View File
@@ -4,10 +4,12 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
# You can obtain one at https://mozilla.org/MPL/2.0/.
import os
import unittest
from pathlib import Path
import shutil
from datetime import datetime
from unittest.mock import MagicMock, patch
import brockit
from brockit import ApplyPatchesRecord
tools/cr/vscode.py
+108
View File
@@ -0,0 +1,108 @@
# Copyright (c) 2025 The Brave Authors. All rights reserved.
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
# You can obtain one at https://mozilla.org/MPL/2.0/.
import http.client
import io
import json
import logging
import os
from pathlib import Path
import platform
import socket
class _VsCodeIpcConnectionBase(http.client.HTTPConnection):
"""Base for platform-specific VS Code IPC HTTP connections."""
def __init__(self, env_var: str):
super().__init__('vscode')
self._socket_path = os.environ.get(env_var, '')
def open_file(self, files: list) -> None:
"""Opens files in the VS Code window that owns this terminal.
Communicates directly with the IPC socket rather than spawning
`code`, so the request is routed to the specific window whose
extension host created the socket — not whichever window happens
to be in the foreground.
"""
if not self._socket_path or not files:
return
file_uris = [Path(str(f)).resolve().as_uri() for f in files]
body = json.dumps({
'type': 'open',
'fileURIs': file_uris,
'forceReuseWindow': True,
}).encode()
logging.debug('VS Code IPC request: socket=%s, body=%s',
self._socket_path, body.decode())
try:
self.request('POST', '/', body,
{'Content-Type': 'application/json'})
resp = self.getresponse()
logging.debug('VS Code IPC response: %d %s\n', resp.status,
resp.reason)
except Exception as e:
logging.warning('Could not open files in VS Code window: %s', e)
class _PosixVsCodeIpcConnection(_VsCodeIpcConnectionBase):
"""VS Code IPC connection via Unix domain socket (POSIX)."""
def __init__(self):
"""Reads the IPC socket path from VSCODE_IPC_HOOK_CLI.
VS Code sets this variable in every integrated terminal it spawns,
pointing to the Unix socket owned by that window's extension host.
"""
super().__init__('VSCODE_IPC_HOOK_CLI')
def connect(self) -> None:
"""Overrides HTTPConnection.connect to use a Unix domain socket."""
self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
self.sock.connect(self._socket_path)
class _NamedPipeSocket:
"""Socket-like wrapper around a Windows named pipe for HTTPConnection."""
def __init__(self, path: str):
"""Opens the named pipe at path for binary read/write."""
self._pipe = open(path, 'r+b', buffering=0)
def sendall(self, data: bytes) -> None:
"""Writes data to the pipe (called by HTTPConnection)."""
self._pipe.write(data)
def makefile(self, _mode: str, _bufsize: int = -1) -> io.BufferedReader:
"""Returns a BufferedReader over the pipe for HTTPResponse to use."""
return io.BufferedReader(self._pipe)
def close(self) -> None:
"""Closes the underlying pipe handle."""
self._pipe.close()
def settimeout(self, _timeout) -> None:
"""No-op: named pipes do not support socket-style timeouts."""
class _WinVsCodeIpcConnection(_VsCodeIpcConnectionBase):
"""VS Code IPC connection via Windows named pipe."""
def __init__(self):
"""Reads the IPC socket path from VSCODE_GIT_IPC_HANDLE.
VS Code sets this variable in every integrated terminal it spawns,
pointing to the named pipe owned by that window's extension host.
"""
super().__init__('VSCODE_GIT_IPC_HANDLE')
def connect(self) -> None:
"""Overrides HTTPConnection.connect to use a Windows named pipe."""
self.sock = _NamedPipeSocket(self._socket_path)
VsCodeIpcConnection = (_WinVsCodeIpcConnection if platform.system()
== 'Windows' else _PosixVsCodeIpcConnection)
tools/cr/vscode_test.py
Executable
+241
View File
@@ -0,0 +1,241 @@
#!/usr/bin/env vpython3
# Copyright (c) 2025 The Brave Authors. All rights reserved.
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
# You can obtain one at https://mozilla.org/MPL/2.0/.
import io
import json
import os
import platform
import socket
import tempfile
import unittest
from unittest.mock import MagicMock, patch
from vscode import (VsCodeIpcConnection, _NamedPipeSocket,
_PosixVsCodeIpcConnection, _WinVsCodeIpcConnection)
class PosixVsCodeIpcConnectionTest(unittest.TestCase):
"""Tests for _PosixVsCodeIpcConnection."""
def setUp(self):
"""Creates a temporary directory and derives a socket path from it."""
self._tmp_dir = tempfile.TemporaryDirectory()
self.sock_path = os.path.join(self._tmp_dir.name, 'vscode.sock')
self.addCleanup(self._tmp_dir.cleanup)
def test_init_reads_socket_path_from_env(self):
"""_socket_path is set from VSCODE_IPC_HOOK_CLI."""
with patch.dict(os.environ, {'VSCODE_IPC_HOOK_CLI': self.sock_path}):
conn = _PosixVsCodeIpcConnection()
self.assertEqual(conn._socket_path, self.sock_path)
def test_init_empty_socket_path_when_env_absent(self):
"""_socket_path is empty when VSCODE_IPC_HOOK_CLI is not set."""
with patch.dict(os.environ, {}):
os.environ.pop('VSCODE_IPC_HOOK_CLI', None)
conn = _PosixVsCodeIpcConnection()
self.assertEqual(conn._socket_path, '')
def test_connect_creates_unix_socket(self):
"""connect() opens an AF_UNIX socket and connects to _socket_path."""
with patch.dict(os.environ, {'VSCODE_IPC_HOOK_CLI': self.sock_path}):
conn = _PosixVsCodeIpcConnection()
with patch('vscode.socket.socket') as mock_socket_cls:
mock_sock = MagicMock()
mock_socket_cls.return_value = mock_sock
conn.connect()
mock_socket_cls.assert_called_once_with(
socket.AF_UNIX, socket.SOCK_STREAM)
mock_sock.connect.assert_called_once_with(self.sock_path)
def test_open_file_skips_when_no_socket_path(self):
"""open_file does nothing when VSCODE_IPC_HOOK_CLI is not set."""
with patch.dict(os.environ, {}):
os.environ.pop('VSCODE_IPC_HOOK_CLI', None)
conn = _PosixVsCodeIpcConnection()
with patch.object(conn, 'request') as mock_request:
conn.open_file(['/some/file.cc'])
mock_request.assert_not_called()
def test_open_file_skips_when_files_empty(self):
"""open_file does nothing when the files list is empty."""
with patch.dict(os.environ, {'VSCODE_IPC_HOOK_CLI': self.sock_path}):
conn = _PosixVsCodeIpcConnection()
with patch.object(conn, 'request') as mock_request:
conn.open_file([])
mock_request.assert_not_called()
def test_open_file_sends_correct_request(self):
"""open_file sends POST / with the correct JSON body and headers."""
with patch.dict(os.environ, {'VSCODE_IPC_HOOK_CLI': self.sock_path}):
conn = _PosixVsCodeIpcConnection()
mock_response = MagicMock()
mock_response.status = 200
mock_response.reason = 'OK'
mock_response.read.return_value = b''
with patch.object(conn, 'request') as mock_request, \
patch.object(conn, 'getresponse', return_value=mock_response):
conn.open_file(['/path/to/file.cc'])
mock_request.assert_called_once()
method, path, body, headers = mock_request.call_args[0]
self.assertEqual(method, 'POST')
self.assertEqual(path, '/')
payload = json.loads(body)
self.assertEqual(payload['type'], 'open')
self.assertEqual(payload['fileURIs'],
['file:///path/to/file.cc'])
self.assertTrue(payload['forceReuseWindow'])
self.assertEqual(headers['Content-Type'], 'application/json')
def test_open_file_logs_error_on_failure(self):
"""open_file logs a warning and does not raise on connection failure."""
with patch.dict(os.environ, {'VSCODE_IPC_HOOK_CLI': self.sock_path}):
conn = _PosixVsCodeIpcConnection()
with patch.object(conn,
'request',
side_effect=ConnectionRefusedError('refused')):
with self.assertLogs(level='WARNING') as captured:
conn.open_file(['/path/to/file.cc'])
self.assertTrue(
any('Could not open files in VS Code window' in line
for line in captured.output))
class _ReadableRawIO(io.RawIOBase):
"""Minimal readable RawIOBase for use in _NamedPipeSocket tests."""
def readable(self):
return True
def readinto(self, _b):
return 0
class NamedPipeSocketTest(unittest.TestCase):
"""Tests for _NamedPipeSocket."""
def test_sendall_writes_to_pipe(self):
"""sendall() delegates to the underlying pipe's write()."""
mock_pipe = MagicMock()
with patch('builtins.open', return_value=mock_pipe):
sock = _NamedPipeSocket(r'\\.\pipe\test')
sock.sendall(b'hello')
mock_pipe.write.assert_called_once_with(b'hello')
def test_makefile_returns_buffered_reader_wrapping_pipe(self):
"""makefile() wraps the pipe in a BufferedReader."""
with patch('builtins.open', return_value=_ReadableRawIO()):
sock = _NamedPipeSocket(r'\\.\pipe\test')
result = sock.makefile('rb')
self.assertIsInstance(result, io.BufferedReader)
def test_close_closes_pipe(self):
"""close() delegates to the underlying pipe's close()."""
mock_pipe = MagicMock()
with patch('builtins.open', return_value=mock_pipe):
sock = _NamedPipeSocket(r'\\.\pipe\test')
sock.close()
mock_pipe.close.assert_called_once()
def test_settimeout_is_noop(self):
"""settimeout() does not raise."""
mock_pipe = MagicMock()
with patch('builtins.open', return_value=mock_pipe):
sock = _NamedPipeSocket(r'\\.\pipe\test')
sock.settimeout(30)
class WinVsCodeIpcConnectionTest(unittest.TestCase):
"""Tests for _WinVsCodeIpcConnection."""
PIPE_PATH = r'\\.\pipe\vscode-ipc-abc123'
def test_init_reads_socket_path_from_env(self):
"""_socket_path is set from VSCODE_GIT_IPC_HANDLE."""
with patch.dict(os.environ, {'VSCODE_GIT_IPC_HANDLE': self.PIPE_PATH}):
conn = _WinVsCodeIpcConnection()
self.assertEqual(conn._socket_path, self.PIPE_PATH)
def test_init_empty_socket_path_when_env_absent(self):
"""_socket_path is empty when VSCODE_GIT_IPC_HANDLE is not set."""
with patch.dict(os.environ, {}):
os.environ.pop('VSCODE_GIT_IPC_HANDLE', None)
conn = _WinVsCodeIpcConnection()
self.assertEqual(conn._socket_path, '')
def test_connect_creates_named_pipe_socket(self):
"""connect() sets sock to a _NamedPipeSocket."""
with patch.dict(os.environ, {'VSCODE_GIT_IPC_HANDLE': self.PIPE_PATH}):
conn = _WinVsCodeIpcConnection()
with patch('builtins.open', return_value=MagicMock()):
conn.connect()
self.assertIsInstance(conn.sock, _NamedPipeSocket)
def test_open_file_skips_when_no_socket_path(self):
"""open_file does nothing when VSCODE_GIT_IPC_HANDLE is not set."""
with patch.dict(os.environ, {}):
os.environ.pop('VSCODE_GIT_IPC_HANDLE', None)
conn = _WinVsCodeIpcConnection()
with patch.object(conn, 'request') as mock_request:
conn.open_file(['/some/file.cc'])
mock_request.assert_not_called()
def test_open_file_skips_when_files_empty(self):
"""open_file does nothing when the files list is empty."""
with patch.dict(os.environ, {'VSCODE_GIT_IPC_HANDLE': self.PIPE_PATH}):
conn = _WinVsCodeIpcConnection()
with patch.object(conn, 'request') as mock_request:
conn.open_file([])
mock_request.assert_not_called()
def test_open_file_sends_correct_request(self):
"""open_file sends POST / with the correct JSON body and headers."""
with patch.dict(os.environ, {'VSCODE_GIT_IPC_HANDLE': self.PIPE_PATH}):
conn = _WinVsCodeIpcConnection()
mock_response = MagicMock()
mock_response.status = 200
mock_response.reason = 'OK'
mock_response.read.return_value = b''
with patch.object(conn, 'request') as mock_request, \
patch.object(conn, 'getresponse', return_value=mock_response):
conn.open_file(['/path/to/file.cc'])
mock_request.assert_called_once()
method, path, body, headers = mock_request.call_args[0]
self.assertEqual(method, 'POST')
self.assertEqual(path, '/')
payload = json.loads(body)
self.assertEqual(payload['type'], 'open')
self.assertEqual(payload['fileURIs'],
['file:///path/to/file.cc'])
self.assertTrue(payload['forceReuseWindow'])
self.assertEqual(headers['Content-Type'], 'application/json')
def test_open_file_logs_error_on_failure(self):
"""open_file logs a warning and does not raise on connection failure."""
with patch.dict(os.environ, {'VSCODE_GIT_IPC_HANDLE': self.PIPE_PATH}):
conn = _WinVsCodeIpcConnection()
with patch.object(conn,
'request',
side_effect=ConnectionRefusedError('refused')):
with self.assertLogs(level='WARNING') as captured:
conn.open_file(['/path/to/file.cc'])
self.assertTrue(
any('Could not open files in VS Code window' in line
for line in captured.output))
class VsCodeIpcConnectionDispatchTest(unittest.TestCase):
"""Plastform-specific VsCodeIpcConnection test."""
def test_alias_matches_platform(self):
if platform.system() == 'Windows':
self.assertIs(VsCodeIpcConnection, _WinVsCodeIpcConnection)
else:
self.assertIs(VsCodeIpcConnection, _PosixVsCodeIpcConnection)
if __name__ == '__main__':
unittest.main()