Skip to content

feat(tui): add config key support to provider create/update forms - #1

Closed
letv1nnn wants to merge 13 commits into
mainfrom
tui-provider-config-keys-creation
Closed

feat(tui): add config key support to provider create/update forms#1
letv1nnn wants to merge 13 commits into
mainfrom
tui-provider-config-keys-creation

Conversation

@letv1nnn

@letv1nnnletv1nnn commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Summary

Add config key (config map) support to the TUI's Create Provider and Update Provider forms.

Previously, both forms only handled credentials and hardcoded config: HashMap::default(), making it impossible to set provider config keys like ANTHROPIC_BASE_URL or OPENAI_BASE_URL via openshell term. The CLI's --config KEY=VALUE flag worked correctly, only the TUI surface was affected.

Related Issue

NVIDIA#1798

Changes

  • Create Provider form: Added config key/value input section with Tab-based navigation between key and
    value fields, auto-commit on Tab, and Ctrl+D to delete entries
  • Update Provider form: Added config key/value editing with existing config hydrated from the provider
    on form open, preserving config that the user doesn't modify
  • Spawn functions: spawn_create_provider and spawn_update_provider now plumb the form's config map
    into the gRPC request instead of sending empty maps

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

Screenshots

You can run cargo run -p openshell-cli -- term to try it out yourself.

Provider Details now lists config keys:

image

Updating Provider Details

imageimage

@letv1nnnletv1nnn closed this Jul 8, 2026
@letv1nnnletv1nnn reopened this Jul 8, 2026
@letv1nnn
letv1nnnforce-pushed the tui-provider-config-keys-creation branch from 7cc376a to 9b02ea9CompareJuly 8, 2026 13:17

@r3v5r3v5 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great work, @letv1nnn ! I left two comments though.

Overall: Solid fix for a real destructive bug. Config keys now editable in both create and update forms, existing config preserved on update. Good use of IndexMap for insertion-order stability.

Comment threadcrates/openshell-tui/src/app.rs Outdated
Comment threadcrates/openshell-tui/src/app.rs
letv1nnn added 12 commits July 21, 2026 15:38
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
… window
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
@letv1nnn
letv1nnnforce-pushed the tui-provider-config-keys-creation branch from 9bba173 to 4d32599CompareJuly 21, 2026 15:13
…_enabled
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
@letv1nnn
letv1nnnforce-pushed the tui-provider-config-keys-creation branch from 4d32599 to c34a3cbCompareJuly 21, 2026 20:19
@letv1nnn

Copy link
Copy Markdown
OwnerAuthor

feat(tui): add config key support to provider create/update forms was merged, so closing the local pr

letv1nnn pushed a commit that referenced this pull request Aug 7, 2026
…VIDIA#2271)
* feat(sdk/go): add Go SDK foundation, types, and sandbox client (A)
Add the Go SDK module with the full API contract and a working sandbox
client as the first vertical slice. All other resource clients are present
as stubs returning Unimplemented errors, to be replaced with real
implementations in subsequent PRs.
Contents:
- Module setup (go.mod, Makefile, mise.toml)
- All domain types (types/ package)
- Full ClientInterface with all sub-client accessors
- Shared infrastructure (errors, auth, gRPC connection, logging)
- Sandbox client with converter and tests (fully functional)
- Stub clients for remaining resources (exec, file, health, provider,
profile, config, refresh, policy, service, ssh, tcp)
Part of the Go SDK decomposition plan (NVIDIA#2270).
Implements NVIDIA#2044.
* fix(sdk/go): address review feedback on PR NVIDIA#2271
- Make scheme parsing drive transport selection: http:// uses plaintext
gRPC, https:// or no scheme uses TLS. Add regression tests.
- Add Resources and DriverConfig fields to SandboxTemplate and update
both converter directions (SandboxFromProto/SandboxSpecToProto).
- Regenerate proto bindings from current canonical proto sources to
eliminate drift (SigV4/MCP fields, params matchers, reserved fields).
- Run gofmt/goimports on all handwritten Go files.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* fix(sdk/go): address principal engineer review findings
- Remove dead boolCount function that would fail golangci-lint (#1)
- Emit EventAdded for the first watch event instead of EventModified,
matching k8s watch semantics (#7)
- Add mutex locking to all mock server methods that access the shared
sandboxes map, fixing latent race conditions (NVIDIA#12)
- Skip HealthCheck integration test that calls an unimplemented stub (NVIDIA#13)
- Scope doc.go examples: mark sections for sub-clients not yet available
in this PR with "available in a future release" (#4)
- Document Config.Timeout/RetryPolicy/Logger and WatchOptions fields
as reserved for future use (#2, #6)
Signed-off-by: Roland Huß <rhuss@redhat.com>
* refactor(sdk/go): migrate mise config to centralized task include
Move Go SDK mise configuration from standalone sdk/go/mise.toml into
the project's centralized pattern:
- Add Go tools (go, golangci-lint, protoc-gen-go, protoc-gen-go-grpc)
to root mise.toml [tools] section
- Create tasks/go.toml with all SDK tasks using go: namespace prefix
and dir=sdk/go for working directory
- Update sdk/go/Makefile to reference namespaced task names
- Update proto:sync default path for monorepo layout
Addresses review feedback from drew on PR NVIDIA#2271 regarding mise
convention alignment.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* refactor(sdk/go): remove UPSTREAM_VERSION standalone repo artifact
Remove sdk/go/proto/UPSTREAM_VERSION file and its exclusion from
proto:check. This was a leftover from the standalone repo prototype.
In a monorepo, proto drift is detectable via git diff between
sdk/go/proto/ and proto/ directly.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* refactor(sdk/go): switch proto generation from protoc to buf
Replace raw protoc invocations with buf for Go SDK proto code generation,
aligning with the TS SDK approach (PR NVIDIA#2122).
- Add repo-level buf.yaml declaring proto/ as the buf module with lint
and breaking change detection config
- Add sdk/go/buf.gen.yaml configuring buf to generate Go code directly
from root proto/ (no more vendored .proto copies)
- Delete vendored .proto source files from sdk/go/proto/
- Rewrite go:proto:gen and go:proto:check mise tasks to use buf
- Remove go:proto:sync and go:proto:clean tasks (no longer needed)
- Add proto target to sdk/go/Makefile
- Add buf 1.72.0 to root mise.toml tool dependencies
- Include options.proto in generation (was stripped from vendored copies)
- Regenerate all .pb.go files via the new buf pipeline
Signed-off-by: Roland Huß <rhuss@redhat.com>
* test(sdk/go): add proto-converter field coverage detection
Use protobuf reflection to enumerate all fields on key proto messages
(SandboxSpec, SandboxTemplate, SandboxStatus, SandboxCondition,
SandboxPolicy) and compare against explicit handled/skipped sets in the
converter tests.
Unhandled fields produce warnings (t.Log), not failures, so proto
contributors are not forced to fix SDK converters in the same PR. Stale
entries in the handled set (removed proto fields) do fail, since they
indicate the converter references something that no longer exists.
A follow-up CI workflow will create GitHub issues when converter drift
lands on main.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* fix(sdk/go): bump Go to 1.26 and fix errcheck lint violations
The upstream go.mod now has `toolchain go1.26.4`, which requires Go 1.26
to build golangci-lint. Bump the mise.toml Go version from 1.25 to 1.26
and wrap deferred Close() calls in test helpers to satisfy errcheck.
Assisted-By: 🤖 Claude Code
* feat(sdk/go): add ObjectMeta fields (annotations, workspace, deletion_timestamp)
Add three new proto ObjectMeta fields to Sandbox and Provider domain
types: Annotations (map), Workspace (string), and DeletionTimestamp
(*time.Time). Update converters in both directions, deep-copy maps at
the proto/SDK boundary, and add TimeFromMillisPtr/MillisFromTimePtr
helper functions.
Assisted-By: 🤖 Claude Code
* chore(sdk/go): regenerate proto bindings after rebase
Pick up workspace fields from upstream PR NVIDIA#2445 (Wire authorization
into workspace model). All request messages now include workspace
parameter in the generated Go bindings.
Assisted-By: 🤖 Claude Code
* feat(sdk/go): add workspace scoping to all RPC interfaces
Add workspace parameter to every sandbox-scoped RPC method across all
interfaces (Sandbox, Exec, File, Service, SSH, TCP, Config, Policy,
Provider, Profile, Refresh). The workspace string is passed as the
second parameter after ctx, following the convention workspace then
resource-name.
Key changes:
- SandboxInterface: all 10 methods gain workspace parameter
- sandbox_client.go: passes Workspace field in every proto request
- ListOptions: add AllWorkspaces field for cross-workspace queries
- All stub interfaces updated to match new signatures
- All sandbox client tests updated with "default" workspace
Assisted-By: 🤖 Claude Code
* chore(sdk/go): remove coverage.out from tracking
Assisted-By: 🤖 Claude Code
* fix(sdk/go): address review feedback from mrunalp
- Add RefreshStrategyAWSStsAssumeRole to match proto enum value 6,
fulfilling the "all domain types upfront" contract
- Wrap context.DeadlineExceeded and context.Canceled in StatusError
so IsDeadlineExceeded() and IsCancelled() helpers work correctly
- Return error from mapToStruct/SandboxSpecToProto instead of silently
discarding structpb.NewStruct failures on invalid template maps
Signed-off-by: Roland Huss <rhuss@redhat.com>
* fix(sdk/go): address remaining review items
- Wire go:ci into root ci task so SDK is tested in repository CI
- Fix gofmt formatting on converter files
- Add goimports to mise.toml tools
- Add coverage.out to .gitignore
- Add Go SDK section to AGENTS.md and CONTRIBUTING.md
- Add regression tests for context-error wrapping (IsDeadlineExceeded,
IsCancelled) and invalid template map rejection
- Remove panic from SandboxToProto, return error instead
Signed-off-by: Roland Huss <rhuss@redhat.com>
* fix(sdk/go): pin goimports version and update lockfile
Pin goimports to 0.48.0 instead of "latest" and regenerate mise.lock
to include the new entry.
Signed-off-by: Roland Huss <rhuss@redhat.com>
* fix(sdk/go): TLS.Insecure means skip-verify, not plaintext
Align TLS.Insecure semantics with the Rust SDK: Insecure: true now
uses TLS with InsecureSkipVerify (skip cert verification) instead of
switching to plaintext. Only the http:// scheme triggers plaintext.
This fixes token auth against dev/k3d gateways: StaticToken and
RefreshableToken require transport security, which real TLS (even
with InsecureSkipVerify) satisfies, but plaintext does not.
For http:// + token auth (dev gateways without TLS), wrap the auth
provider to override RequireTransportSecurity, matching the Rust
SDK's behavior where http:// accepts any auth mode.
Transport decision table (matches Rust SDK crates/openshell-sdk):
http:// + any TLS config -> plaintext (TLS config ignored)
https:// + Insecure: true -> TLS, skip cert verify
https:// + Insecure: false -> TLS, full verification
no scheme -> same as https://
Signed-off-by: Roland Huss <rhuss@redhat.com>
* feat(sdk/go): add missing policy proto fields
Add 6 previously silently dropped fields to the network policy types
and converters, preventing security-relevant data loss on round-trip:
NetworkEndpoint fields 19-23:
- CredentialSigning: SigV4 re-signing mode
- SigningService: AWS service name for SigV4
- SigningRegion: AWS region override for SigV4
- JsonRpcMaxBodyBytes: JSON-RPC body inspection limit
- Mcp: MCP-specific policy options (new McpOptions type)
L7Allow and L7DenyRule field 9:
- Params: MCP params matcher map for tools/call filtering
New type McpOptions with StrictToolNames and AllowAllKnownMcpMethods
optional booleans matching the proto definitions.
Signed-off-by: Roland Huss <rhuss@redhat.com>
* fix(sdk/go): enforce coverage test and extend to policy messages
Change coverage_test.go from t.Logf (silent) to t.Errorf so that
unhandled proto fields fail the test immediately. Add coverage tests
for NetworkEndpoint (23 fields), L7Allow (8 fields), L7DenyRule
(8 fields), and McpOptions (2 fields).
Any new proto field that is not in the handled set or explicitly
skipped now breaks the build, closing the silent-drift gap.
Signed-off-by: Roland Huss <rhuss@redhat.com>
* ci(sdk/go): add Go SDK job to branch-checks workflow
Add a Go SDK job to branch-checks.yml that runs mise run go:ci
(lint, build, test, proto-check, docs-check) on every PR. This
ensures the SDK is tested in CI, not just locally.
Signed-off-by: Roland Huss <rhuss@redhat.com>
* fix(sdk/go): address should-fix review items
#6 Fix broken godoc examples: add workspace parameter to all method
calls in doc.go that were broken after workspace scoping.
#7 Add Err field to Event[T]: Watch error events now carry the
underlying error instead of discarding it.
#8 Separate Unauthenticated from PermissionDenied: add
ErrorUnauthenticated code and IsUnauthenticated() helper. gRPC
Unauthenticated (401) now maps to its own code instead of
collapsing into PermissionDenied (403).
#9 Add Unwrap to StatusError: replace dead Details field with Cause
error field. StatusError.Unwrap() returns Cause, enabling
errors.Is/As unwrapping. FromGRPCError and contextError both
populate Cause.
Signed-off-by: Roland Huss <rhuss@redhat.com>
* ci(sdk/go): add go:format:check to CI pipeline
Add gofmt format verification to go:ci. Catches unformatted Go files
before they reach the PR. Fix formatting on coverage_test.go.
Signed-off-by: Roland Huss <rhuss@redhat.com>
* chore(sdk/go): remove Makefile in favor of mise tasks
All build, lint, test, and proto-gen tasks are already defined in
tasks/go.toml and invoked via mise. The Makefile was a leftover
that duplicated this and raised questions in review.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* feat(sdk/go): sync proto bindings and add credential handle support
Regenerate Go proto bindings after rebase to pick up new
CredentialHandle message and Provider.credential_handles and
profile_workspace fields from upstream. Add domain types, converter
support, and proto field coverage tests for Provider and
CredentialHandle.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* fix(sdk/go): reject plaintext auth leak and fix watch error handling
Reject http:// addresses when the auth provider requires transport
security instead of silently stripping the requirement. Remove the
insecureAuthWrapper that overrode RequireTransportSecurity.
Fix watch stream error handling: use blocking send for terminal
errors so they are never silently dropped when the channel is full,
and wrap mid-stream errors with converter.FromGRPCError so SDK error
helpers like IsUnavailable work on watch Event.Err.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* fix(sdk/go): address review findings from multi-agent code review
- WaitReady now detects SandboxDeleting phase and returns immediately
instead of polling indefinitely
- Watch goroutine defers streamCancel() to prevent context leaks
- Fix StopOnTerminal=false test to keep stream open (was wrong-reason
pass due to stream ending, not StopOnTerminal logic)
- Add EventDeleted test covering the Deleting phase branch
- Add provider converter unit tests for CredentialHandle round-trip,
nil handling, and empty maps
Signed-off-by: Roland Huß <rhuss@redhat.com>
---------
Signed-off-by: Roland Huß <rhuss@redhat.com>
Signed-off-by: Roland Huss <rhuss@redhat.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@letv1nnn@r3v5