Skip to content

feat: add --offline flag to skip network access during up #316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hubertdeng123 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: add --offline flag to skip network access during up #316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3440d0a
feat: add --offline flag to skip network access during devservices up
hubertdeng123 Apr 29, 2026
3af9c2d
fix: remove extra blank line to pass ruff format
hubertdeng123 Apr 30, 2026
9f976b4
fix: improve offline error message and skip_pull status text
hubertdeng123 Apr 30, 2026
dc13b02
fix: isolate supervisor config dir in test_serve_success
hubertdeng123 Apr 30, 2026
48f958a
fix: use exist_ok=True in State makedirs to prevent TOCTOU race
hubertdeng123 Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 47 additions & 28 deletions devservices/commands/up.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,10 @@ def up(args: Namespace, existing_status: Status | None = None) -> None:
},
)

remote_dependencies = _install_service_dependencies(service, mode, status)
offline = getattr(args, "offline", False)
remote_dependencies = _install_service_dependencies(
service, mode, status, offline=offline
)
_create_devservices_network()
# Add the service to the starting services table
state.update_service_entry(service.name, mode, StateTables.STARTING_SERVICES)
Expand Down Expand Up @@ -204,6 +207,7 @@ def up(args: Namespace, existing_status: Status | None = None) -> None:
service_name=local_runtime_dependency_name,
mode="default", # We intentionally don't use the mode from the parent command here
exclude_local=True, # TODO: This should be False (or maybe whatever the parent command is set to)
offline=offline,
),
status,
)
Expand All @@ -216,7 +220,12 @@ def up(args: Namespace, existing_status: Status | None = None) -> None:
span.set_data("exclude_local", exclude_local)
try:
bring_up_docker_compose_services(
service, [mode], remote_dependencies, mode_dependencies, status
service,
[mode],
remote_dependencies,
mode_dependencies,
status,
skip_pull=offline,
)
except DockerComposeError as dce:
capture_exception(dce, level="info")
Expand All @@ -239,25 +248,32 @@ def up(args: Namespace, existing_status: Status | None = None) -> None:


def _install_service_dependencies(
service: Service, mode: str, status: Status
service: Service, mode: str, status: Status, offline: bool = False
) -> set[InstalledRemoteDependency]:
with start_span(
op="service.dependencies.install", name="Install dependencies"
) as span:
status.info("Retrieving dependencies")
status.info(
"Using cached dependencies" if offline else "Retrieving dependencies"
)
span.set_data("service_name", service.name)
span.set_data("mode", mode)
span.set_data("offline", offline)
try:
remote_dependencies = install_and_verify_dependencies(
service, force_update_dependencies=True, modes=[mode]
service,
force_update_dependencies=not offline,
modes=[mode],
offline=offline,
)
span.set_data("remote_dependency_count", len(remote_dependencies))
return remote_dependencies
except DependencyError as de:
capture_exception(de)
status.failure(
f"{str(de)}. If this error persists, try running `devservices purge`"
)
msg = de.stderr if de.stderr else str(de)
if not offline:
msg = f"{msg}. If this error persists, try running `devservices purge`"
status.failure(msg)
Copy link
Copy Markdown

@cursor cursor Bot Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error message displays raw bytes in non-offline path

Medium Severity

The changed error handling now uses de.stderr if de.stderr else str(de) for the failure message. However, DependencyError.stderr is populated from CalledProcessError.stderr (via stderr=e.stderr in _update_dependency, _checkout_dependency, etc.), and _run_command calls subprocess.run without text=True, meaning e.stderr is bytes, not str. In the non-offline path, when a git operation fails, msg becomes a bytes object, and the subsequent f-string renders it as b'fatal: ...' — producing a confusing, ugly user-facing error message. Previously, str(de) was always used, which gave a clean formatted message.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 48f958a. Configure here.

exit(1)
except ModeDoesNotExistError as mde:
status.failure(str(mde))
Expand Down Expand Up @@ -310,6 +326,7 @@ def bring_up_docker_compose_services(
remote_dependencies: set[InstalledRemoteDependency],
mode_dependencies: list[str],
status: Status,
skip_pull: bool = False,
) -> None:
relative_local_dependency_directory = os.path.relpath(
os.path.join(DEVSERVICES_DEPENDENCIES_CACHE_DIR, DEPENDENCY_CONFIG_VERSION),
Expand All @@ -334,27 +351,29 @@ def bring_up_docker_compose_services(
),
)

# Pull all images in parallel
status.info("Pulling images")
pull_commands = get_docker_compose_commands_to_run(
service=service,
remote_dependencies=sorted_remote_dependencies,
current_env=current_env,
command="pull",
options=[],
service_config_file_path=service_config_file_path,
mode_dependencies=mode_dependencies,
)
if not skip_pull:
status.info("Pulling images")
pull_commands = get_docker_compose_commands_to_run(
service=service,
remote_dependencies=sorted_remote_dependencies,
current_env=current_env,
command="pull",
options=[],
service_config_file_path=service_config_file_path,
mode_dependencies=mode_dependencies,
)

with concurrent.futures.ThreadPoolExecutor() as pull_dependency_executor:
futures = [
pull_dependency_executor.submit(
_pull_dependency_images, cmd, current_env, status
)
for cmd in pull_commands
]
for future in concurrent.futures.as_completed(futures):
_ = future.result()
with concurrent.futures.ThreadPoolExecutor() as pull_dependency_executor:
futures = [
pull_dependency_executor.submit(
_pull_dependency_images, cmd, current_env, status
)
for cmd in pull_commands
]
for future in concurrent.futures.as_completed(futures):
_ = future.result()
else:
status.info("Skipping image pull (using cached images)")
Copy link
Copy Markdown

@cursor cursor Bot Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Offline mode doesn't prevent docker compose up from pulling

Medium Severity

When skip_pull is True (offline mode), the explicit docker compose pull step is skipped, but the subsequent docker compose up -d command is still issued without --pull never. By default, docker compose up uses --pull policy (which resolves to pulling missing images). In a truly offline environment, if any image isn't locally cached (e.g., after a docker image prune), docker compose up will still attempt a network pull — causing hangs or timeouts rather than a clear immediate failure. The options list needs to include --pull never when skip_pull is True.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 48f958a. Configure here.


# Bring up all necessary containers
up_commands = get_docker_compose_commands_to_run(
Expand Down
6 changes: 6 additions & 0 deletions devservices/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@ def main() -> None:
help="Path to a custom devservices config file",
default=None,
)
parser.add_argument(
"--offline",
help="Use cached dependencies and images without network access",
action="store_true",
default=False,
)

subparsers = parser.add_subparsers(dest="command", title="commands", metavar="")

Expand Down
12 changes: 12 additions & 0 deletions devservices/utils/dependencies.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ def install_and_verify_dependencies(
service: Service,
force_update_dependencies: bool = False,
modes: list[str] | None = None,
offline: bool = False,
) -> set[InstalledRemoteDependency]:
"""
Install and verify dependencies for a service
Expand All @@ -235,6 +236,17 @@ def install_and_verify_dependencies(
if dependency_key in mode_dependencies
]

if offline:
are_dependencies_valid = verify_local_dependencies(matching_dependencies)
if not are_dependencies_valid:
raise DependencyError(
repo_name="",
repo_link="",
branch="",
stderr="Cannot use --offline: some dependencies are not cached locally. Run 'devservices up' without --offline first to cache dependencies",
)
Comment thread
cursor[bot] marked this conversation as resolved.
return get_installed_remote_dependencies(matching_dependencies)
Comment on lines +239 to +248
Copy link
Copy Markdown

@sentry sentry Bot Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The DependencyError raised in offline mode doesn't display the helpful error message from stderr because its __str__ method omits it, showing a malformed message instead.
Severity: MEDIUM

Suggested Fix

Update the __str__ method for the DependencyError class in exceptions.py to include the self.stderr attribute in the output string. This will ensure the helpful error message is displayed when the exception is printed.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: devservices/utils/dependencies.py#L239-L248

Potential issue: When running with the `--offline` flag where dependencies are not
cached, a `DependencyError` is raised. The helpful error message is stored in the
`stderr` attribute of this exception. However, the `DependencyError.__str__` method does
not include the `stderr` attribute in its output. As a result, when the exception is
caught and displayed to the user, they see a confusing, malformed error message like
`DependencyError: () on .` instead of the intended actionable message.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment thread
sentry[bot] marked this conversation as resolved.
Comment thread
sentry[bot] marked this conversation as resolved.
Copy link
Copy Markdown

@cursor cursor Bot Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Offline validation misses nested dependencies, yields confusing error

Medium Severity

In offline mode, verify_local_dependencies only validates direct dependencies, but then get_installed_remote_dependencies recursively traverses nested (transitive) dependencies. If a nested dependency isn't cached, it raises DependencyNotInstalledError (with stderr=None). In the error handler, since offline is true, no helpful guidance is appended — the user just sees a generic "Dependency not installed correctly" message without being told to run without --offline first.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9f976b4. Configure here.


if force_update_dependencies:
remote_dependencies = install_dependencies(matching_dependencies)
else:
Expand Down
2 changes: 1 addition & 1 deletion devservices/utils/state.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def __new__(cls) -> State:
if cls._instance is None:
cls._instance = super(State, cls).__new__(cls)
if not os.path.exists(DEVSERVICES_LOCAL_DIR):
os.makedirs(DEVSERVICES_LOCAL_DIR)
os.makedirs(DEVSERVICES_LOCAL_DIR, exist_ok=True)
cls._instance.state_db_file = STATE_DB_FILE
cls._instance.conn = sqlite3.connect(cls._instance.state_db_file)
cls._instance.initialize_database()
Expand Down
6 changes: 5 additions & 1 deletion tests/commands/test_serve.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ def test_serve_success(

args = Namespace(extra=[])

serve(args)
with patch(
"devservices.utils.supervisor.DEVSERVICES_SUPERVISOR_CONFIG_DIR",
str(tmp_path / "supervisor"),
):
serve(args)

mock_pty_spawn.assert_called_once_with(["run", "devserver"])

Expand Down
72 changes: 72 additions & 0 deletions tests/utils/test_dependencies.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3448,3 +3448,75 @@ def test_get_active_service_names_removes_stale_entries(
remaining = state.get_service_entries(StateTables.STARTED_SERVICES)
assert "stale-service" not in remaining
assert "service-1" not in remaining


@mock.patch(
"devservices.utils.dependencies.verify_local_dependencies", return_value=True
)
@mock.patch(
"devservices.utils.dependencies.get_installed_remote_dependencies",
return_value=set(),
)
@mock.patch("devservices.utils.dependencies.install_dependencies", return_value=set())
def test_install_and_verify_dependencies_offline_cached(
mock_install_dependencies: mock.Mock,
mock_get_installed: mock.Mock,
mock_verify: mock.Mock,
) -> None:
service = Service(
name="test-service",
repo_path="/path/to/test-service",
config=ServiceConfig(
version=0.1,
service_name="test-service",
dependencies={
"dependency-1": Dependency(
description="dependency-1",
remote=RemoteConfig(
repo_name="dependency-1",
repo_link="file://path/to/dependency-1",
branch="main",
),
dependency_type=DependencyType.SERVICE,
),
},
modes={"default": ["dependency-1"]},
),
)
install_and_verify_dependencies(service, offline=True)

# Should NOT call install_dependencies (no network)
mock_install_dependencies.assert_not_called()
# Should verify local dependencies and use cached versions
mock_verify.assert_called_once()
mock_get_installed.assert_called_once()


@mock.patch(
"devservices.utils.dependencies.verify_local_dependencies", return_value=False
)
def test_install_and_verify_dependencies_offline_not_cached(
mock_verify: mock.Mock,
) -> None:
service = Service(
name="test-service",
repo_path="/path/to/test-service",
config=ServiceConfig(
version=0.1,
service_name="test-service",
dependencies={
"dependency-1": Dependency(
description="dependency-1",
remote=RemoteConfig(
repo_name="dependency-1",
repo_link="file://path/to/dependency-1",
branch="main",
),
dependency_type=DependencyType.SERVICE,
),
},
modes={"default": ["dependency-1"]},
),
)
with pytest.raises(DependencyError):
install_and_verify_dependencies(service, offline=True)
Loading