From 96c33eae7f7ab8e6f48ac14eda9504f3d2af62e8 Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Wed, 4 Nov 2026 21:19:53 -0800 Subject: [PATCH] fix(cli): explain static provider credential rotation Signed-off-by: Shailendra Singh --- crates/openshell-cli/src/commands/provider.rs | 19 ++- .../tests/provider_commands_integration.rs | 120 ++++++++++++++++++ docs/how-it-works/providers/overview.mdx | 2 +- skills/openshell-cli/SKILL.md | 2 +- 4 files changed, 140 insertions(+), 3 deletions(-) diff --git a/crates/openshell-cli/src/commands/provider.rs b/crates/openshell-cli/src/commands/provider.rs index 0098297b38..1005325f33 100644 --- a/crates/openshell-cli/src/commands/provider.rs +++ b/crates/openshell-cli/src/commands/provider.rs @@ -2820,6 +2820,7 @@ pub async fn provider_update(options: ProviderUpdateOptions<'_>) -> Result<()> { } } + let credentials_updated = !credential_map.is_empty(); let response = client .update_provider(UpdateProviderRequest { request_id: String::new(), @@ -2869,6 +2870,7 @@ pub async fn provider_update(options: ProviderUpdateOptions<'_>) -> Result<()> { "gateway did not return a provider mutation receipt; saved credentials cannot establish readiness" )); } + let has_attached_sandboxes = !response.target_receipts.is_empty(); finish_provider_mutation( &client, &response.mutation_id, @@ -2882,7 +2884,22 @@ pub async fn provider_update(options: ProviderUpdateOptions<'_>) -> Result<()> { }, readiness, ) - .await + .await?; + + // Explain the delivery contract without requiring profile-read permission + // or assuming every updated credential uses ordinary static references. + if credentials_updated && has_attached_sandboxes && readiness.output == "table" { + println!( + " Ordinary static credentials: existing processes keep their old references, even with --wait." + ); + println!( + " After readiness, use `openshell sandbox exec` to launch a new client, or recreate the sandbox to restart its main process." + ); + println!( + " For live value-only rotation, configure stable_placeholder: true in the credential profile before starting the workload, or use gateway-managed refresh." + ); + } + Ok(()) } pub async fn provider_delete( diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index 538eebf060..f2fe08d4c8 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -2290,6 +2290,126 @@ async fn provider_readiness_update_with_no_targets_preserves_saved_success() { } } +#[tokio::test] +async fn provider_update_explains_static_rotation_without_profile_read_permission() { + let server = run_server().await; + seed_readiness_provider(&server).await; + server + .state + .deny_provider_reads + .store(true, Ordering::SeqCst); + server + .state + .profile_read_errors + .lock() + .await + .insert("openai".to_string(), Code::PermissionDenied); + for sandbox in ["sandbox-a", "sandbox-b"] { + server + .state + .sandbox_providers + .lock() + .await + .insert(sandbox.to_string(), vec![READINESS_PROVIDER.to_string()]); + script_readiness( + &server, + sandbox, + vec![Ok(readiness_status( + ProviderReadinessState::Ready, + ProviderReadinessReason::Unspecified, + true, + ))], + ) + .await; + } + let credential = format!("OPENAI_API_KEY={SYNTHETIC_READINESS_CREDENTIAL}"); + for wait in [false, true] { + let mut args = vec![ + "provider", + "update", + READINESS_PROVIDER, + "--credential", + &credential, + ]; + if wait { + args.extend(["--wait", "--timeout", "2"]); + } + let output = run_readiness_cli(&server, &args).await; + assert!(output.status.success(), "{output:?}"); + assert_readiness_output_redacted(&output); + let stdout = String::from_utf8_lossy(&output.stdout); + assert_eq!(stdout.matches("Ordinary static credentials:").count(), 1); + assert!(stdout.contains("existing processes keep their old references, even with --wait")); + assert!(stdout.contains("openshell sandbox exec")); + assert!(stdout.contains("recreate the sandbox")); + assert!(stdout.contains("stable_placeholder: true")); + assert!(stdout.contains("gateway-managed refresh")); + assert!(stdout.contains(if wait { "ready" } else { "persisted" })); + assert!(server.state.profile_read_requests.lock().await.is_empty()); + } +} + +#[tokio::test] +async fn provider_update_rotation_guidance_preserves_structured_output() { + let server = run_server().await; + seed_readiness_provider(&server).await; + server.state.sandbox_providers.lock().await.insert( + "sandbox-a".to_string(), + vec![READINESS_PROVIDER.to_string()], + ); + for format in ["json", "yaml"] { + let output = run_readiness_cli( + &server, + &[ + "provider", + "update", + READINESS_PROVIDER, + "--credential", + "OPENAI_API_KEY=updated-fixture", + "--output", + format, + ], + ) + .await; + assert!(output.status.success(), "{output:?}"); + assert!(output.stderr.is_empty()); + let value: serde_json::Value = if format == "json" { + serde_json::from_slice(&output.stdout).expect("update JSON") + } else { + serde_yml::from_slice(&output.stdout).expect("update YAML") + }; + assert_eq!(value["targets"].as_array().unwrap().len(), 1); + assert_eq!(value["targets"][0]["state"], "persisted"); + assert!(!String::from_utf8_lossy(&output.stdout).contains("Ordinary static credentials:")); + assert_readiness_output_redacted(&output); + } +} + +#[tokio::test] +async fn provider_update_omits_rotation_guidance_without_affected_processes() { + let server = run_server().await; + seed_readiness_provider(&server).await; + for (attached, flag, value) in [ + (false, "--credential", "OPENAI_API_KEY=updated-fixture"), + (true, "--config", "OPENAI_BASE_URL=https://example.com/v1"), + ] { + if attached { + server.state.sandbox_providers.lock().await.insert( + "sandbox-a".to_string(), + vec![READINESS_PROVIDER.to_string()], + ); + } + let output = run_readiness_cli( + &server, + &["provider", "update", READINESS_PROVIDER, flag, value], + ) + .await; + assert!(output.status.success(), "{output:?}"); + assert!(!String::from_utf8_lossy(&output.stdout).contains("Ordinary static credentials:")); + assert_readiness_output_redacted(&output); + } +} + #[tokio::test] async fn provider_profile_permission_denial_preserves_safe_workspace_guidance() { let server = run_server().await; diff --git a/docs/how-it-works/providers/overview.mdx b/docs/how-it-works/providers/overview.mdx index 73f209738e..d8c760d01d 100644 --- a/docs/how-it-works/providers/overview.mdx +++ b/docs/how-it-works/providers/overview.mdx @@ -228,7 +228,7 @@ The update records which sandboxes are attached before saving the new credential Credential refresh status tells you whether OpenShell obtained credentials. [Provider readiness](/how-it-works/providers/profiles#inspect-provider-readiness) tells you whether the sandbox applied the credentials, policy, and environment for new processes. It does not test access to a backend model or cancel requests already sent upstream. -After a static credential update completes, launch a new client process to use the updated reference. An existing process keeps its revision-scoped reference; readiness does not make that old reference resolve the new value. Acknowledged detach revokes retained references and removes them from environments for future launches. +After an ordinary static credential update reaches readiness, use `openshell sandbox exec` to launch a new client process with the updated reference, or recreate the sandbox to restart its main process. An existing process keeps its revision-scoped reference, even with `--wait`. The update command reminds you of this in its default table output when credentials change on an attached provider. For live value-only rotation, configure a [stable placeholder](/how-it-works/providers/profiles#keep-an-externally-updated-placeholder-stable) before starting the workload, or use [gateway-managed refresh](/how-it-works/providers/profiles#configure-credential-refresh). Acknowledged detach revokes retained references and removes them from environments for future launches. Set or clear a credential expiry timestamp: diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index c4ce907671..a6fc62304c 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -866,7 +866,7 @@ openshell sandbox exec work-session -- The application owns the native base URL, model, request shape, and timeout. Launch a new process after attachment readiness so it inherits the installed provider environment. Use the `debug-inference` skill for endpoint, policy, credential-binding, or migration failures. -For an ordinary static provider update, wait for the update and launch a new client process to obtain the new reference. Do not claim that readiness updates the environment of an existing process or retargets its old reference. Acknowledged detach revokes retained references and removes them from future process environments. +For an ordinary static provider update, wait for readiness and use `openshell sandbox exec` to launch a new client with the new reference, or recreate the sandbox to restart its main process. The update command includes this reminder in table output for credential updates with attached sandboxes. Do not claim that readiness updates the environment of an existing process or retargets its old reference. For live value-only rotation, configure `stable_placeholder: true` before starting the workload, or use gateway-managed refresh. Acknowledged detach revokes retained references and removes them from future process environments. ## Workflow 8: Gateway Management