Skip to content

refactor: deduplicate shared driver and provider constants#1474

Merged
johntmyers merged 1 commit into
NVIDIA:mainfrom
ericcurtin:refactor/deduplicate-driver-provider-constants
May 20, 2026
Merged

refactor: deduplicate shared driver and provider constants#1474
johntmyers merged 1 commit into
NVIDIA:mainfrom
ericcurtin:refactor/deduplicate-driver-provider-constants

Conversation

@ericcurtin
Copy link
Copy Markdown
Contributor

Summary

Remove three categories of copy-pasted code that had accumulated across multiple crates, replacing them with single authoritative definitions.

Related Issue

N/A

Changes

  • openshell-core::image: Add default_sandbox_image() and remove four identical copies spread across openshell-driver-docker, openshell-driver-kubernetes, openshell-driver-podman, and openshell-server.
  • openshell-core::driver_utils: Add SUPERVISOR_IMAGE_BINARY_PATH constant and remove two identical copies from openshell-driver-docker and openshell-driver-kubernetes. This path is semantically critical — a divergence would cause a silent runtime failure.
  • openshell-providers: Implement ProviderPlugin for ProviderDiscoverySpec (blanket impl). Eight simple providers (anthropic, claude, codex, copilot, github, gitlab, nvidia, openai) previously repeated an identical three-method impl ProviderPlugin block delegating straight to their SPEC constant. Each file now contains only the SPEC declaration; the registry registers SPEC directly.

Net change: -146 lines across 16 files.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated (all existing tests preserved)
  • E2E tests added/updated (not applicable — no behavior change)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

Move default_sandbox_image() and SUPERVISOR_IMAGE_BINARY_PATH into
openshell-core so all compute drivers share a single authoritative
definition. Eliminates four identical copies of default_sandbox_image()
and two identical copies of SUPERVISOR_IMAGE_BINARY_PATH.

Implement ProviderPlugin for ProviderDiscoverySpec to remove the
repeated three-method delegation boilerplate from eight simple
provider modules. Each now declares only a SPEC constant; the blanket
impl handles id(), discover_existing(), and credential_env_vars().
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 20, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Comment thread crates/openshell-providers/src/providers/codex.rs
Comment thread crates/openshell-driver-kubernetes/src/config.rs
Copy link
Copy Markdown
Member

@elezar elezar left a comment

Choose a reason for hiding this comment

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

My questions are largely unrelated to the actual changes.

It looks like a reasonable cleanup from my perspective!

@johntmyers johntmyers added the test:e2e Requires end-to-end coverage label May 20, 2026
@github-actions
Copy link
Copy Markdown

Label test:e2e applied, but pull-request/1474 is at {"messa while the PR head is 42d8f46. A maintainer needs to comment /ok to test 42d8f468de78d74602f2abe9c5544a646d510759 to refresh the mirror. Once the mirror catches up, re-run Branch E2E Checks from the Actions tab.

@johntmyers
Copy link
Copy Markdown
Collaborator

/ok to test 42d8f46

@johntmyers johntmyers merged commit b332ffd into NVIDIA:main May 20, 2026
47 of 52 checks passed
@ericcurtin ericcurtin deleted the refactor/deduplicate-driver-provider-constants branch May 20, 2026 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants