diff --git a/AGENTS.md b/AGENTS.md index f2b01d2..bc5332e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,6 +1,60 @@ # Contributing -## Principles +## Coding Guidelines + +**Tradeoff:** These guidelines bias toward caution over speed. For trivial tasks, use judgment. + +### Think Before Coding + +Don't assume. Don't hide confusion. Surface tradeoffs. + +- State your assumptions explicitly. If uncertain, ask. +- If multiple interpretations exist, present them - don't pick silently. +- If a simpler approach exists, say so. Push back when warranted. +- If something is unclear, stop. Name what's confusing. Ask. + +### Simplicity First + +Minimum code that solves the problem. Nothing speculative. + +- No features beyond what was asked. +- No abstractions for single-use code. +- No "flexibility" or "configurability" that wasn't requested. +- No error handling for impossible scenarios. +- If you write 200 lines and it could be 50, rewrite it. + +### Surgical Changes + +Touch only what you must. Clean up only your own mess. + +When editing existing code: +- Don't "improve" adjacent code, comments, or formatting. +- Don't refactor things that aren't broken. +- Match existing style, even if you'd do it differently. +- If you notice unrelated dead code, mention it - don't delete it. + +When your changes create orphans: +- Remove imports/variables/functions that YOUR changes made unused. +- Don't remove pre-existing dead code unless asked. + +The test: Every changed line should trace directly to the user's request. + +### Goal-Driven Execution + +Define success criteria. Loop until verified. + +- "Add validation" → "Write tests for invalid inputs, then make them pass" +- "Fix the bug" → "Write a test that reproduces it, then make it pass" +- "Refactor X" → "Ensure tests pass before and after" + +For multi-step tasks, state a brief plan: +``` +1. [Step] → verify: [check] +2. [Step] → verify: [check] +3. [Step] → verify: [check] +``` + +## Project Principles 1. **Simplicity** — fewer scripts, fewer moving parts, less code. If something can be a single command, don't wrap it in a script. diff --git a/README.md b/README.md index 3197465..ecdbd5d 100644 --- a/README.md +++ b/README.md @@ -1,83 +1,78 @@ # OpenShell Harness -Deploy AI agent sandboxes on Podman (local) or OpenShift using [OpenShell](https://github.com/NVIDIA/OpenShell). Each sandbox gets: +A gateway harness for [OpenShell](https://github.com/NVIDIA/OpenShell). One command to a working AI agent sandbox — gateway deployed, providers registered, credentials validated, sandbox running. -- **Claude Code** via Google Vertex AI (`inference.local` routing) -- **Jira/Confluence** via mcp-atlassian MCP server -- **Gmail, Calendar, Drive** via gws CLI -- **GitHub** via gh CLI -- Network policy enforcement per sandbox +Without this tool, setting up a sandbox means manually deploying an OpenShell gateway (Helm install, mTLS cert extraction, SCC grants), manually registering providers with credentials scattered across env vars and files, and debugging when credentials expire or proxies misconfigure. The harness handles all of that. -## Prerequisites +## What it does -- [OpenShell CLI](https://github.com/NVIDIA/OpenShell) (`openshell`) -- Python 3.11+ (for TOML parsing) -- Podman (local) or kubectl + helm (OCP) -- `gcloud auth application-default login` (for Vertex AI) +The harness wraps `openshell` — it doesn't replace it. It adds orchestration, validation, and configuration management across three domains: -Optional: `gws` CLI (Google Workspace), `bats` (for unit tests) +| Domain | What | Config | +|--------|------|--------| +| **Infrastructure** | Deploy the gateway (local Podman or remote OpenShift), Helm, mTLS, RBAC | `openshell.toml` | +| **Providers** | Register credential providers (Vertex AI, GitHub, Atlassian), validate inputs | `providers.toml` | +| **Sandbox** | Create sandboxes from profiles, stage files, connect | `profiles/*.toml` | -## Setup +Each sandbox gets: +- Claude Code via Vertex AI (`inference.local` gateway routing) +- Jira and Confluence via mcp-atlassian MCP server +- Gmail, Calendar, Drive via gws CLI +- GitHub via gh CLI +- Network policy enforcement per sandbox -```bash -# Add harness CLI to PATH -export PATH="$PWD/bin:$PATH" +## Prerequisites -# See available commands -harness -``` +- [OpenShell CLI](https://github.com/NVIDIA/OpenShell) (`openshell`) +- Podman (local) or kubectl + helm (OpenShift) +- `gcloud auth application-default login` (Vertex AI) -## Quick Start (Local) +Optional: `gws` CLI (Google Workspace), `bats` (tests) -```bash -# Install OpenShell if you haven't -curl -LsSf https://raw.githubusercontent.com/NVIDIA/OpenShell/main/install.sh | sh +## Quick Start +```bash # Set credentials export GITHUB_TOKEN="ghp_..." export JIRA_API_TOKEN="..." export ANTHROPIC_VERTEX_PROJECT_ID="my-project" -export CLOUD_ML_REGION="us-east5" -# Create a sandbox (deploys gateway + registers providers if needed) +# Local — deploy gateway, register providers, create sandbox harness new --local -``` - -## Quick Start (OpenShift) -```bash -# Create a sandbox on the cluster +# OpenShift — same flow, remote cluster harness new --remote + +# Reconnect to a running sandbox +harness connect ``` -## CLI Reference +## Commands ``` -harness new [--local|--remote] [--profile NAME] [SANDBOX_NAME] - Create a new sandbox. Auto-deploys gateway and registers providers if needed. +harness new [--local|--remote] [--profile NAME] + Full flow: deploy gateway + register providers + create sandbox. -harness connect [SANDBOX_NAME] +harness connect [NAME] Reconnect to a running sandbox. harness deploy [--local|--remote] Deploy or verify the gateway without creating a sandbox. -harness teardown [--sandboxes] [--providers] [--k8s] - Tear down sandboxes, providers, or k8s resources. - -harness preflight - Check environment prerequisites. - -harness providers +harness providers [--force] Register providers with the gateway. -harness test [podman|ocp|all] [--full] - End-to-end validation. +harness preflight [--strict] + Check environment prerequisites (credentials, CLI tools, gateway). + +harness teardown [--sandboxes] [--providers] [--k8s] + Tear down sandboxes, providers, or cluster resources. + At least one flag required. ``` ## Profiles -Sandboxes are configured via `profiles/*.toml`: +Sandboxes are configured via TOML profiles: ```toml # profiles/default.toml @@ -91,58 +86,63 @@ ANTHROPIC_BASE_URL = "https://inference.local" JIRA_URL = "https://mysite.atlassian.net" ``` -Use a specific profile: `harness new --profile coder` +Use a specific profile: `harness new --profile research` -## Testing +## Why a sandbox -```bash -harness test podman --full # full local validation -harness test ocp --full # full OCP validation -bats test/preflight.bats # unit tests (29 tests) -make test # build images + test both +Compared to running Claude Code locally: + +- **Credential isolation** — the sandbox never sees real API tokens (proxy-resolved placeholders) +- **Network policy** — per-binary egress rules control which processes reach which hosts +- **Reproducible environment** — pinned tool versions, same setup across machines and team members +- **Team sharing** — OpenShift deployment with mTLS, shared gateway, per-user sandboxes + +## Architecture + +``` +Your machine OpenShift cluster +┌──────────┐ ┌──────────────────────────────┐ +│ harness │ Route (mTLS) │ Gateway (StatefulSet) │ +│ CLI ├───────────────────►│ ├─ gRPC API │ +│ │ │ ├─ inference.local proxy │ +└──────────┘ │ ├─ Provider credential store │ + │ └─ OAuth token refresh │ + │ │ + │ Sandbox Pods │ + │ ├─ Claude Code → Vertex AI │ + │ ├─ mcp-atlassian │ + │ ├─ gws CLI │ + │ ├─ gh CLI │ + │ └─ L7 network proxy │ + └──────────────────────────────┘ ``` -## Files +## Project Layout | Path | Purpose | |------|---------| -| `bin/harness` | CLI entry point | -| `bin/scripts/` | Subcommand scripts (new, deploy, teardown, etc.) | -| `bin/scripts/lib/` | Shared libraries (profile parsing, providers, common) | -| `profiles/default.toml` | Default sandbox profile | -| `providers.toml` | Provider definitions (env/file/check inputs) | -| `openshell.toml` | Which providers to enable, upstream version pin | -| `sandbox/` | Sandbox image (Dockerfile, startup.sh, policy.yaml, CLAUDE.md) | -| `sandbox/launcher/` | In-cluster launcher image (for OCP sandboxes) | -| `test/` | Tests (preflight.bats, test-flow.sh) | -| `values-ocp.yaml` | Helm values for OpenShift deployment | -| `AGENTS.md` | Project principles and workaround tracking | - -## Why Use a Sandbox? +| `main.go`, `cmd/` | CLI commands (Go) | +| `internal/gateway/` | OpenShell CLI wrapper (Gateway interface) | +| `internal/k8s/` | kubectl/helm/oc runner with retry | +| `internal/profile/` | Profile TOML parsing | +| `internal/preflight/` | Provider prerequisite checks | +| `profiles/` | Sandbox profiles (TOML) | +| `providers.toml` | Provider catalog (inputs, prerequisites) | +| `sandbox/` | Sandbox image (Dockerfile, startup, policy, CLAUDE.md) | +| `sandbox/launcher/` | In-cluster launcher for OCP sandboxes | +| `sandbox/profiles/` | OpenShell provider type profiles (YAML) | +| `deploy/` | K8s manifests (RBAC, route) | +| `test/` | Tests (bats preflight, test-flow.sh integration) | -Compared to running Claude Code locally: -- **Credential isolation** — sandbox never sees real API tokens (proxy-resolved placeholders) -- **Network policy** — per-binary egress rules (policy.yaml controls which processes reach which hosts) -- **Reproducible environment** — pinned tool versions in Dockerfile -- **Team sharing** — OCP deployment with mTLS, shared gateway, per-user sandboxes - -## Architecture +## Testing +```bash +make validate # full matrix: {bash,go} x {podman,ocp} +make test # build images + test +bats test/preflight.bats # 29 preflight unit tests +go test ./... # Go unit tests ``` -Your Mac OpenShift Cluster -┌──────────┐ ┌──────────────────────────────┐ -│ harness │ OpenShift Route │ Gateway (StatefulSet) │ -│ CLI ├──────────────────►│ ├─ gRPC API │ -│ │ TLS passthrough │ ├─ inference.local proxy │ -│ │ mTLS :443 │ ├─ Provider credential store │ -└──────────┘ │ └─ OAuth token refresh │ - │ │ - │ Sandbox Pods │ - │ ├─ Claude Code → inference │ - │ │ .local → Vertex AI │ - │ ├─ mcp-atlassian │ - │ ├─ gws CLI │ - │ ├─ gh CLI │ - │ └─ Network proxy │ - └──────────────────────────────┘ -``` + +## Design + +The harness is a thin wrapper that should shrink over time. As OpenShell adds native support for features the harness currently bridges (GWS credentials, provider config injection, in-cluster sandbox creation), the custom code gets replaced by upstream. See [AGENTS.md](AGENTS.md) for workaround tracking and [docs/design.md](docs/design.md) for the full design document. diff --git a/cmd/deploy.go b/cmd/deploy.go index b65ba5b..4b4de6e 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -16,6 +16,11 @@ import ( "github.com/spf13/cobra" ) +var ( + sccPrivilegedSAs = []string{"openshell", "openshell-sandbox", "default"} + secretNames = []string{"openshell-gws", "openshell-atlassian"} +) + func NewDeployCmd(harnessDir, cli string) *cobra.Command { var ( local bool @@ -61,7 +66,7 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kc, clusterRunner k8s.R if chartVersion == "" { cfg, _ := preflight.LoadConfig(filepath.Join(harnessDir, "openshell.toml")) if cfg != nil { - chartVersion = cfg.ChartVersion + chartVersion = cfg.Upstream.ChartVersion } } if chartVersion == "" { @@ -94,7 +99,7 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kc, clusterRunner k8s.R // Step 3: OpenShift SCCs (best-effort — oc may not exist on non-OpenShift) status.Step(3, "Granting OpenShift SCCs") - for _, sa := range []string{"openshell", "openshell-sandbox", "default"} { + for _, sa := range sccPrivilegedSAs { kc.RunOC(ctx, "adm", "policy", "add-scc-to-user", "privileged", "-z", sa, "-n", namespace) } kc.RunOC(ctx, "adm", "policy", "add-scc-to-user", "anyuid", "-z", "openshell", "-n", namespace) @@ -109,10 +114,7 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kc, clusterRunner k8s.R // Step 4: Helm install status.Step(4, "Deploying gateway via Helm") - sandboxImage := os.Getenv("SANDBOX_IMAGE") - if sandboxImage == "" { - sandboxImage = "quay.io/rcochran/openshell:sandbox" - } + sandboxImage := envOr("SANDBOX_IMAGE", "quay.io/rcochran/openshell:sandbox") appsDomain, err := clusterRunner.GetJSONPath(ctx, "ingresses.config.openshift.io/cluster", "{.spec.domain}") if err != nil || appsDomain == "" { @@ -152,10 +154,7 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kc, clusterRunner k8s.R // Step 6: CLI gateway config status.Step(6, "Configuring CLI gateway") - gatewayName := os.Getenv("GATEWAY_NAME") - if gatewayName == "" { - gatewayName = "openshell-remote-ocp" - } + gatewayName := envOr("GATEWAY_NAME", "openshell-remote-ocp") gatewayURL := fmt.Sprintf("https://%s:443", routeHost) // Remove existing gateways for this host @@ -223,13 +222,11 @@ func deployLocal(gw gateway.Gateway) error { } status.Section("Container Runtime") - podmanPath, _ := exec.LookPath("podman") - if podmanPath == "" { + if _, err := exec.LookPath("podman"); err != nil { status.Fail("Podman not found") return fmt.Errorf("podman is required") } - cmd := exec.Command("podman", "--version") - out, _ := cmd.Output() + out, _ := exec.Command("podman", "--version").Output() status.OKf("Podman: %s", strings.TrimSpace(string(out))) status.Section("Gateway") diff --git a/cmd/new.go b/cmd/new.go index 8cb2510..8914942 100644 --- a/cmd/new.go +++ b/cmd/new.go @@ -11,6 +11,7 @@ import ( "github.com/robbycochran/harness-openshell/internal/gateway" "github.com/robbycochran/harness-openshell/internal/k8s" + "github.com/robbycochran/harness-openshell/internal/preflight" "github.com/robbycochran/harness-openshell/internal/profile" "github.com/robbycochran/harness-openshell/internal/status" "github.com/spf13/cobra" @@ -144,8 +145,8 @@ func newRemote(harnessDir string, gw gateway.Gateway, profileName, sandboxName s // 3. Clean up old job (best-effort — may not exist) jobName := "sandbox-" + cfg.Name - kc.RunKubectl(ctx, "delete", "job", jobName, "--force", "--grace-period=0") - kc.RunKubectl(ctx, "delete", "pod", "-l", "job-name="+jobName, "--force", "--grace-period=0") + kc.RunKubectl(ctx, "delete", "job", jobName, "--grace-period=30") + kc.RunKubectl(ctx, "delete", "pod", "-l", "job-name="+jobName, "--grace-period=30") // 4. Apply launcher Job job := map[string]any{ @@ -280,12 +281,30 @@ func newLocal(opts newLocalOpts) error { status.Warn("no providers available — run: harness providers") } - // 5. Stage files - harnessUploadDir, err := os.MkdirTemp("", "harness-") + // 5. Inject non-secret provider env vars into sandbox env + providersPath := filepath.Join(opts.harnessDir, "providers.toml") + if allProviders, err := preflight.LoadProviders(providersPath); err == nil { + providerEnv := preflight.ProviderEnvVars(allProviders, cfg.Providers) + if cfg.Env == nil { + cfg.Env = make(map[string]string) + } + for k, v := range providerEnv { + if _, exists := cfg.Env[k]; !exists { + cfg.Env[k] = v + } + } + } + + // 6. Stage files + // The upload preserves the source dir name as a subdirectory at the destination. + // startup.sh expects files at /sandbox/.config/openshell/sandbox.env, so the + // staging dir must be named "openshell". + tmpParent, err := os.MkdirTemp("", "harness-") if err != nil { return fmt.Errorf("creating staging dir: %w", err) } - defer os.RemoveAll(harnessUploadDir) + defer os.RemoveAll(tmpParent) + harnessUploadDir := filepath.Join(tmpParent, "openshell") if err := profile.StageHarnessDir(cfg, harnessUploadDir); err != nil { return fmt.Errorf("staging files: %w", err) } diff --git a/cmd/providers.go b/cmd/providers.go index e3c37c2..c5ed4b3 100644 --- a/cmd/providers.go +++ b/cmd/providers.go @@ -1,7 +1,6 @@ package cmd import ( - "bufio" "encoding/json" "fmt" "os" @@ -31,10 +30,7 @@ func NewProvidersCmd(harnessDir, cli string) *cobra.Command { } func registerProviders(harnessDir string, gw gateway.Gateway, force bool) error { - model := os.Getenv("OPENSHELL_MODEL") - if model == "" { - model = "claude-sonnet-4-6" - } + model := envOr("OPENSHELL_MODEL", "claude-sonnet-4-6") // Force mode: require no running sandboxes if force { @@ -88,10 +84,7 @@ func registerProviders(harnessDir string, gw gateway.Gateway, force bool) error adcPath = filepath.Join(home, ".config", "gcloud", "application_default_credentials.json") } project := os.Getenv("ANTHROPIC_VERTEX_PROJECT_ID") - region := os.Getenv("CLOUD_ML_REGION") - if region == "" { - region = "global" - } + region := envOr("CLOUD_ML_REGION", "global") if project == "" { project = readADCProject(adcPath) @@ -174,21 +167,25 @@ func deleteCustomProfiles(harnessDir string, gw gateway.Gateway) { } func extractYAMLID(path string) string { - f, err := os.Open(path) + data, err := os.ReadFile(path) if err != nil { return "" } - defer f.Close() - scanner := bufio.NewScanner(f) - for scanner.Scan() { - line := scanner.Text() - if strings.HasPrefix(line, "id:") { - return strings.TrimSpace(strings.TrimPrefix(line, "id:")) + for _, line := range strings.Split(string(data), "\n") { + if id, ok := strings.CutPrefix(line, "id:"); ok { + return strings.TrimSpace(id) } } return "" } +func envOr(key, fallback string) string { + if v := os.Getenv(key); v != "" { + return v + } + return fallback +} + func fileExists(path string) bool { _, err := os.Stat(path) return err == nil diff --git a/cmd/teardown.go b/cmd/teardown.go index 72a5dd5..82149e1 100644 --- a/cmd/teardown.go +++ b/cmd/teardown.go @@ -165,7 +165,7 @@ func teardownK8s(gw gateway.Gateway, kc, clusterRunner k8s.Runner) { // OpenShift SCCs fmt.Println() status.Section("OpenShift SCCs") - for _, sa := range []string{"openshell", "openshell-sandbox", "default"} { + for _, sa := range sccPrivilegedSAs { kc.RunOC(ctx, "adm", "policy", "remove-scc-from-user", "privileged", "-z", sa, "-n", namespace) } kc.RunOC(ctx, "adm", "policy", "remove-scc-from-user", "anyuid", "-z", "openshell", "-n", namespace) @@ -175,7 +175,7 @@ func teardownK8s(gw gateway.Gateway, kc, clusterRunner k8s.Runner) { // Secrets fmt.Println() status.Section("K8s secrets") - for _, secret := range []string{"openshell-gws", "openshell-atlassian"} { + for _, secret := range secretNames { if _, err := kc.RunKubectl(ctx, "delete", "secret", secret); err == nil { status.OKf("Deleted %s", secret) } else { diff --git a/docs/design.md b/docs/design.md new file mode 100644 index 0000000..53ca0b0 --- /dev/null +++ b/docs/design.md @@ -0,0 +1,297 @@ +# Design: openshell-harness + +## Why this tool exists + +AI agent sandboxes need a consistent, reproducible environment: the right +gateway deployed, the right providers registered with valid credentials, +the right image with the right tools, and the right configuration — every +time, on every machine, for every team member. + +Without this tool, setting up a sandbox means: +- Manually deploying an OpenShell gateway (Helm install with 6 steps, mTLS + cert extraction, SCC grants on OpenShift) +- Manually registering providers (Vertex AI, GitHub, Atlassian) with + credentials scattered across env vars, ADC files, and API tokens +- Debugging when credentials expire, proxies misconfigure, or sandboxes + fail to reach endpoints — with no visibility into what's broken +- Repeating all of this on every new machine, every cluster, every time + someone joins the team + +**Goals:** + +1. **One command to working sandbox.** `harness up` takes a developer from + zero to a running AI agent sandbox — gateway deployed, providers + registered, sandbox created, credentials validated. + +2. **Reproducible environments.** Sandbox profiles define exactly what a + sandbox needs. The same profile produces the same environment regardless + of who runs it or where. + +3. **Credential visibility.** Two-level provider health checking tells you + whether credentials are valid both locally (can you register?) and + inside the sandbox (can the agent actually use them?). No more "it + worked on my machine." + +4. **Clean separation of concerns.** Infrastructure config, provider + management, and sandbox profiles are independent. Changing your Jira + token doesn't require editing sandbox profiles. Switching clusters + doesn't require re-registering providers. + +5. **Thin wrapper, not a platform.** The harness wraps openshell — it + doesn't replace it. Users can drop to `openshell` commands at any time. + The harness adds orchestration, validation, and configuration management + on top. + +## What this tool is + +openshell-harness is a **gateway harness** — it deploys, configures, and manages +an OpenShell gateway so you can create sandboxes through it. The gateway is the +central concept. Providers and sandboxes are things you do through the gateway. + +``` + ┌─────────────┐ + │ gateway │ ← the harness IS this + │ (deploy, │ + │ configure, │ + │ manage) │ + └──────┬───────┘ + │ + ┌───────┴───────┐ + │ │ + providers sandboxes + (register, (create, + validate) connect) +``` + +Every command is implicitly scoped to "the gateway." The tool name is the +noun — `harness deploy` means deploy the gateway, `harness create` means +create a sandbox through it, `harness providers` means register providers +with it. + +## Three domains + +The tool has three domains of concern. Each has its own config, commands, +and code boundary. + +### 1. Infrastructure — "How is the backend deployed?" + +The gateway itself: deploying to local podman or remote OpenShift, Helm +chart management, mTLS, namespace setup, CRDs, RBAC. + +**Config:** `harness.toml` (chart version, namespace, registry) +**Commands:** `deploy`, `down`, `status` +**Code:** `internal/gateway/` (CLI wrapper), k8s deploy/teardown logic + +### 2. Providers — "What's available and ready?" + +Provider catalog, credential validation, registration with the gateway, +and runtime health checking inside sandboxes. + +**Config:** `providers.toml` (catalog with preflight inputs and in-sandbox verification) +**Commands:** `providers register`, `providers list`, `preflight`, `status providers` +**Code:** provider registration, preflight checks + +### 3. Sandbox — "What sandbox do I want?" + +Sandbox profiles, creation, connection, lifecycle management. + +**Config:** `profiles/*.toml` (image, command, env, provider list) +**Commands:** `create`, `connect`, `status` +**Code:** profile parsing, staging, sandbox create/connect + +## Command interface + +All commands are flat — no `gateway X` grouping because the gateway is +implicit in everything the harness does. + +``` +harness deploy [--local|--remote] Deploy the gateway (local is default) +harness create [NAME] [--profile P] Create a sandbox (errors if gateway not ready) +harness connect [NAME] Reconnect to a running sandbox +harness up [--local|--remote] Full flow: deploy → providers → create +harness down [--sandboxes|--providers|--gateway|--all] +harness providers register [--force] Register providers with the gateway +harness providers list List registered providers +harness preflight [--strict] Validate local environment prerequisites +harness status Overview: gateway, providers, sandboxes +harness status providers Deep provider health (local + in-sandbox) +``` + +### Command design principles + +- `create` only creates a sandbox. It does not deploy the gateway or + register providers. If the gateway isn't ready, it errors with guidance. +- `up` is the orchestrator. It chains deploy → providers → create. This is + the "I don't care, just make it work" command. +- `down` is the inverse of `up`. Bare `down` prompts for confirmation. +- `status` is always read-only, always safe. +- Commands mirror openshell verbs where possible (`create` not `new`, + matching `openshell sandbox create`). + +### Naming decisions + +| Current | Proposed | Why | +|---------|----------|-----| +| `new` | `create` | Matches `openshell sandbox create`. "new" is for scaffolding. | +| `deploy` (requires --local) | `deploy` (defaults to local) | Local is the 90% case. | +| `teardown --k8s` | `down --gateway` | `--k8s` is an implementation detail. | +| `openshell.toml` | `harness.toml` | The file configures harness, not openshell. | +| `OPENSHELL_MODEL` etc. | `HARNESS_MODEL` etc. | With backward-compat shims. | +| `sandbox/profiles/` | `sandbox/provider-profiles/` | Disambiguate from sandbox profiles. | + +## Config files + +Three files, one per domain. No duplication. + +### `harness.toml` — infrastructure config + +```toml +# Gateway deployment settings. + +[upstream] +chart-version = "0.0.55" + +# Optional overrides (also settable via env vars): +# namespace = "openshell" # HARNESS_NAMESPACE +# registry = "quay.io/rcochran" # HARNESS_REGISTRY +``` + +Also serves as the harnessDir sentinel (replaces `profiles/default.toml` +for directory detection). + +### `providers.toml` — provider catalog + +Defines all available providers, their preflight inputs (local validation), +and in-sandbox verification commands. + +```toml +[[providers]] +name = "atlassian" +type = "openshell" +description = "Jira and Confluence via mcp-atlassian" +required = false + +# Local preflight checks — run on the user's machine before registration +inputs = [ + { key = "JIRA_API_TOKEN", kind = "env", secret = true }, + { key = "JIRA_URL", kind = "env" }, +] + +# In-sandbox verification — run inside a running sandbox to confirm +# the provider actually works end-to-end +verify = "curl -sf -u $JIRA_USERNAME:$JIRA_API_TOKEN $JIRA_URL/rest/api/2/myself -o /dev/null" +``` + +The provider list in the profile (domain 3) says what a sandbox *wants*. +The provider catalog says what *exists* and how to validate it. +Registration errors surface the gap. + +### `profiles/*.toml` — sandbox profiles + +```toml +name = "agent" +image = "quay.io/rcochran/openshell:sandbox" +command = "claude --bare" +keep = true + +providers = ["github", "vertex-local", "atlassian"] + +[env] +# Sandbox-level env vars only. Provider-specific config (JIRA_URL, etc.) +# flows from provider registration, not from here. +CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS = "1" +``` + +**Principle:** the profile describes the sandbox shape. Provider credentials +and provider-specific env vars are provider concerns, not profile concerns. + +## Provider health: two levels + +Provider validation happens at two levels: + +### Level 1: Local preflight + +"Do I have the credentials to register this provider?" + +Runs on the user's machine. Uses `inputs` from `providers.toml` to check +env vars, files, and external commands. This is what `harness preflight` +does today. + +### Level 2: In-sandbox verification + +"Can the sandbox actually use this provider?" + +Runs inside a running sandbox via `openshell sandbox exec`. Uses the +`verify` field from `providers.toml`. Catches: +- Expired credentials +- Gateway proxy misconfiguration +- Network endpoint unreachable from sandbox +- Env vars staged incorrectly + +### `harness status providers` + +Shows both levels: + +``` +=== Providers === + ✓ github local: GITHUB_TOKEN set sandbox: git ls-remote ok + ✓ vertex-local local: ADC valid sandbox: inference reachable + ✗ atlassian local: JIRA_API_TOKEN set sandbox: curl /myself failed (401) +``` + +If no sandbox is running, only local checks run. The in-sandbox column +shows "no sandbox" instead of pass/fail. + +## Code organization + +The three domains map to internal packages: + +``` +internal/ + gateway/ Domain 1: Gateway CLI wrapper, gateway admin operations + provider/ Domain 2: Provider catalog, preflight, registration (new, from existing preflight/) + profile/ Domain 3: Sandbox profile parsing, staging, env building (exists) + k8s/ Shared: kubectl/helm/oc runner (exists) + status/ Shared: terminal output helpers (exists) +``` + +`cmd/` files are thin — flag parsing and one function call into the +appropriate domain package. Orchestration logic (`up` = deploy → providers +→ create) lives in `cmd/up.go`, composing domain operations. + +### Migration path + +This is not a rewrite. The domains already exist in the code — they're +just tangled in `cmd/new.go` (325 lines spanning all three domains). +The migration is: + +1. Extract provider registration from `cmd/providers.go` into `internal/provider/` +2. Move preflight from `internal/preflight/` into `internal/provider/` (it's provider validation) +3. Slim `cmd/new.go` → `cmd/create.go` (profile parse + sandbox create, ~60 lines) +4. New `cmd/up.go` composes: deploy + providers register + create +5. Rename `cmd/teardown.go` → `cmd/down.go` +6. New `cmd/status.go` reads across all three domains + +Each step is independently committable and testable. + +## Proto migration (deferred) + +The proto migration plan (proto-migration.md) is architecturally sound but +premature. The tool has ~5 profiles and ~4 providers. Schema drift is real +but manageable at this scale. + +**When to do it:** when the tool moves from CLI exec to gRPC, at which +point proto types ARE the request payloads and the migration pays for itself. + +**Interim approach:** if compile-time safety is needed before gRPC, parse +TOML into proto-generated structs internally with a ~50-line mapping layer. +Users keep writing TOML. No textproto, no format change. + +## Open questions + +- Should `harness init` (from release-plan.md) be a separate command or + part of the first `harness deploy` run? +- Should provider-specific env vars (JIRA_URL, JIRA_USERNAME) flow from + `providers.toml` values or from a separate provider config mechanism? +- Should `verify` commands in providers.toml support a timeout field? +- How should `status providers` handle providers that take >5s to verify? diff --git a/docs/profile-concepts.md b/docs/profile-concepts.md new file mode 100644 index 0000000..ad84f3a --- /dev/null +++ b/docs/profile-concepts.md @@ -0,0 +1,228 @@ +# Profile Concepts: OpenShell, Kaiden, and harness-openshell + +Three projects use the word "profile" to mean fundamentally different things. This document maps each concept, identifies where they overlap and diverge, and notes implications for harness-openshell. + +--- + +## 1. OpenShell — Provider Type Profiles + +**Source:** [NVIDIA/OpenShell#896](https://github.com/NVIDIA/OpenShell/issues/896) ("Enhanced Provider Management") + +### What it is + +A **Provider Type Profile** is a declarative YAML file that bundles everything OpenShell needs to know about a single external service (GitHub, Claude, Slack, Vertex AI, etc.): + +| Section | Purpose | +|---------|---------| +| `credentials` | Expected secrets, env vars, injection style (bearer, header, query, path) | +| `endpoints` | Host/port allowlist using the existing network policy language | +| `deny` | Deny rules the user cannot override (e.g., block branch-protection endpoints) | +| `binaries` | Allowed executables for this provider | +| `verify` | Optional probe endpoint for creation-time connectivity checks | +| `inference` | Base URL, protocol, default headers (inference-capable providers only) | +| `refresh` | Credential refresh strategy (static, oauth2, external) | + +### Key design decisions + +- **Policy composition:** Three layers composed JIT — base sandbox policy, auto-generated provider policy, user-authored policy. Deny wins over allow; provider deny rules cannot be bypassed. +- **Attach/detach lifecycle:** Providers can be added/removed from running sandboxes because credential injection is proxy-side, not env-var-at-start. +- **Custom registry:** Users can `export`, fork, and `import` their own profiles, enabling custom provider types without upstream changes. +- **Credential scoping:** Credentials bound to `(credential, endpoint, binary)` triples — a GitHub PAT can only reach `api.github.com`, not be exfiltrated to an arbitrary endpoint. + +### Implementation status (as of June 2026) + +**Shipped:** Profile schema, built-in profiles, custom registry (import/export/lint), JIT policy composition, attach/detach lifecycle, `providers_v2_enabled` feature gate, docs. + +**Remaining:** Profile-driven credential injection (replacing placeholder env scanning), credential scoping enforcement, credential verification, credential refresh lifecycle, inference automation (`inference.local` routing), HA hardening. + +### Open sub-issues (blockers for full vision) + +- [#894](https://github.com/NVIDIA/OpenShell/issues/894) — Placeholder model breaks SDKs that validate token format before any network call (Slack `xoxb-` prefix check) +- [#913](https://github.com/NVIDIA/OpenShell/issues/913) — WSS payloads bypass L7 rewriting (Discord gateway opcode 4004) + +Both motivate the move from placeholder env vars to proxy-side credential injection declared in provider profiles. + +### Related active work + +- [#1306](https://github.com/NVIDIA/OpenShell/issues/1306) — Gateway-owned credential refresh for short-lived tokens +- [#1622](https://github.com/NVIDIA/OpenShell/pull/1622) — Path-based `auth_style` for provider profiles +- [#1638](https://github.com/NVIDIA/OpenShell/pull/1638) — AWS SigV4 credential signing at the proxy +- [#1681](https://github.com/NVIDIA/OpenShell/pull/1681) — Okta OBO token exchange +- [#1736](https://github.com/NVIDIA/OpenShell/issues/1736) — Dynamic identity sources (SPIFFE/K8s) for OAuth2TokenExchange + +--- + +## 2. Kaiden — Projects (Workspace Configuration Profiles) + +**Source:** [openkaiden/kaiden#1272](https://github.com/openkaiden/kaiden/issues/1272) ("Projects management"), [Milestone 0.3 — Mycelium](https://github.com/openkaiden/kaiden/milestone/4) + +### What it is + +A **Project** is a reusable configuration template that defines everything needed to spin up an agent workspace. It sits between "raw resources" (models, MCP servers, skills, knowledge bases) and "running sandboxed workspaces." + +```typescript +interface WorkspaceProjectInfo { + id: string; + name: string; + folder: string; // local directory or git URL + skills: string[]; // enabled skill names + mcpServers: string[]; // enabled MCP server names + knowledges: string[]; // enabled RAG knowledge base names + secrets: string[]; // enabled secret names + filesystem: FilesystemConfiguration; // mode + mount points + network: NetworkConfiguration; // mode + allowed hosts +} +``` + +### Key design decisions + +- **Local-directory-first:** A project is anchored to a folder on the host machine (or a Git URL). The folder is the source code the agent will work in. +- **Resource selection, not definition:** Projects reference skills, MCP servers, knowledges, and secrets by name — they don't define them. The resources exist independently; projects toggle which ones are active. +- **Template → instance separation:** Projects are reusable templates; workspaces are ephemeral running instances created from projects. +- **UI-driven creation:** Two-step wizard (source → review) with per-resource checklists for skills, MCP servers, etc. +- **Stored as JSON** on disk under a `workspace-projects` directory. + +### Implementation status (as of June 2026, milestone due 2026-06-17) + +| Sub-issue | Status | What | +|-----------|--------|------| +| [#1925](https://github.com/openkaiden/kaiden/issues/1925) | Merged | Backend CRUD, JSON storage, reference validation | +| [#1926](https://github.com/openkaiden/kaiden/issues/1926) | Merged | Projects list page (searchable table) | +| [#1927](https://github.com/openkaiden/kaiden/issues/1927) | Merged | Project details page (Overview + Settings tabs) | +| [#1928](https://github.com/openkaiden/kaiden/issues/1928) | PR open | Create wizard (local folder or Git URL) | +| [#1929](https://github.com/openkaiden/kaiden/issues/1929) | Open | Wire MCP servers to create wizard | +| [#1930](https://github.com/openkaiden/kaiden/issues/1930) | PR open | Wire skills to create wizard | +| [#2066](https://github.com/openkaiden/kaiden/issues/2066) | Open | Feed project config into workspace creation | + +### What Kaiden's "project" is NOT + +- It is not about provider type definitions — Kaiden delegates that entirely to OpenShell +- It does not define credential injection, endpoint allowlists, or deny rules +- It does not manage credential refresh or verification +- It is a higher-level orchestration concern: "which combination of pre-existing resources does this workspace need?" + +--- + +## 3. harness-openshell — Profiles + Custom Providers + +### What it is + +**Profiles** in harness-openshell are TOML files (`profiles/*.toml`) that define per-sandbox configuration: + +```toml +image = "ghcr.io/org/agent-image:latest" +command = "claude --bare" +name = "agent" +keep = true +providers = ["github", "vertex-local", "atlassian"] + +[env] +SOME_VAR = "value" +``` + +**Custom providers** are a workaround layer for integrations OpenShell doesn't natively support yet. Defined in `providers.toml` with type `"custom"` (vs. `"openshell"` for gateway-managed providers). Today only `gws` (Google Workspace) is custom — its credentials are decrypted locally and uploaded directly into the sandbox, bypassing the gateway's credential store. + +### How the pieces fit + +``` +providers.toml profiles/*.toml sandbox/profiles/*.yaml +───────────── ────────────── ────────────────────── +Catalog of all known Per-sandbox config: OpenShell-native provider +providers + their image, command, env, type profiles (YAML) that +prerequisites (inputs) which providers to get imported into the + attach gateway via `openshell + provider profile import` +``` + +1. `harness providers` registers providers with the gateway (using `providers.toml` inputs) +2. `harness new --profile NAME` reads `profiles/NAME.toml`, validates its `providers` list against the gateway, creates the sandbox +3. Custom providers (like `gws`) bypass the gateway entirely — files are uploaded directly + +### Current limitations that motivated this analysis + +- Only one profile exists today: `default.toml` +- Custom providers are workarounds that should shrink as OpenShell's provider v2 matures +- The `sandbox/profiles/*.yaml` directory (OpenShell provider type profiles) only has `atlassian.yaml` — as more custom providers get native support upstream, more would move here +- Profile config doesn't express the resource-selection semantics Kaiden projects add (skills, MCP servers, knowledge bases) + +--- + +## Comparison Matrix + +| Dimension | OpenShell Provider Profile | Kaiden Project | harness-openshell Profile | +|-----------|---------------------------|----------------|---------------------------| +| **Unit of** | A service/integration type | A workspace template | A sandbox configuration | +| **Defines** | Credentials, endpoints, deny rules, binaries, inference, refresh | Which skills/MCP/secrets/knowledge are active, source folder, filesystem/network mode | Image, command, env vars, which providers to attach | +| **Scope** | Single provider (GitHub, Claude, etc.) | Entire workspace (many providers, tools, data sources) | Single sandbox (one image, one command, N providers) | +| **Stored as** | YAML in gateway registry | JSON on disk | TOML on disk | +| **Lifecycle** | Registered once, attached/detached per sandbox | Created once, instantiated as workspaces | Read at sandbox creation time | +| **Credential handling** | Proxy-side injection, refresh, scoping | References secrets by name (delegates to OpenShell) | Delegates to gateway (openshell providers) or uploads directly (custom providers) | +| **Network policy** | Auto-generated from endpoint declarations + deny rules | Mode + hosts array (delegates enforcement to OpenShell) | Not expressed — relies on gateway defaults + manual policy | +| **Custom extensibility** | export/fork/import registry | Add resources to the project's resource lists | Add TOML files to `profiles/` or YAML to `sandbox/profiles/` | + +--- + +## Key Observations + +### 1. Three layers of the same stack + +These three concepts operate at different layers of the same stack: + +``` +┌─────────────────────────────────────────────┐ +│ Kaiden Project │ "What combination of resources +│ (workspace template) │ does this workspace need?" +├─────────────────────────────────────────────┤ +│ harness-openshell Profile │ "What image, command, env, and +│ (sandbox configuration) │ providers does this sandbox use?" +├─────────────────────────────────────────────┤ +│ OpenShell Provider Type Profile │ "What does this single service +│ (service integration definition) │ need to work inside a sandbox?" +└─────────────────────────────────────────────┘ +``` + +They are complementary, not competing. Kaiden selects which providers a workspace gets; harness-openshell attaches those providers to a sandbox; OpenShell's provider profiles define what each provider actually means. + +### 2. Kaiden's "project" adds the resource-selection layer harness-openshell doesn't have + +harness-openshell profiles today are focused narrowly: image + command + env + provider list. Kaiden adds: +- **Skills selection** (enable/disable per workspace) +- **MCP server selection** (enable/disable per workspace) +- **Knowledge base selection** (RAG sources per workspace) +- **Secrets selection** (which secrets are accessible) +- **Filesystem configuration** (mount mode + mount points) +- **Network configuration** (mode + host allowlist) + +This is relevant if harness-openshell ever needs to support multi-agent or multi-tool configurations where different sandboxes need different subsets of available resources. + +### 3. harness-openshell's custom providers are a bridge + +The `providers.toml` custom provider mechanism (type: `"custom"`) exists specifically to fill gaps in OpenShell's provider v2 system. As #896's remaining work lands (especially profile-driven credential injection and credential refresh), custom providers like `gws` should migrate to native OpenShell provider type profiles. The `upstream` field in `providers.toml` already tracks which upstream issue would obsolete each workaround. + +### 4. OpenShell's credential model is evolving toward what harness-openshell needs + +The two open sub-issues under #896 (#894 and #913) are exactly the problems that make custom providers necessary in harness-openshell — SDKs that can't work with placeholder tokens need real credentials, which means bypassing the gateway. Once proxy-side injection and credential refresh land upstream, the custom provider workaround layer can shrink. + +### 5. Kaiden's worktree/local-dir focus is orthogonal to harness-openshell's image-first approach + +Kaiden projects start from a local folder (the code you want the agent to work on). harness-openshell profiles start from a container image (the pre-built agent environment). These are different entry points to the same result (a running sandbox), but they reflect different usage patterns: +- **Kaiden:** "I have source code, give me a configured agent to work on it" +- **harness-openshell:** "I have a pre-built agent image, configure the sandbox it runs in" + +--- + +## Implications for harness-openshell + +### Short term (now) +- Continue using custom providers as the bridge for services OpenShell doesn't natively support +- Track OpenShell #896 progress — each shipped sub-feature is a chance to remove a custom workaround +- Keep profiles simple (TOML, image + command + env + providers) — don't add Kaiden-style resource selection unless needed + +### Medium term (as OpenShell provider v2 matures) +- Migrate custom providers to native OpenShell provider type profiles (`sandbox/profiles/*.yaml`) +- Adopt profile-driven credential injection when it ships, eliminating direct credential upload +- Consider whether `providers.toml` should shrink to just a preflight validation catalog (no longer distinguishing custom vs. openshell types) + +### Long term (if scope expands) +- If harness-openshell needs multi-tool or multi-agent workspace configuration, Kaiden's resource-selection model (skills, MCP servers, knowledge bases per profile) is a reasonable pattern to follow +- The three-layer stack (provider type → sandbox config → workspace template) is a natural factoring if the project grows beyond single-sandbox use cases diff --git a/docs/proto-migration.md b/docs/proto-migration.md new file mode 100644 index 0000000..404ff51 --- /dev/null +++ b/docs/proto-migration.md @@ -0,0 +1,231 @@ +# Proto-Based Profile Format Migration + +Migrate harness-openshell's profile and provider profile formats from +hand-rolled TOML/YAML to OpenShell's proto-generated Go types. The proto +becomes the schema; the serialization format (textproto, JSON, or TOML +with a mapping layer) is a separate decision. + +## Why + +### Upstream alignment + +OpenShell's `SandboxSpec`, `ProviderProfile`, and related messages are the +canonical types that the gateway operates on. Every field harness-openshell +cares about already exists in the proto: + +| harness-openshell concept | Proto message | +|---------------------------|---------------| +| Sandbox profile (`profiles/*.toml`) | `CreateSandboxRequest` → `SandboxSpec` → `SandboxTemplate` | +| Provider profile (`sandbox/profiles/*.yaml`) | `ProviderProfile` | +| Provider credentials | `ProviderProfileCredential` | +| Network endpoints | `NetworkEndpoint` (from `sandbox.proto`) | +| Provider catalog (`providers.toml`) | Partially: `ProviderProfileDiscovery` + `ProviderProfileCredential.env_vars` | + +When OpenShell adds fields (e.g., `user_namespaces`, `volume_claim_templates`, +credential refresh material), re-running `protoc` makes them available +immediately. No manual struct updates, no drift. + +### Compile-time safety + +Today, profile fields are parsed from TOML into a hand-written `Config` +struct. If OpenShell renames or restructures a field, the harness discovers +this at runtime when CLI flags fail. With proto-generated types, a field +change is a compile error. + +### Format for the future + +The proto types are the same ones a gRPC client would use. If/when +harness-openshell moves from CLI exec to direct gRPC, the profile structs +are already the request payloads — no translation step. + +## What changes + +### Sandbox profiles + +**Today:** `profiles/default.toml` → parsed into `profile.Config` struct → +translated to `openshell sandbox create --name X --image Y --provider Z` flags. + +**After:** `profiles/default.textproto` (or `.json`) → unmarshalled into +generated `openshellv1.CreateSandboxRequest` + a `HarnessConfig` wrapper → +same CLI flags, but sourced from the proto struct. + +Harness-only fields (`command`, `keep`) have no proto equivalent. These +live in a wrapper: + +```go +type Profile struct { + Request *openshellv1.CreateSandboxRequest + Command string + Keep bool +} +``` + +### Provider profiles + +**Today:** `sandbox/profiles/atlassian.yaml` → hand-written YAML matching +OpenShell's profile schema → imported via `openshell provider profile import`. + +**After:** `sandbox/profiles/atlassian.textproto` → unmarshalled into +generated `openshellv1.ProviderProfile` → serialized to YAML or JSON for +`openshell provider profile import`, or passed directly if/when using gRPC. + +### Provider catalog (providers.toml) + +`providers.toml` tracks preflight validation inputs and custom provider +workarounds. The proto doesn't cover preflight checks (`kind = "file"`, +`kind = "check"`) or the `upstream` tracking field. This file stays as-is — +it's harness-specific orchestration, not an upstream type. + +Over time, `providers.toml` shrinks as: +- OpenShell's `ProviderProfileDiscovery` handles more credential discovery +- Credential verification on create ships upstream (#896 roadmap) +- Custom providers migrate to native OpenShell provider profiles + +### Bash path + +The bash path (`profile.sh` + `parse-profile.py`) currently reads TOML. +Options when the format changes: + +| Option | Tradeoff | +|--------|----------| +| **Go shim** (`harness profile dump NAME` → shell vars) | One parser, two consumers. Bash calls Go. Cleanest. | +| **Swap Python parser** (protobuf or json module) | Keeps bash self-contained but adds protobuf pip dependency. | +| **Drop bash path** | Simplest, but premature if Go migration isn't complete. | + +**Recommendation:** Go shim. Add a `harness profile dump` subcommand that +reads the proto-format profile and emits `SANDBOX_NAME=...` shell variable +assignments. `parse-profile.py` is replaced by a one-line call to the Go +binary. No new dependencies in the bash path, no duplicated parsing logic. + +## Tradeoffs + +### Serialization format + +| Format | Pros | Cons | +|--------|------|------| +| **Textproto** | Comments. Snake_case field names match proto exactly. `prototext.Unmarshal` in Go. | Map entries are verbose (`key:` / `value:` pairs). Less familiar to most users. | +| **Proto JSON** | Universal. `protojson.Unmarshal` in Go. Easy to generate/consume from other tools. | No comments. camelCase field names (protobuf JSON convention). | +| **TOML (keep, marshal internally)** | Best authoring UX. Users don't need to learn a new format. | Manual mapping layer between TOML keys and proto fields. Defeats some of the alignment benefit. | + +**Recommendation:** Textproto for provider profiles (they read cleanly, +comments are valuable for documenting credential vs. config distinctions). +For sandbox profiles, textproto works but the `environment` map is ugly — +proto JSON may be more readable there. Support both via file extension +detection (`.textproto` → `prototext`, `.json` → `protojson`). + +### Proto stability + +OpenShell is alpha (v0.0.55). The proto could change. However: + +- `SandboxSpec` core fields (`image`, `environment`, `providers`) are stable + — they map directly to the CLI's `sandbox create` flags which have been + stable since v0.0.20+. +- `ProviderProfile` shipped with providers v2 and has a published schema in + the docs. Breaking changes would affect all provider profile YAML users, + not just harness-openshell. +- Proto changes produce compile errors, which is strictly better than + discovering CLI output format changes at runtime. +- If a field is removed or renamed, the fix is: re-generate, update the one + or two references in harness code, done. + +### Complexity budget + +Adding `protoc` to the build introduces: +- Proto files vendored (4 files: `openshell.proto`, `datamodel.proto`, + `sandbox.proto`, `inference.proto`) +- `protoc-gen-go` and `protoc-gen-go-grpc` as build-time dependencies +- A `make proto` (or `buf generate`) target +- Generated `.pb.go` files checked in (or generated in CI) + +This is standard Go infrastructure but it's not zero. The project currently +has no protobuf dependency. + +## Implementation plan + +### Phase 1: Vendor proto, generate types + +1. Create `proto/` directory, vendor the 4 proto files from NVIDIA/OpenShell +2. Add `buf.yaml` + `buf.gen.yaml` (or a `make proto` target with `protoc`) +3. Generate Go types into `internal/openshell/` (or `pkg/openshell/`) +4. Add generated files to the repo (avoid requiring protoc for contributors) +5. Verify: generated `CreateSandboxRequest`, `SandboxSpec`, `ProviderProfile` + types compile and match the upstream schema + +### Phase 2: Migrate provider profiles + +Start here because the provider profile format is the cleanest mapping — +almost 1:1 with the existing `atlassian.yaml`. + +1. Convert `sandbox/profiles/atlassian.yaml` → `sandbox/profiles/atlassian.textproto` +2. Update `cmd/providers.go` to unmarshal via `prototext.Unmarshal` into + `*openshellv1.ProviderProfile` +3. For `openshell provider profile import`: serialize the proto struct to + YAML (or JSON) since the CLI expects that format, or write a temp file +4. Update tests +5. Verify: `harness providers` still registers the atlassian profile correctly + +### Phase 3: Migrate sandbox profiles + +1. Define the `Profile` wrapper struct (proto `CreateSandboxRequest` + + harness-only `Command` and `Keep` fields) +2. Convert `profiles/default.toml` → `profiles/default.textproto` (or `.json`) + with a sidecar section or separate file for harness-only fields +3. Update `internal/profile/` to parse the new format into the wrapper struct +4. Update `internal/profile/` to translate the proto struct into CLI flags + (same logic as today, different source struct) +5. Add `harness profile dump NAME` subcommand for bash path compatibility +6. Replace `parse-profile.py` call in `profile.sh` with `harness profile dump` +7. Update tests (Go unit tests + bats integration) +8. Verify: `make validate` passes on all matrix combinations + +### Phase 4: Cleanup + +1. Remove `parse-profile.py` +2. Remove old `profile.Config` struct (replaced by wrapper + proto types) +3. Update docs and `profile-concepts.md` to reflect the new format +4. Document the format in a comment block or README section + +## Decision: harness-only fields + +Two fields exist in harness profiles that have no proto equivalent: + +| Field | Why it's harness-only | Where it goes | +|-------|----------------------|---------------| +| `command` | Passed at `sandbox connect` / `sandbox exec` time, not at creation | Wrapper struct | +| `keep` | Client-side lifecycle decision (keep sandbox after command exits) | Wrapper struct | + +Options for encoding these alongside the proto: + +1. **Wrapper struct with separate parsing** — proto fields in `.textproto`, + harness fields as Go defaults or a small sidecar `harness.toml`. +2. **Proto extensions** — technically possible but overkill and non-standard. +3. **Comments-as-config** — parse specially formatted comments in the + textproto. Fragile, don't do this. +4. **Custom proto message** — define a `HarnessProfile` message that embeds + `CreateSandboxRequest`. Cleanest if we're already running protoc. + +**Recommendation:** Option 4. Define a small `harness.proto`: + +```protobuf +syntax = "proto3"; +package harness.v1; +import "openshell.proto"; + +message Profile { + openshell.v1.CreateSandboxRequest request = 1; + string command = 2; // default: "claude --bare" + bool keep = 3; // default: true + string description = 4; // human-readable profile description +} +``` + +This gives you one file, one parse call, full type safety, and a natural +place for any future harness-only fields. + +## References + +- [OpenShell proto](https://github.com/NVIDIA/OpenShell/blob/main/proto/openshell.proto) +- [OpenShell providers v2 docs](https://github.com/NVIDIA/OpenShell/blob/main/docs/sandboxes/providers-v2.mdx) +- [OpenShell #896 — Enhanced Provider Management](https://github.com/NVIDIA/OpenShell/issues/896) +- [Kaiden #1272 — Projects management](https://github.com/openkaiden/kaiden/issues/1272) +- [profile-concepts.md](profile-concepts.md) — cross-project profile concept analysis diff --git a/release_plan.md b/docs/release-plan.md similarity index 100% rename from release_plan.md rename to docs/release-plan.md diff --git a/internal/gateway/cli_test.go b/internal/gateway/cli_test.go index f171e67..2505eb6 100644 --- a/internal/gateway/cli_test.go +++ b/internal/gateway/cli_test.go @@ -101,8 +101,9 @@ exit 1 } func TestSandboxCreate_ArgsMinimal(t *testing.T) { + argsFile := filepath.Join(t.TempDir(), "args") bin := writeStub(t, `#!/bin/bash -echo "$@" > /tmp/test-create-args +echo "$@" > `+argsFile+` `) gw := New(bin) gw.SandboxCreate(SandboxCreateOpts{ @@ -110,7 +111,7 @@ echo "$@" > /tmp/test-create-args TTY: false, Keep: true, }) - data, _ := os.ReadFile("/tmp/test-create-args") + data, _ := os.ReadFile(argsFile) args := strings.TrimSpace(string(data)) if !strings.Contains(args, "--name test") { t.Errorf("missing --name: %s", args) @@ -124,7 +125,6 @@ echo "$@" > /tmp/test-create-args if strings.Contains(args, "--from") { t.Errorf("should not have --from: %s", args) } - os.Remove("/tmp/test-create-args") } func TestSandboxCreate_ArgsFull(t *testing.T) { diff --git a/internal/preflight/preflight.go b/internal/preflight/preflight.go index 5b281e5..996e325 100644 --- a/internal/preflight/preflight.go +++ b/internal/preflight/preflight.go @@ -26,9 +26,10 @@ type Provider struct { } type Input struct { - Key string `toml:"key"` - Kind string `toml:"kind"` - Secret bool `toml:"secret"` + Key string `toml:"key"` + Kind string `toml:"kind"` + Secret bool `toml:"secret"` + Sandbox bool `toml:"sandbox"` } type ProvidersFile struct { @@ -39,7 +40,6 @@ type ConfigFile struct { Providers []string `toml:"providers"` ProvidersCustom []string `toml:"providers-custom"` Upstream UpstreamConfig `toml:"upstream"` - ChartVersion string `toml:"-"` } type UpstreamConfig struct { @@ -62,7 +62,6 @@ func LoadConfig(path string) (*ConfigFile, error) { if _, err := toml.DecodeFile(path, &cf); err != nil { return nil, fmt.Errorf("reading %s: %w", path, err) } - cf.ChartVersion = cf.Upstream.ChartVersion return &cf, nil } @@ -86,6 +85,31 @@ func EnabledProviders(all []Provider, config *ConfigFile) []Provider { return result } +// ProviderEnvVars returns environment variables marked with sandbox = true +// for the named providers, resolved from the local environment. Only +// includes env vars that are set. +func ProviderEnvVars(providers []Provider, names []string) map[string]string { + wanted := make(map[string]bool, len(names)) + for _, n := range names { + wanted[n] = true + } + env := make(map[string]string) + for _, p := range providers { + if !wanted[p.Name] { + continue + } + for _, inp := range p.Inputs { + if inp.Kind != "env" || !inp.Sandbox { + continue + } + if val := os.Getenv(inp.Key); val != "" { + env[inp.Key] = val + } + } + } + return env +} + func CheckInput(inp Input) (bool, string) { switch inp.Kind { case "env": @@ -101,25 +125,23 @@ func CheckInput(inp Input) (bool, string) { case "file": path := expandPath(inp.Key) - if _, err := os.Stat(path); err == nil { - meta := FileMetadata(path) - if meta != nil && inp.Secret { - safe := pickKeys(meta, "project", "type") - masked := pickKeysExcept(meta, "project", "type") - parts := formatMeta(safe) - parts = append(parts, formatMeta(masked)...) - if len(parts) > 0 { - return true, fmt.Sprintf("✓ local file: %s (%s)", inp.Key, strings.Join(parts, ", ")) - } - } else if meta != nil { - parts := formatMeta(meta) - if len(parts) > 0 { - return true, fmt.Sprintf("✓ local file: %s (%s)", inp.Key, strings.Join(parts, ", ")) - } - } + if _, err := os.Stat(path); err != nil { + return false, fmt.Sprintf("✗ local file: %s not found", inp.Key) + } + meta := FileMetadata(path) + if meta == nil { return true, fmt.Sprintf("✓ local file: %s", inp.Key) } - return false, fmt.Sprintf("✗ local file: %s not found", inp.Key) + var parts []string + for k, v := range meta { + if v != "" { + parts = append(parts, fmt.Sprintf("%s=%s", k, v)) + } + } + if len(parts) > 0 { + return true, fmt.Sprintf("✓ local file: %s (%s)", inp.Key, strings.Join(parts, ", ")) + } + return true, fmt.Sprintf("✓ local file: %s", inp.Key) case "check": expanded := os.ExpandEnv(inp.Key) @@ -195,40 +217,6 @@ func runQuiet(command string) bool { return exec.CommandContext(ctx, "bash", "-c", command).Run() == nil } -func pickKeys(m map[string]string, keys ...string) map[string]string { - result := make(map[string]string) - for _, k := range keys { - if v, ok := m[k]; ok && v != "" { - result[k] = v - } - } - return result -} - -func pickKeysExcept(m map[string]string, except ...string) map[string]string { - skip := make(map[string]bool) - for _, k := range except { - skip[k] = true - } - result := make(map[string]string) - for k, v := range m { - if !skip[k] && v != "" { - result[k] = v - } - } - return result -} - -func formatMeta(m map[string]string) []string { - var parts []string - for k, v := range m { - if v != "" { - parts = append(parts, fmt.Sprintf("%s=%s", k, v)) - } - } - return parts -} - func loadEnabledProviders(harnessDir string) ([]Provider, error) { providersPath := os.Getenv("PROVIDERS_TOML") if providersPath == "" { diff --git a/internal/preflight/preflight_test.go b/internal/preflight/preflight_test.go index adadb67..3c3c907 100644 --- a/internal/preflight/preflight_test.go +++ b/internal/preflight/preflight_test.go @@ -90,8 +90,8 @@ chart-version = "0.0.55" if len(cfg.ProvidersCustom) != 1 || cfg.ProvidersCustom[0] != "gws" { t.Errorf("providers-custom = %v", cfg.ProvidersCustom) } - if cfg.ChartVersion != "0.0.55" { - t.Errorf("ChartVersion = %q", cfg.ChartVersion) + if cfg.Upstream.ChartVersion != "0.0.55" { + t.Errorf("Upstream.ChartVersion = %q", cfg.Upstream.ChartVersion) } } diff --git a/profiles/default.toml b/profiles/default.toml index 1e09014..a4a7f3d 100644 --- a/profiles/default.toml +++ b/profiles/default.toml @@ -1,8 +1,8 @@ # Default agent configuration. # # Usage: -# ./sandbox-ocp.sh # uses agents/default.toml -# ./sandbox-ocp.sh research # uses agents/research.toml +# harness create # uses profiles/default.toml +# harness create --profile research # uses profiles/research.toml name = "agent" image = "quay.io/rcochran/openshell:sandbox" @@ -17,5 +17,5 @@ ANTHROPIC_API_KEY = "sk-ant-openshell-proxy-managed" CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS = "1" GOOGLE_WORKSPACE_CLI_CONFIG_DIR = "/sandbox/.config/openshell" GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE = "/sandbox/.config/openshell/credentials.json" -JIRA_URL = "https://redhat.atlassian.net" -JIRA_USERNAME = "rcochran@redhat.com" +# JIRA_URL and JIRA_USERNAME flow automatically from the local environment +# when the atlassian provider is enabled (see providers.toml). diff --git a/providers.toml b/providers.toml index 41b71de..2188ed3 100644 --- a/providers.toml +++ b/providers.toml @@ -29,8 +29,8 @@ type = "openshell" description = "Jira and Confluence (read-only, Basic auth resolved by proxy)" inputs = [ { key = "JIRA_API_TOKEN", kind = "env", secret = true }, - { key = "JIRA_URL", kind = "env" }, - { key = "JIRA_USERNAME", kind = "env" }, + { key = "JIRA_URL", kind = "env", sandbox = true }, + { key = "JIRA_USERNAME", kind = "env", sandbox = true }, { key = "curl -sf ${JIRA_URL}/rest/api/2/serverInfo -o /dev/null", kind = "check" }, { key = "curl -sf -u ${JIRA_USERNAME}:${JIRA_API_TOKEN} ${JIRA_URL}/rest/api/2/myself -o /dev/null", kind = "check" }, ] diff --git a/sandbox/launcher/main.go b/sandbox/launcher/main.go index 96bb997..a0f9479 100644 --- a/sandbox/launcher/main.go +++ b/sandbox/launcher/main.go @@ -59,12 +59,14 @@ func configureGateway(endpoint, mtlsDir, cli string) error { cmd := exec.Command(cli, "gateway", "add", httpEndpoint, "--name", "openshell") cmd.Stdout = os.Stdout cmd.Stderr = os.Stdout - cmd.Run() + if err := cmd.Run(); err != nil { + return fmt.Errorf("gateway add: %w", err) + } home := os.Getenv("HOME") gwDir := filepath.Join(home, ".config", "openshell", "gateways", "openshell") mtlsDest := filepath.Join(gwDir, "mtls") - if err := os.MkdirAll(mtlsDest, 0o755); err != nil { + if err := os.MkdirAll(mtlsDest, 0o700); err != nil { return fmt.Errorf("creating mtls dir: %w", err) } @@ -73,7 +75,9 @@ func configureGateway(endpoint, mtlsDir, cli string) error { metaPath := filepath.Join(gwDir, "metadata.json") var meta map[string]any if data, err := os.ReadFile(metaPath); err == nil { - json.Unmarshal(data, &meta) + if err := json.Unmarshal(data, &meta); err != nil { + return fmt.Errorf("parsing metadata.json: %w", err) + } } if meta == nil { meta = make(map[string]any) @@ -217,13 +221,15 @@ func copyFile(src, dst string) error { return err } defer in.Close() - out, err := os.Create(dst) + out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) if err != nil { return err } - defer out.Close() - _, err = io.Copy(out, in) - return err + if _, err = io.Copy(out, in); err != nil { + out.Close() + return err + } + return out.Close() } func fileExists(path string) bool { diff --git a/test/test-flow.sh b/test/test-flow.sh index e3d5948..042fc3c 100755 --- a/test/test-flow.sh +++ b/test/test-flow.sh @@ -147,8 +147,8 @@ test_errors() { step "teardown (first)" "$HARNESS" teardown --sandboxes --providers step "teardown (second)" "$HARNESS" teardown --sandboxes --providers else - step "teardown (first)" "$HARNESS" teardown - step "teardown (second)" "$HARNESS" teardown + step "teardown (first)" "$HARNESS" teardown --sandboxes --providers --k8s + step "teardown (second)" "$HARNESS" teardown --sandboxes --providers --k8s fi echo "" diff --git a/todo-improve.md b/todo-improve.md new file mode 100644 index 0000000..97aa472 --- /dev/null +++ b/todo-improve.md @@ -0,0 +1,97 @@ +# harness-openshell improvement backlog + +Generated from 12-expert review (2 workflows, 6 agents each). +Triaged against design.md rearchitect plan. +Effort: S (<1hr), M (1-4hr), L (4hr+). + +--- + +## Bugs + +- [x] **quiet-mode kubectl never retries on transient errors** [S] +- [x] **non-seekable stdin retries replay empty input** [S] +- [x] **gateway timeout returns exit 0** [S] +- [x] **buggy contains()/indexOf() test helpers** [S] +- [x] **kubectl error message uses opts.Args not actual executed args** [S] +- [x] **runQuiet goroutine leak on timeout** [S] + +## Done — safety and quality + +- [x] **teardown no-args defaults to maximum destruction** [S] +- [x] **--local/--remote not mutually exclusive on new** [S] +- [x] **deployRemote prints no guidance on partial failure** [S] +- [x] **ProviderList/SandboxList dedup** — `parseFirstColumn` helper [S] +- [x] **RunHelm returns (string, error) but string is always empty** [S] +- [x] **runQuiet should use context.WithTimeout** [S] +- [x] **Merge preflight.go + check.go** [S] +- [x] **Merge parse.go into profile.go** [S] +- [x] **shared test mock in wrong file** → `cmd/helpers_test.go` [S] +- [x] **detectHarnessDir silently falls back to "."** [S] +- [x] **detectHarnessDir loop duplicated** [S] +- [x] **Makefile lacks lint/vet targets** [S] + +## Done — dead code + +- [x] `SandboxUpload` + `SandboxExec` — zero callers +- [x] `status.Warnf` — zero callers +- [x] `decodeBase64` — trivial wrapper, inlined +- [x] `cmd.Stderr = nil` in `runOutput` — no-op +- [x] `internal/util` package — inlined at call site +- [x] Four Gateway sub-interfaces — zero consumers + +--- + +## Do now — quick independent fixes + +These are safe to do before the rearchitect. Each is independent and +won't conflict with design.md changes. + +- [x] **error wrapping inconsistency** — audited, all already use `%w` [S] +- [x] **env-var-with-fallback** — added `envOr()` helper, applied to 4 cases [S] +- [x] **launcher binary swallows errors** — check cmd.Run(), json.Unmarshal, copyFile close, mTLS 0700 [S] +- [ ] **duplicated Config struct in launcher** — separate go.mod, can't import internal/. Deferred. [S] +- [x] **test writes to hardcoded /tmp path** — use `t.TempDir()` [S] +- [x] **secret name string literals** — shared `secretNames` slice [S] +- [x] **hardcoded sk-ant- placeholder and personal email** — removed PII from default.toml [S] +- [x] **CheckInput file case** — flattened with early returns [S] +- [x] **deployLocal unnecessary podmanPath** — inlined [S] +- [x] **force-deleting pods with --grace-period=0** — changed to `--grace-period=30` [S] +- [x] **ConfigFile.ChartVersion alias** — removed, use `cfg.Upstream.ChartVersion` [S] +- [x] **pickKeys/pickKeysExcept/formatMeta** — deleted, inlined at call site [S] +- [x] **extractYAMLID** — simplified with `strings.CutPrefix` [S] +- [ ] **unreachable return nil** — `cmd/new.go` (already annotated with comment) [S] +- [ ] **swallowed errors in deploy.go and new.go** — remaining discarded errors [S] +- [ ] **sandbox CRD from unversioned latest URL** — pin version [S] +- [x] **SCC grant/revoke lists** — shared `sccPrivilegedSAs` slice [S] +- [ ] **launcher image single-arch** — use `docker buildx` for launcher. `Makefile` [S] + +## Config improvements + +- [ ] **Provider env vars (JIRA_URL, JIRA_USERNAME) should come from environment** — these are validated as `kind = "env"` inputs in `providers.toml` but hardcoded in `profiles/default.toml`. Profile should use `$JIRA_URL` / `$JIRA_USERNAME` references expanded at build time via `os.ExpandEnv`, so changing a Jira URL doesn't require editing the profile. Needs `BuildSandboxEnv` to expand env var references in values. [S] +- [ ] **Provider-specific env vars should flow from provider config, not profile** — longer-term, JIRA_URL and JIRA_USERNAME shouldn't be in the profile at all. Provider registration should carry config values that get injected into the sandbox automatically. See design.md "Config files" section. [M] + +## Defer to rearchitect (design.md) + +These will be restructured or replaced during the command/code reorg. + +- [ ] **Move orchestration out of cmd/** → the rearchitect IS this [L] +- [ ] **Table-driven provider registration** → redesigned with `providers register` [M] +- [ ] **Preflight subcommands** → redesigned in new command structure [S] +- [ ] **Cobra examples** → add after commands are renamed [S] +- [ ] **Hardcoded values → harness.toml config** → addressed by config redesign [M] +- [ ] **Context propagation + cancellation** → natural to add when splitting new→create/up [L] +- [ ] **Gateway CLI timeout** → comes with context propagation [M] +- [ ] **kubectl log tailing bypasses k8s.Runner** → restructured code [S] +- [ ] **Configmap creation dedup** → restructured code [S] +- [ ] **No test coverage for registerProviders/RunCheck** → rewritten functions [M] +- [ ] **ProviderGet → ProviderExists rename** → interface redesign [S] +- [ ] **CLIVersion removal** → interface cleanup [S] +- [ ] **Inline Job spec → YAML template** → move to deploy/ [M] +- [ ] **Gateway CLI output parsing** → depends on openshell `--output json` [M] +- [ ] **GWS credential export flow duplicated** → restructured code [M] +- [ ] **Provider registration order** → `inference_provider` config field [S] + +## Critical — address during rearchitect + +- [ ] **cluster-admin ClusterRoleBinding + privileged SCC on default SA** — scoped ClusterRole in `deploy/`, use `kubectl apply`, remove `default` from SCC grants [M] +- [ ] **credentials visible in ps aux and verbose logs** — pass via subprocess env vars or stdin, add credential redaction to `status.Cmd` [M]