test(inference): add gRPC provider-create-path coverage for built-in profiles

@johntmyers

Important

Corrections verified against main @ 83284129 — see this comment.
The entrypoint named in the Scope section below (ProviderService::create_provider) does not exist; the real functions are create_provider_record{,_with_catalog}.
Scope item 2 is also blocked on a visibility change: create_provider_record (pub(super) in crate::grpc) and upsert_inference_route (private in crate::inference) are not visible to any common module.

Context

Follow-up to PR #1704 (merged), per @johntmyers's request.

The new route-resolution tests added in PR #1704 (upsert_cluster_route_* in crates/openshell-server/src/inference.rs) insert Provider records directly via store.put_message. That covers the routing logic but bypasses the public CLI path operators actually use:

openshell provider create --type aws-bedrock --credential ... --config BEDROCK_BASE_URL=...

If a built-in profile (aws-bedrock, deepinfra, nvidia, openai, anthropic, google-vertex-ai) drifts from what the gRPC provider create handler accepts — required vs. optional credential validation, discovery behaviour, defaulting — the existing tests won't catch it.

Scope

Add coverage that goes through openshell_server::grpc::provider::ProviderService::create_provider (or the equivalent public entrypoint) for each built-in profile:

Positive create path: the documented provider create --type <profile> ... command shape produces a stored Provider record matching what resolve_provider_route expects.
Cluster route end-to-end: chain create_provider → upsert_cluster_inference_route (with verify: false for protocols not yet supported by the validation probe — currently aws_bedrock_invoke, but this list is short).
Negative paths: for each profile, the gRPC create_provider rejects malformed inputs (empty credential map where required, unknown profile type, etc.) with the same error shape the CLI surfaces.

Why this matters

Documentation in docs/sandboxes/manage-providers.mdx and docs/sandboxes/inference-routing.mdx shows specific provider create commands. Today nothing pins those down to behaviour. A test layer one level up from the route-resolution unit tests would catch docs/profile drift before users do.

📋 triage-agent

Triage Assessment

Classification: feature-valid

Summary

The requested provider-create contract coverage is a concrete and still-relevant testing task.

Investigation

Route tests in crates/openshell-server/src/inference.rs still persist provider records directly before calling route logic. grpc/provider.rs has substantial handler validation coverage, but it does not table-drive the documented inference profiles through handle_create_provider and then upsert_inference_route. PR #1704 supplied the Bedrock routing behavior, not this end-to-end contract coverage.

Recommendation

Keep the issue open as a Task. Prefer a table-driven suite for inference-capable built-in profiles, covering positive creation, route compatibility, and representative invalid inputs.

Re-checked this against main @ 83284129. The gap is still real, but the issue text has gone stale in a way that would stop an implementer, so recording corrections here.

Still valid

Route tests in crates/openshell-server/src/inference.rs still insert Provider records via store.put_message, bypassing the create path — 6 sites (lines 1236, 1316, 1391, 1443, 1499, 1534).
create_provider_record is called ~71 times across tests, but all ad hoc. There is still no table-driven pass over the built-in inference profiles.
The existing *built_in* tests cover profile management (import / update / delete / list), not "create a provider of each built-in type, then resolve a route".
Profile list unchanged: anthropic, aws-bedrock, deepinfra, google-vertex-ai, nvidia, openai. Both cited docs files still exist at their original paths.

Corrections to the scope above

1. The entrypoint named in this issue does not exist. I wrote openshell_server::grpc::provider::ProviderService::create_provider. The actual functions are:

// crates/openshell-server/src/grpc/provider.rs:74
pub(super) async fn create_provider_record(
    store: &Store, workspace: &str, provider: Provider,
) -> Result<Provider, Status>

// crates/openshell-server/src/grpc/provider.rs:85
pub(super) async fn create_provider_record_with_catalog(
    store: &Store, catalog: &EffectiveProviderProfileCatalog,
    workspace: &str, provider: Provider,
) -> Result<Provider, Status>

The _with_catalog variant is the better target for a table-driven suite — it takes the catalog explicitly instead of rebuilding it per call.

2. The route function signature. Scope item 2 should target:

// crates/openshell-server/src/inference.rs:219
async fn upsert_inference_route(
    store: &Store, workspace: &str, route_name: &str,
    provider_name: &str, model_id: &str, timeout_secs: u64,
    verify: bool,
) -> Result<UpsertedInferenceRoute, Status>

The verify: false note in item 2 does map correctly — verify is derived from !req.no_verify.

3. Blocker: the two functions cannot currently be called from the same module.

mod grpc and mod inference are sibling private modules under the crate root (lib.rs:33, lib.rs:35):

create_provider_record is pub(super) → visible only within crate::grpc
upsert_inference_route has no visibility modifier → visible only within crate::inference

So nothing in the crate can see both, and scope item 2 (chaining create → route) cannot be written as specified without first widening one of them to pub(crate). Worth deciding which, since it's a small API-surface change reviewers will want to be deliberate about. Items 1 and 3 are unaffected and can be written against create_provider_record_with_catalog inside the existing mod tests in grpc/provider.rs.

Scope note

The provider-profile system grew considerably after this issue was filed (custom profile import/lint/update/delete, AWS STS credential refresh). This issue should be read as covering built-in profiles only; custom-profile create coverage is a separate concern.

This issue has had no activity for 14 days and is now marked stale. It may be closed in 7 days if there is no further activity. Comment or remove the state:stale label to keep it open.

Still relevant — re-verified against main @ 69a05ebb. Refreshing the references, since the line numbers in the earlier comments have gone stale again.

The gap has widened, not closed. Provider records inserted directly via store.put_message in crates/openshell-server/src/inference.rs:

checked	sites
2026-08-04 (`83284129`)	6
today (`69a05ebb`)	27

So route tests are still bypassing the create path, and the pattern has spread as the file grew.

Entrypoints unchanged in name, moved in position — crates/openshell-server/src/grpc/provider.rs:

:104  pub(super) async fn create_provider_record(..)
:116  pub(super) async fn create_provider_record_with_catalog(..)

(were :74 / :85.)

The *built_in* tests still cover profile management only, not create-then-route. Current set:

update_provider_profile_rejects_built_in_and_missing_profiles
list_provider_profiles_returns_built_in_profile_categories
import_provider_profile_allows_legacy_provider_type_ids_without_built_in_profiles

None of these creates a provider of each built-in type and then resolves a route, which is what this issue asks for and what the triage assessment recommended keeping open as a Task.

Could a maintainer drop state:stale? I do not have permission to remove the label myself.

This issue has had no activity for 14 days and is now marked stale. It may be closed in 7 days if there is no further activity. Comment or remove the state:stale label to keep it open.

Built-in providers have been removed in 0.1.x, closing, if still relevant to 0.1.x please open a new issue.

Navigation Menu

Repository navigation

test(inference): add gRPC provider-create-path coverage for built-in profiles #1941

Context

Scope

Why this matters

Related

Triage Assessment

Summary

Investigation

Recommendation

Still valid

Corrections to the scope above

Scope note

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

test(inference): add gRPC provider-create-path coverage for built-in profiles #1941

Description

Context

Scope

Why this matters

Related

Activity

matthewgrossman commented on Jul 29, 2026

Triage Assessment

Summary

Investigation

Recommendation

st-gr commented on Aug 4, 2026

Still valid

Corrections to the scope above

Scope note

github-actions commented on Aug 28, 2026

st-gr commented on Aug 30, 2026

github-actions commented on Sep 16, 2026

johntmyers commented on Sep 28, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions