Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 2
PR4a: provider + inference reconciliation wired into apply#101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
f862148
feat(openshell): widen firewall Provider with Config/Labels (PR4a S1)
robbycochran d2d3603
feat(openshell): credential-preserving provider write + live gate (PR…
robbycochran 343d072
feat(plan,config): provider diff rule with ownership + config validat…
robbycochran 276a781
S4: provider reconcile engine (SDK-free)
robbycochran f2d0d18
feat(cmd): apply SDK seam + inference reconcile swap (PR4a S5)
robbycochran 12ad022
PR4a S6: wire provider reconcile into apply + providerCreatePlan boot…
robbycochran dd0dcfa
PR4a S7: hard-cutover cleanup — remove dead legacy interface surface
robbycochran cf7c970
PR4a whole-spec review: honor type delta on update; audit adoptions
robbycochran 685b545
PR4a review: bound reconcile ctx, warn on absent create, fix SPEC flags
robbycochran File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -10,11 +10,12 @@ import ( | ||
| "github.com/stackrox/harness-openshell/internal/agent" | ||
| "github.com/stackrox/harness-openshell/internal/gateway" | ||
| "github.com/stackrox/harness-openshell/internal/openshell" | ||
| "github.com/stackrox/harness-openshell/internal/status" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
| func NewApplyCmd(harnessDir, cli string) *cobra.Command { | ||
| func NewApplyCmd(harnessDir, cli string, newClient openshell.Factory) *cobra.Command { | ||
| var ( | ||
| file string | ||
| agentName string | ||
| @@ -24,8 +25,8 @@ func NewApplyCmd(harnessDir, cli string) *cobra.Command { | ||
| task string | ||
| entrypoint string | ||
| attach bool | ||
| providerRefresh bool | ||
| dryRun bool | ||
| setupOnly bool | ||
| output string | ||
| ) | ||
| @@ -137,8 +138,9 @@ then deploy a sandbox. Use --dry-run to validate without deploying, or | ||
| agentPath: agentPath, | ||
| sandboxName: sandboxName, | ||
| noTTY: !attach, | ||
| providerRefresh: providerRefresh, | ||
| setupOnly: setupOnly, | ||
| harness: harness, | ||
| newClient: newClient, | ||
| retrySleep: 5 * time.Second, | ||
| }) | ||
| }, | ||
| @@ -152,8 +154,8 @@ then deploy a sandbox. Use --dry-run to validate without deploying, or | ||
| cmd.Flags().StringVar(&task, "task", "", "Task to pass to the agent (inline text or @filepath)") | ||
| cmd.Flags().StringVar(&entrypoint, "entrypoint", "", "Override agent entrypoint (claude, opencode, bash)") | ||
| cmd.Flags().BoolVar(&attach, "attach", false, "Attach TTY after creation (interactive mode)") | ||
| cmd.Flags().BoolVar(&providerRefresh, "provider-refresh", false, "Delete and recreate all providers") | ||
| cmd.Flags().BoolVar(&dryRun, "dry-run", false, "Validate configuration without deploying") | ||
| cmd.Flags().BoolVar(&setupOnly, "setup-only", false, "Deploy the gateway and reconcile providers/inference, but do not create a sandbox or run the agent") | ||
coderabbitai[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| cmd.Flags().StringVarP(&output, "output", "o", "", "Output format: yaml or json") | ||
| return cmd | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| package cmd | ||
| import ( | ||
| "github.com/stackrox/harness-openshell/internal/agent" | ||
| "github.com/stackrox/harness-openshell/internal/config" | ||
| ) | ||
| // desiredFromAgent bridges the legacy agent-config world (agent.AgentConfig, | ||
| // agent.ProviderRef) into the reconcile world (config.Provider, config.Inference). | ||
| // | ||
| // It is the single seam between the two config models. Today apply is driven by | ||
| // agent.AgentConfig while the SDK reconcile path (plan/reconcile) speaks | ||
| // config.Harness; until apply is migrated to author config.Harness directly this | ||
| // function is where the two meet. When that migration lands, this function — and | ||
| // only this function — is deleted. | ||
| // | ||
| // It is a classifier, not per-provider credential logic: it maps profile names | ||
| // to desired resources and derives the inference route from whichever configured | ||
| // provider serves inference. It never materializes secrets and never contacts a | ||
| // gateway. getenv is injected (production passes os.Getenv) so the OPENSHELL_MODEL | ||
| // default is testable. | ||
| func desiredFromAgent(agentCfg *agent.AgentConfig, getenv func(string) string) ([]config.Provider, config.Inference) { | ||
| model := getenv("OPENSHELL_MODEL") | ||
| if model == "" { | ||
| model = "claude-sonnet-4-6" | ||
| } | ||
| var providers []config.Provider | ||
| var inference config.Inference | ||
| for _, p := range agentCfg.Providers { | ||
| desired := config.Provider{ | ||
| Name: p.Profile, | ||
| // For the well-known agent-config profiles the profile name IS the | ||
| // gateway provider type; the config.Harness world can distinguish them, | ||
| // but here they coincide. | ||
| Type: p.Profile, | ||
| Management: managementFor(p.Profile), | ||
| Credentials: credentialSourceFor(p.Profile), | ||
| } | ||
| // A managed provider is one the harness bootstraps and owns; authorize | ||
| // reconcile to adopt it (stamp the owner label) on first pass, since the | ||
| // CLI-bridge create cannot stamp the SDK owner label itself. Referenced | ||
| // providers are never written, so Adopt is irrelevant to them. | ||
| if desired.Management == "managed" { | ||
| desired.Adopt = true | ||
| } | ||
| providers = append(providers, desired) | ||
| // The inference route points at whichever provider serves inference. | ||
| // Verify is left unset so config.Inference.VerifyEnabled defaults to | ||
| // true — verify-by-default. There is deliberately no agent-config field | ||
| // to opt out yet; the escape hatch (inference.verify: false) lives in the | ||
| // config.Harness world consumed by `harness plan`/reconcile. | ||
| if inferenceProviders[p.Profile] { | ||
| inference = config.Inference{ | ||
| Provider: p.Profile, | ||
| Model: model, | ||
| } | ||
| } | ||
| } | ||
| return providers, inference | ||
| } | ||
| // managementFor classifies a provider profile as "managed" (the harness owns its | ||
| // lifecycle — credentials and refresh flow through the gateway) or "referenced" | ||
| // (the harness only points at an existing registration). This mirrors the legacy | ||
| // registration split in registerProviders: ADC/OAuth-refresh providers are | ||
| // managed; the rest are referenced. | ||
| func managementFor(profile string) string { | ||
| switch profile { | ||
| case "google-vertex-ai", "google-workspace": | ||
| return "managed" | ||
| default: | ||
| return "referenced" | ||
| } | ||
| } | ||
| // credentialSourceFor records how a managed profile's credentials are acquired, | ||
| // without materializing any secret — it is the key providerCreatePlan dispatches | ||
| // on to pick the CLI-bridge create strategy. Vertex uses gcloud Application | ||
| // Default Credentials; google-workspace's OAuth path is keyed on its type, not a | ||
| // SecretRef, so it carries none; referenced providers have no harness-owned | ||
| // credentials. | ||
| func credentialSourceFor(profile string) *config.SecretRef { | ||
| switch profile { | ||
| case "google-vertex-ai": | ||
| return &config.SecretRef{Source: "gcloud-adc"} | ||
| default: | ||
| return nil | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| package cmd | ||
| import ( | ||
| "testing" | ||
| "github.com/stackrox/harness-openshell/internal/agent" | ||
| ) | ||
| func noEnv(string) string { return "" } | ||
| func TestDesiredFromAgent_InferenceFromVertexProvider(t *testing.T) { | ||
| agentCfg := &agent.AgentConfig{ | ||
| Providers: []agent.ProviderRef{ | ||
| {Profile: "github"}, | ||
| {Profile: "google-vertex-ai"}, | ||
| {Profile: "atlassian"}, | ||
| }, | ||
| } | ||
| _, inf := desiredFromAgent(agentCfg, noEnv) | ||
| if inf.Provider != "google-vertex-ai" { | ||
| t.Errorf("inference provider = %q, want google-vertex-ai", inf.Provider) | ||
| } | ||
| if inf.Model != "claude-sonnet-4-6" { | ||
| t.Errorf("inference model = %q, want default claude-sonnet-4-6", inf.Model) | ||
| } | ||
| // Verify unset → verify-by-default (the S5 behavior change). | ||
| if inf.Verify != nil { | ||
| t.Errorf("inference Verify = %v, want nil (verify-by-default)", *inf.Verify) | ||
| } | ||
| if !inf.VerifyEnabled() { | ||
| t.Error("VerifyEnabled() = false, want true for unset Verify") | ||
| } | ||
| } | ||
| func TestDesiredFromAgent_ModelFromEnv(t *testing.T) { | ||
| agentCfg := &agent.AgentConfig{ | ||
| Providers: []agent.ProviderRef{{Profile: "google-vertex-ai"}}, | ||
| } | ||
| getenv := func(k string) string { | ||
| if k == "OPENSHELL_MODEL" { | ||
| return "claude-opus-4-8" | ||
| } | ||
| return "" | ||
| } | ||
| _, inf := desiredFromAgent(agentCfg, getenv) | ||
| if inf.Model != "claude-opus-4-8" { | ||
| t.Errorf("inference model = %q, want claude-opus-4-8 from env", inf.Model) | ||
| } | ||
| } | ||
| func TestDesiredFromAgent_NoInferenceProvider(t *testing.T) { | ||
| agentCfg := &agent.AgentConfig{ | ||
| Providers: []agent.ProviderRef{ | ||
| {Profile: "github"}, | ||
| {Profile: "atlassian"}, | ||
| }, | ||
| } | ||
| _, inf := desiredFromAgent(agentCfg, noEnv) | ||
| if inf.Provider != "" || inf.Model != "" { | ||
| t.Errorf("inference = %+v, want empty (no inference provider configured)", inf) | ||
| } | ||
| } | ||
| func TestDesiredFromAgent_ProviderClassification(t *testing.T) { | ||
| agentCfg := &agent.AgentConfig{ | ||
| Providers: []agent.ProviderRef{ | ||
| {Profile: "github"}, | ||
| {Profile: "google-vertex-ai"}, | ||
| {Profile: "google-workspace"}, | ||
| {Profile: "atlassian"}, | ||
| }, | ||
| } | ||
| providers, _ := desiredFromAgent(agentCfg, noEnv) | ||
| if len(providers) != 4 { | ||
| t.Fatalf("got %d providers, want 4", len(providers)) | ||
| } | ||
| want := map[string]string{ | ||
| "github": "referenced", | ||
| "google-vertex-ai": "managed", | ||
| "google-workspace": "managed", | ||
| "atlassian": "referenced", | ||
| } | ||
| for _, p := range providers { | ||
| if p.Management != want[p.Name] { | ||
| t.Errorf("%s: Management = %q, want %q", p.Name, p.Management, want[p.Name]) | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Document reconciliation for normal apply.
Line 102 describes provider and inference reconciliation only for
--setup-only. Lines 85 and 87-98 still describe normal apply as gateway and provider deployment followed by sandbox creation. Update the apply steps to state that normal apply registers missing providers, reconciles providers and inference, and then creates the sandbox. Keep provider registration separate from reconciliation.As per path instructions:
SPEC.mdis authoritative, and apply must deploy the gateway and reconcile providers/inference before setup-only stops execution; provider registration and reconciliation must remain distinct.🤖 Prompt for AI Agents
Source: Path instructions