From 1ef7e95de3463fa7151e025adcd81ca27c2d3c6a Mon Sep 17 00:00:00 2001 From: Taras Yemets Date: Wed, 27 May 2026 16:28:09 +0300 Subject: [PATCH 01/11] feat/batch-changes: support codingAgent steps in batch exec Vendors lib/batches/codingagent + lib/batches/codex from sourcegraph/ sourcegraph, adds the v3 codingAgent step + image alias to the spec + schema, and rewrites step.Run via codingagent.RenderRunCommand when step.CodingAgent is set. SRC_BATCHES_MODEL_PROVIDER_TOKEN and SRC_BATCHES_JOB_ID are forwarded from src's own env into the user container so the agent CLI can reach the server's model-provider proxy. Demo/MVP for v1 codingAgent support. Not for merge as-is. --- .gitignore | 1 + cmd/src/batch_exec.go | 1 + internal/batches/executor/run_steps.go | 69 +++++++++++++++- internal/batches/executor/task.go | 3 + lib/batches/batch_spec.go | 40 ++++++++-- lib/batches/codex/codex.go | 43 ++++++++++ lib/batches/codingagent/codingagent.go | 87 +++++++++++++++++++++ lib/batches/codingagent/codingagent_test.go | 52 ++++++++++++ lib/batches/codingagent/types/types.go | 40 ++++++++++ lib/batches/schema/batch_spec_stringdata.go | 30 ++++++- lib/batches/workspaces_execution_input.go | 3 + lib/go.mod | 1 + lib/go.sum | 2 + 13 files changed, 362 insertions(+), 10 deletions(-) create mode 100644 lib/batches/codex/codex.go create mode 100644 lib/batches/codingagent/codingagent.go create mode 100644 lib/batches/codingagent/codingagent_test.go create mode 100644 lib/batches/codingagent/types/types.go diff --git a/.gitignore b/.gitignore index 0091b7092a..e7be9bbe06 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ bazel-src-cli .DS_Store samples .amp +bin/ diff --git a/cmd/src/batch_exec.go b/cmd/src/batch_exec.go index 11aadf39e3..a8cc9584c9 100644 --- a/cmd/src/batch_exec.go +++ b/cmd/src/batch_exec.go @@ -278,6 +278,7 @@ func convertWorkspace(w batcheslib.WorkspacesExecutionInput) *executor.Task { BatchChangeAttributes: &w.BatchChangeAttributes, CachedStepResultFound: w.CachedStepResultFound, CachedStepResult: w.CachedStepResult, + ModelProviderURL: w.ModelProviderURL, } return task diff --git a/internal/batches/executor/run_steps.go b/internal/batches/executor/run_steps.go index 47b5715887..f6664d157a 100644 --- a/internal/batches/executor/run_steps.go +++ b/internal/batches/executor/run_steps.go @@ -14,6 +14,8 @@ import ( "time" batcheslib "github.com/sourcegraph/sourcegraph/lib/batches" + "github.com/sourcegraph/sourcegraph/lib/batches/codingagent" + codingagenttypes "github.com/sourcegraph/sourcegraph/lib/batches/codingagent/types" "github.com/sourcegraph/sourcegraph/lib/batches/execution" "github.com/sourcegraph/sourcegraph/lib/batches/git" "github.com/sourcegraph/sourcegraph/lib/batches/template" @@ -272,12 +274,32 @@ func executeSingleStep( return bytes.Buffer{}, bytes.Buffer{}, err } - runScriptFile, runScript, cleanup, err := createRunScriptFile(ctx, opts.TempDir, step.Run, stepContext) + var ( + runScriptFile string + runScript string + runScriptCleanup func() + ) + if step.CodingAgent != nil { + if opts.Task.ModelProviderURL == "" { + err = errors.New("codingAgent step requires WorkspacesExecutionInput.ModelProviderURL to be set") + opts.UI.StepPreparingFailed(stepIdx+1, err) + return bytes.Buffer{}, bytes.Buffer{}, err + } + runScript, err = codingagent.RenderRunCommand(step.CodingAgent, opts.Task.ModelProviderURL, stepContext) + if err != nil { + err = errors.Wrap(err, "rendering codingAgent step") + opts.UI.StepPreparingFailed(stepIdx+1, err) + return bytes.Buffer{}, bytes.Buffer{}, err + } + runScriptFile, runScriptCleanup, err = writeRunScriptFile(opts.TempDir, runScript) + } else { + runScriptFile, runScript, runScriptCleanup, err = createRunScriptFile(ctx, opts.TempDir, step.Run, stepContext) + } if err != nil { opts.UI.StepPreparingFailed(stepIdx+1, err) return bytes.Buffer{}, bytes.Buffer{}, err } - defer cleanup() + defer runScriptCleanup() // Parse and render the step.Files. filesToMount, cleanup, err := createFilesToMount(opts.TempDir, step, stepContext) @@ -303,6 +325,21 @@ func executeSingleStep( return bytes.Buffer{}, bytes.Buffer{}, err } + // For codingAgent steps, forward the model-provider auth env from the + // executor's environment (injected via CliStep.Env and JobTokenEnvVar on + // the server) into the user container so the agent CLI can talk to the + // /model-provider/batches proxy. + if step.CodingAgent != nil { + for _, key := range []string{codingagenttypes.ModelProviderTokenEnvVar, codingagenttypes.JobIDEnvVar} { + for _, e := range opts.GlobalEnv { + if v, ok := strings.CutPrefix(e, key+"="); ok { + env[key] = v + break + } + } + } + } + opts.UI.StepPreparingSuccess(stepIdx + 1) // ---------- @@ -573,6 +610,34 @@ func createRunScriptFile(ctx context.Context, tempDir string, stepRun string, st return runScriptFile.Name(), runScript.String(), cleanup, nil } +// writeRunScriptFile writes a pre-rendered run script (e.g. a codingAgent +// step desugared by the codingagent registry) verbatim to a temp file, with +// the same permission semantics as createRunScriptFile. Unlike that helper, +// this one does NOT pass the content through template.RenderStepTemplate: +// the script is treated as opaque shell text so embedded sequences like +// `{{` inside a shell-quoted prompt are not re-parsed as templates. +func writeRunScriptFile(tempDir, script string) (string, func(), error) { + runScriptFile, err := os.CreateTemp(tempDir, "") + if err != nil { + return "", nil, errors.Wrap(err, "creating temporary file") + } + cleanup := func() { os.Remove(runScriptFile.Name()) } + + if _, err := runScriptFile.WriteString(script); err != nil { + cleanup() + return "", nil, errors.Wrap(err, "writing temporary file") + } + if err := runScriptFile.Close(); err != nil { + cleanup() + return "", nil, errors.Wrap(err, "closing temporary file") + } + if err := os.Chmod(runScriptFile.Name(), 0644); err != nil { + cleanup() + return "", nil, errors.Wrap(err, "setting permissions on the temporary file") + } + return runScriptFile.Name(), cleanup, nil +} + // createCidFile creates a temporary file that will contain the container ID // when executing steps. // It returns the location of the file and a function that cleans up the diff --git a/internal/batches/executor/task.go b/internal/batches/executor/task.go index ee7f8ad55e..7c9718396c 100644 --- a/internal/batches/executor/task.go +++ b/internal/batches/executor/task.go @@ -29,6 +29,9 @@ type Task struct { // When this field is true, CachedStepResult is also populated. CachedStepResultFound bool CachedStepResult execution.AfterStepResult + // ModelProviderURL is the resolved proxy base URL for coding-agent + // steps; empty unless the spec contains at least one codingAgent step. + ModelProviderURL string } func (t *Task) ArchivePathToFetch() string { diff --git a/lib/batches/batch_spec.go b/lib/batches/batch_spec.go index 269093a82d..6ffa91f462 100644 --- a/lib/batches/batch_spec.go +++ b/lib/batches/batch_spec.go @@ -90,13 +90,29 @@ func (oqor *OnQueryOrRepository) GetBranches() ([]string, error) { } type Step struct { - Run string `json:"run,omitempty" yaml:"run"` - Container string `json:"container,omitempty" yaml:"container"` - Env env.Environment `json:"env" yaml:"env"` - Files map[string]string `json:"files,omitempty" yaml:"files,omitempty"` - Outputs Outputs `json:"outputs,omitempty" yaml:"outputs,omitempty"` - Mount []Mount `json:"mount,omitempty" yaml:"mount,omitempty"` - If any `json:"if,omitempty" yaml:"if,omitempty"` + Run string `json:"run,omitempty" yaml:"run"` + CodingAgent *CodingAgentStep `json:"codingAgent,omitempty" yaml:"codingAgent,omitempty"` + Container string `json:"container,omitempty" yaml:"container"` + Image string `json:"image,omitempty" yaml:"image,omitempty"` + Env env.Environment `json:"env" yaml:"env"` + Files map[string]string `json:"files,omitempty" yaml:"files,omitempty"` + Outputs Outputs `json:"outputs,omitempty" yaml:"outputs,omitempty"` + Mount []Mount `json:"mount,omitempty" yaml:"mount,omitempty"` + If any `json:"if,omitempty" yaml:"if,omitempty"` +} + +// CodingAgentType identifies a registered coding-agent implementation. +type CodingAgentType string + +const ( + CodingAgentTypeCodex CodingAgentType = "codex" +) + +// CodingAgentStep is a v3-spec step that delegates the step's work to a +// coding agent CLI invoked via the server-side model-provider proxy. +type CodingAgentStep struct { + Type CodingAgentType `json:"type,omitempty" yaml:"type"` + Prompt string `json:"prompt,omitempty" yaml:"prompt"` } func (s *Step) IfCondition() string { @@ -161,6 +177,16 @@ func parseBatchSpec(schema string, data []byte) (*BatchSpec, error) { return nil, err } + if spec.Version == 3 { + // Mirror v3 `image:` into `container:` so executor consumers that + // read step.Container keep working. + for i := range spec.Steps { + if spec.Steps[i].Image != "" { + spec.Steps[i].Container = spec.Steps[i].Image + } + } + } + var errs error if len(spec.Steps) != 0 && spec.ChangesetTemplate == nil { diff --git a/lib/batches/codex/codex.go b/lib/batches/codex/codex.go new file mode 100644 index 0000000000..6ce809dfa7 --- /dev/null +++ b/lib/batches/codex/codex.go @@ -0,0 +1,43 @@ +// Package codex implements the codex coding agent run-command rewrite. +package codex + +import ( + "fmt" + + "github.com/kballard/go-shellquote" + + batcheslib "github.com/sourcegraph/sourcegraph/lib/batches" + "github.com/sourcegraph/sourcegraph/lib/batches/codingagent/types" +) + +const model = "gpt-5.4" + +var routes = []types.ModelProviderRoute{ + {WirePath: "/responses", UpstreamPath: "/v1/completions/openai-responses"}, +} + +type Agent struct{} + +func (Agent) Type() batcheslib.CodingAgentType { return batcheslib.CodingAgentTypeCodex } +func (Agent) ModelProviderRoutes() []types.ModelProviderRoute { return routes } +func (Agent) ImageRequirements() []string { return []string{"codex"} } + +func (Agent) RunCommand(prompt, modelProviderURL string) string { + return shellquote.Join( + "codex", + "exec", + "--json", + "--sandbox", "danger-full-access", + "--ephemeral", + "--model", model, + "-c", `approval_policy="never"`, + "-c", `model_reasoning_effort="medium"`, + "-c", `model_provider="sourcegraph"`, + "-c", `model_providers.sourcegraph.name="Sourcegraph"`, + "-c", fmt.Sprintf(`model_providers.sourcegraph.base_url=%q`, modelProviderURL), + "-c", fmt.Sprintf(`model_providers.sourcegraph.env_key=%q`, types.ModelProviderTokenEnvVar), + "-c", fmt.Sprintf(`model_providers.sourcegraph.env_http_headers={%q=%q}`, types.JobIDHeaderName, types.JobIDEnvVar), + "-c", `model_providers.sourcegraph.wire_api="responses"`, + prompt, + ) +} diff --git a/lib/batches/codingagent/codingagent.go b/lib/batches/codingagent/codingagent.go new file mode 100644 index 0000000000..8256986d14 --- /dev/null +++ b/lib/batches/codingagent/codingagent.go @@ -0,0 +1,87 @@ +// Package codingagent rewrites v3 batch spec coding-agent steps into the +// shell commands that drive them. Register new agents in the agents list. +package codingagent + +import ( + "bytes" + "fmt" + "strings" + + "github.com/kballard/go-shellquote" + + batcheslib "github.com/sourcegraph/sourcegraph/lib/batches" + "github.com/sourcegraph/sourcegraph/lib/batches/codex" + "github.com/sourcegraph/sourcegraph/lib/batches/codingagent/types" + "github.com/sourcegraph/sourcegraph/lib/batches/template" + "github.com/sourcegraph/sourcegraph/lib/errors" +) + +// ErrUnknownType is returned when a codingAgent step references a type that +// has no registered Agent. +var ErrUnknownType = errors.New("unknown codingAgent type") + +// RenderRunCommand returns the shell-quoted command that runs agentStep. The +// prompt is rendered before quoting; reversing would let templated values +// break out of the shell quoting. +func RenderRunCommand(agentStep *batcheslib.CodingAgentStep, modelProviderURL string, stepCtx *template.StepContext) (string, error) { + a, ok := agents[agentStep.Type] + if !ok { + return "", errors.Wrapf(ErrUnknownType, "codingAgent type %q", agentStep.Type) + } + var renderedPrompt bytes.Buffer + if err := template.RenderStepTemplate("codingagent-prompt", agentStep.Prompt, &renderedPrompt, stepCtx); err != nil { + return "", errors.Wrap(err, "rendering codingAgent.prompt") + } + prefixed := strings.TrimRight(modelProviderURL, "/") + "/" + string(a.Type()) + + var b strings.Builder + for _, binary := range a.ImageRequirements() { + b.WriteString(failIfMissing(a.Type(), binary)) + } + b.WriteString(a.RunCommand(renderedPrompt.String(), prefixed)) + return b.String(), nil +} + +// failIfMissing returns a shell snippet that, when prepended to the step +// script, writes a message to stderr and exits the container with status 1 +// if binary isn't on PATH. +func failIfMissing(agentType batcheslib.CodingAgentType, binary string) string { + msg := fmt.Sprintf( + "codingAgent %q requires %q on PATH in the run container; ensure your image includes the %s binary", + agentType, binary, binary, + ) + return fmt.Sprintf("command -v %s >/dev/null 2>&1 || { echo %s >&2; exit 1; }\n", + binary, + shellquote.Join(msg), + ) +} + +var agents = func() map[batcheslib.CodingAgentType]types.Agent { + out := map[batcheslib.CodingAgentType]types.Agent{} + for _, a := range []types.Agent{ + codex.Agent{}, + } { + if _, exists := out[a.Type()]; exists { + panic("duplicate codingagent agent for " + a.Type()) + } + out[a.Type()] = a + } + return out +}() + +// RegisteredModelProviderRoutes returns the model-provider proxy routes +// contributed by every registered Agent, each WirePath prefixed with +// its agent type. +func RegisteredModelProviderRoutes() []types.ModelProviderRoute { + var out []types.ModelProviderRoute + for _, a := range agents { + prefix := "/" + string(a.Type()) + for _, route := range a.ModelProviderRoutes() { + out = append(out, types.ModelProviderRoute{ + WirePath: prefix + route.WirePath, + UpstreamPath: route.UpstreamPath, + }) + } + } + return out +} diff --git a/lib/batches/codingagent/codingagent_test.go b/lib/batches/codingagent/codingagent_test.go new file mode 100644 index 0000000000..dcf50e634e --- /dev/null +++ b/lib/batches/codingagent/codingagent_test.go @@ -0,0 +1,52 @@ +package codingagent_test + +import ( + "errors" + "testing" + + "github.com/kballard/go-shellquote" + + batcheslib "github.com/sourcegraph/sourcegraph/lib/batches" + "github.com/sourcegraph/sourcegraph/lib/batches/codingagent" + "github.com/sourcegraph/sourcegraph/lib/batches/template" +) + +func TestRenderRunCommand_unknownType(t *testing.T) { + agentStep := &batcheslib.CodingAgentStep{Type: "nope", Prompt: "x"} + _, err := codingagent.RenderRunCommand(agentStep, "https://example/", &template.StepContext{}) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, codingagent.ErrUnknownType) { + t.Fatalf("expected ErrUnknownType, got %v", err) + } +} + +// TestRenderRunCommand_promptShellQuoting ensures the rendered prompt is +// shell-quoted as a single argument even when it contains shell +// metacharacters (apostrophes, semicolons), so prompt text can't break out +// into the host shell. +func TestRenderRunCommand_promptShellQuoting(t *testing.T) { + const repoName = `github.com/sourcegraph/sourcegraph` + prompt := "You're working in the ${{ repository.name }} repository.\n" + + "Add a README section describing the project; don't touch existing files." + agentStep := &batcheslib.CodingAgentStep{ + Type: batcheslib.CodingAgentTypeCodex, + Prompt: prompt, + } + stepCtx := &template.StepContext{Repository: template.Repository{Name: repoName}} + cmd, err := codingagent.RenderRunCommand(agentStep, "https://example/", stepCtx) + if err != nil { + t.Fatal(err) + } + + tokens, err := shellquote.Split(cmd) + if err != nil { + t.Fatal(err) + } + wantPrompt := "You're working in the " + repoName + " repository.\n" + + "Add a README section describing the project; don't touch existing files." + if got := tokens[len(tokens)-1]; got != wantPrompt { + t.Fatalf("prompt mismatch:\n got: %q\n want: %q", got, wantPrompt) + } +} diff --git a/lib/batches/codingagent/types/types.go b/lib/batches/codingagent/types/types.go new file mode 100644 index 0000000000..8d11201551 --- /dev/null +++ b/lib/batches/codingagent/types/types.go @@ -0,0 +1,40 @@ +// Package types holds the contract shared between the codingagent registry +// and individual coding-agent implementations, in a leaf package to avoid an +// import cycle with the registry. +package types + +import ( + batcheslib "github.com/sourcegraph/sourcegraph/lib/batches" +) + +const ModelProviderTokenEnvVar = "SRC_BATCHES_MODEL_PROVIDER_TOKEN" + +// JobIDEnvVar binds ModelProviderTokenEnvVar to its job; sent as JobIDHeaderName. +const JobIDEnvVar = "SRC_BATCHES_JOB_ID" + +const JobIDHeaderName = "X-Sourcegraph-Job-ID" + +type Agent interface { + Type() batcheslib.CodingAgentType + // RunCommand receives the already-templated prompt and MUST shell-quote it. + RunCommand(renderedPrompt, modelProviderURL string) string + // ModelProviderRoutes returns routes whose WirePath is relative to the + // agent type; the registry prefixes "/" before exposing them. + ModelProviderRoutes() []ModelProviderRoute + // ImageRequirements returns binaries the agent expects on PATH in the run + // container. The registry emits a check before RunCommand. + ImageRequirements() []string +} + +// ModelProviderRoute is one wire→upstream mapping served by the Sourcegraph +// model-provider proxy (mounted under /.executors/model-provider/batches). +type ModelProviderRoute struct { + // WirePath is the proxy subpath the agent CLI POSTs to, relative to the + // agent type (e.g. "/responses"). The registry prefixes "/" + // before registering it on the router (e.g. "/codex/responses"). + WirePath string + // UpstreamPath is the path on the Sourcegraph Model Provider upstream + // (e.g. Cody Gateway) that serves WirePath, such as + // "/v1/completions/openai-responses". + UpstreamPath string +} diff --git a/lib/batches/schema/batch_spec_stringdata.go b/lib/batches/schema/batch_spec_stringdata.go index 1123970faf..31d59f1d74 100644 --- a/lib/batches/schema/batch_spec_stringdata.go +++ b/lib/batches/schema/batch_spec_stringdata.go @@ -128,7 +128,12 @@ const BatchSpecJSON = `{ "type": "object", "description": "A command to run (as part of a sequence) in a repository branch to produce the required changes.", "additionalProperties": false, - "required": ["run", "container"], + "anyOf": [ + { "required": ["run", "container"] }, + { "required": ["run", "image"] }, + { "required": ["codingAgent", "container"] }, + { "required": ["codingAgent", "image"] } + ], "properties": { "run": { "type": "string", @@ -139,6 +144,29 @@ const BatchSpecJSON = `{ "description": "The Docker image used to launch the Docker container in which the shell command is run.", "examples": ["alpine:3"] }, + "image": { + "type": "string", + "description": "The Docker image used to launch the Docker container in which the shell command is run. Equivalent to 'container' in version 3 batch specs.", + "examples": ["alpine:3"] + }, + "codingAgent": { + "title": "CodingAgent", + "type": "object", + "description": "An out-of-the-box coding agent step that runs the given prompt inside a managed container. Mutually exclusive with run. Only supported in version 3 batch specs.", + "additionalProperties": false, + "required": ["type", "prompt"], + "properties": { + "type": { + "type": "string", + "description": "The coding agent to use.", + "enum": ["codex"] + }, + "prompt": { + "type": "string", + "description": "The prompt to send to the agent." + } + } + }, "outputs": { "type": ["object", "null"], "description": "Output variables of this step that can be referenced in the changesetTemplate or other steps via outputs.", diff --git a/lib/batches/workspaces_execution_input.go b/lib/batches/workspaces_execution_input.go index 76a5a8cb18..c63e0d881c 100644 --- a/lib/batches/workspaces_execution_input.go +++ b/lib/batches/workspaces_execution_input.go @@ -21,6 +21,9 @@ type WorkspacesExecutionInput struct { CachedStepResult execution.AfterStepResult `json:"cachedStepResult"` // SkippedSteps determines which steps are skipped in the execution. SkippedSteps map[int]struct{} `json:"skippedSteps"` + // ModelProviderURL is the resolved proxy base URL for coding-agent + // steps; only set when the spec contains at least one codingAgent step. + ModelProviderURL string `json:"modelProviderURL,omitempty"` } type WorkspaceRepo struct { diff --git a/lib/go.mod b/lib/go.mod index 44fdc19356..67fe8a2790 100644 --- a/lib/go.mod +++ b/lib/go.mod @@ -56,6 +56,7 @@ require ( github.com/gorilla/css v1.0.1 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect + github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect github.com/klauspost/compress v1.18.0 // indirect github.com/kr/pretty v0.3.1 // indirect github.com/kr/text v0.2.0 // indirect diff --git a/lib/go.sum b/lib/go.sum index 434c58adc8..dd1329c5ff 100644 --- a/lib/go.sum +++ b/lib/go.sum @@ -93,6 +93,8 @@ github.com/jackc/pgx/v5 v5.9.2 h1:3ZhOzMWnR4yJ+RW1XImIPsD1aNSz4T4fyP7zlQb56hw= github.com/jackc/pgx/v5 v5.9.2/go.mod h1:mal1tBGAFfLHvZzaYh77YS/eC6IX9OWbRV1QIIM0Jn4= github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo= github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= +github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= +github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo= From 7612fb34a38ab43f319f14992fdef2a7ca279932 Mon Sep 17 00:00:00 2001 From: Taras Yemets Date: Thu, 28 May 2026 13:26:14 +0300 Subject: [PATCH 02/11] feat/batch-changes: address sourcegraph#12503 review for v1 codingAgent port Apply review feedback from sourcegraph/sourcegraph#12503 (where the upstream codingAgent step lives) to the src-cli v1 port. The same lib/batches/codingagent surface is shared, so the same review applies here. Changes: - lib/batches/codex: switch to upstream-recommended curl-based install (codex/claude code both recommend it), pin codex CLI version, drop dependency on the agent CLI being pre-baked in the run image. ImageRequirements is now [curl, tar] so misconfigured images fail fast at step start. - lib/batches/codingagent/types: add Agent.InstallScript and InstallDir so each agent owns its own pinned install at a Sourcegraph-controlled path, ignoring whatever the user image ships. - lib/batches: add Step.MarshalJSON to canonicalize v3 image: into container: on the wire. Without this the prep-side cache key (which marshals Step to JSON) includes image while src-cli's executor side does not, producing silent cache misses on every v3 spec. - lib/batches: reject codingAgent + run in the same step at parse time instead of silently picking one. - internal/batches/executor: extract forwardCodingAgentEnv with a docstring spelling out what's forwarded and why. - Tests for forwardCodingAgentEnv and the v3 image/codingAgent parse paths. Test Plan: - go test ./internal/batches/executor/ (root module) - cd lib && go test ./batches/... --- internal/batches/executor/run_steps.go | 25 +- internal/batches/executor/run_steps_test.go | 77 ++++ lib/batches/batch_spec.go | 37 +- lib/batches/batch_spec_test.go | 83 ++++ lib/batches/codex/codex.go | 26 +- lib/batches/codingagent/codingagent.go | 5 +- lib/batches/codingagent/types/types.go | 12 +- lib/batches/schema/batch_spec_stringdata.go | 481 ++++++++++++++------ 8 files changed, 572 insertions(+), 174 deletions(-) create mode 100644 internal/batches/executor/run_steps_test.go create mode 100644 lib/batches/batch_spec_test.go diff --git a/internal/batches/executor/run_steps.go b/internal/batches/executor/run_steps.go index f6664d157a..eb22e153db 100644 --- a/internal/batches/executor/run_steps.go +++ b/internal/batches/executor/run_steps.go @@ -330,14 +330,7 @@ func executeSingleStep( // the server) into the user container so the agent CLI can talk to the // /model-provider/batches proxy. if step.CodingAgent != nil { - for _, key := range []string{codingagenttypes.ModelProviderTokenEnvVar, codingagenttypes.JobIDEnvVar} { - for _, e := range opts.GlobalEnv { - if v, ok := strings.CutPrefix(e, key+"="); ok { - env[key] = v - break - } - } - } + forwardCodingAgentEnv(opts.GlobalEnv, env) } opts.UI.StepPreparingSuccess(stepIdx + 1) @@ -610,6 +603,22 @@ func createRunScriptFile(ctx context.Context, tempDir string, stepRun string, st return runScriptFile.Name(), runScript.String(), cleanup, nil } +// forwardCodingAgentEnv copies the model-provider auth env vars +// (SRC_BATCHES_MODEL_PROVIDER_TOKEN, SRC_BATCHES_JOB_ID) from globalEnv into +// stepEnv. These are placed on the v1 CliStep that runs `src batch exec` by +// the Sourcegraph server; the agent CLI in the user container needs them to +// reach the /.executors/model-provider/batches proxy. +func forwardCodingAgentEnv(globalEnv []string, stepEnv map[string]string) { + for _, key := range []string{codingagenttypes.ModelProviderTokenEnvVar, codingagenttypes.JobIDEnvVar} { + for _, e := range globalEnv { + if v, ok := strings.CutPrefix(e, key+"="); ok { + stepEnv[key] = v + break + } + } + } +} + // writeRunScriptFile writes a pre-rendered run script (e.g. a codingAgent // step desugared by the codingagent registry) verbatim to a temp file, with // the same permission semantics as createRunScriptFile. Unlike that helper, diff --git a/internal/batches/executor/run_steps_test.go b/internal/batches/executor/run_steps_test.go new file mode 100644 index 0000000000..23ec6bfc3d --- /dev/null +++ b/internal/batches/executor/run_steps_test.go @@ -0,0 +1,77 @@ +package executor + +import ( + "testing" + + codingagenttypes "github.com/sourcegraph/sourcegraph/lib/batches/codingagent/types" +) + +// TestForwardCodingAgentEnv verifies that the model-provider auth env vars +// placed on the v1 CliStep by the Sourcegraph server are forwarded into the +// user container env for codingAgent steps. +func TestForwardCodingAgentEnv(t *testing.T) { + cases := []struct { + name string + globalEnv []string + stepEnv map[string]string + want map[string]string + }{ + { + name: "forwards both vars", + globalEnv: []string{ + "PATH=/bin", + codingagenttypes.ModelProviderTokenEnvVar + "=tok-abc", + codingagenttypes.JobIDEnvVar + "=job-123", + }, + stepEnv: map[string]string{}, + want: map[string]string{ + codingagenttypes.ModelProviderTokenEnvVar: "tok-abc", + codingagenttypes.JobIDEnvVar: "job-123", + }, + }, + { + name: "forwards only what is set", + globalEnv: []string{ + codingagenttypes.JobIDEnvVar + "=job-456", + }, + stepEnv: map[string]string{}, + want: map[string]string{ + codingagenttypes.JobIDEnvVar: "job-456", + }, + }, + { + name: "preserves preexisting step env and overwrites on match", + globalEnv: []string{ + codingagenttypes.ModelProviderTokenEnvVar + "=from-global", + }, + stepEnv: map[string]string{ + "OTHER": "x", + codingagenttypes.ModelProviderTokenEnvVar: "from-step", + }, + want: map[string]string{ + "OTHER": "x", + codingagenttypes.ModelProviderTokenEnvVar: "from-global", + }, + }, + { + name: "no-op when env not present", + globalEnv: []string{"PATH=/bin"}, + stepEnv: map[string]string{}, + want: map[string]string{}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + forwardCodingAgentEnv(tc.globalEnv, tc.stepEnv) + if len(tc.stepEnv) != len(tc.want) { + t.Fatalf("len mismatch: got %d want %d (got=%v want=%v)", len(tc.stepEnv), len(tc.want), tc.stepEnv, tc.want) + } + for k, v := range tc.want { + if got := tc.stepEnv[k]; got != v { + t.Errorf("env[%q]: got %q want %q", k, got, v) + } + } + }) + } +} diff --git a/lib/batches/batch_spec.go b/lib/batches/batch_spec.go index 6ffa91f462..83b5d9bb6f 100644 --- a/lib/batches/batch_spec.go +++ b/lib/batches/batch_spec.go @@ -1,6 +1,7 @@ package batches import ( + "encoding/json" "fmt" "strings" @@ -93,7 +94,7 @@ type Step struct { Run string `json:"run,omitempty" yaml:"run"` CodingAgent *CodingAgentStep `json:"codingAgent,omitempty" yaml:"codingAgent,omitempty"` Container string `json:"container,omitempty" yaml:"container"` - Image string `json:"image,omitempty" yaml:"image,omitempty"` + Image string `json:"image,omitempty" yaml:"image"` Env env.Environment `json:"env" yaml:"env"` Files map[string]string `json:"files,omitempty" yaml:"files,omitempty"` Outputs Outputs `json:"outputs,omitempty" yaml:"outputs,omitempty"` @@ -101,20 +102,35 @@ type Step struct { If any `json:"if,omitempty" yaml:"if,omitempty"` } -// CodingAgentType identifies a registered coding-agent implementation. type CodingAgentType string const ( CodingAgentTypeCodex CodingAgentType = "codex" ) -// CodingAgentStep is a v3-spec step that delegates the step's work to a -// coding agent CLI invoked via the server-side model-provider proxy. type CodingAgentStep struct { Type CodingAgentType `json:"type,omitempty" yaml:"type"` Prompt string `json:"prompt,omitempty" yaml:"prompt"` } +// MarshalJSON canonicalizes the v3 `image:` field into `container:` on the +// wire. Both fields exist on Step for ergonomic reasons (v3 specs use +// `image:`, v1/v2 specs use `container:`), but src-cli's executor reads +// `Container`. Without canonicalization, the prep-side cache key — computed +// by JSON-marshaling Step — would include `image` while the executor side +// (which round-trips through src-cli) would not, producing divergent keys +// and silent cache misses for any v3 spec. +func (s Step) MarshalJSON() ([]byte, error) { + // Use an alias type to avoid infinite recursion through MarshalJSON. + type stepAlias Step + canon := stepAlias(s) + if canon.Container == "" { + canon.Container = canon.Image + } + canon.Image = "" + return json.Marshal(canon) +} + func (s *Step) IfCondition() string { switch v := s.If.(type) { case bool: @@ -178,12 +194,12 @@ func parseBatchSpec(schema string, data []byte) (*BatchSpec, error) { } if spec.Version == 3 { - // Mirror v3 `image:` into `container:` so executor consumers that - // read step.Container keep working. + // Mirror v3 `image:` into `container:` so in-memory consumers that + // read step.Container (e.g. the executor transform) keep working. + // JSON serialization is canonicalized separately in Step.MarshalJSON + // so prep-side cache hashing matches src-cli/executor serialization. for i := range spec.Steps { - if spec.Steps[i].Image != "" { - spec.Steps[i].Container = spec.Steps[i].Image - } + spec.Steps[i].Container = spec.Steps[i].Image } } @@ -207,6 +223,9 @@ func parseBatchSpec(schema string, data []byte) (*BatchSpec, error) { errs = errors.Append(errs, NewValidationError(errors.Newf("step %d files target path contains invalid characters", i+1))) } } + if step.CodingAgent != nil && step.Run != "" { + errs = errors.Append(errs, NewValidationError(errors.Newf("step %d: codingAgent and run cannot be combined in the same step", i+1))) + } } return &spec, errs diff --git a/lib/batches/batch_spec_test.go b/lib/batches/batch_spec_test.go new file mode 100644 index 0000000000..8cd0686bf0 --- /dev/null +++ b/lib/batches/batch_spec_test.go @@ -0,0 +1,83 @@ +package batches + +import ( + "testing" +) + +// TestParseBatchSpec_v3_imageMirroredToContainer ensures that in v3 specs the +// step-level `image:` field is mirrored into Step.Container so executor +// consumers that read step.Container keep working. +func TestParseBatchSpec_v3_imageMirroredToContainer(t *testing.T) { + spec := []byte(` +version: 3 +name: test +description: test +on: + - repository: github.com/sourcegraph/sourcegraph +steps: + - run: echo hi + image: alpine:3 +changesetTemplate: + title: test + body: test + branch: test + commit: + message: test +`) + got, err := ParseBatchSpec(spec) + if err != nil { + t.Fatalf("ParseBatchSpec failed: %v", err) + } + if len(got.Steps) != 1 { + t.Fatalf("expected 1 step, got %d", len(got.Steps)) + } + if got.Steps[0].Image != "alpine:3" { + t.Errorf("Step.Image: got %q want %q", got.Steps[0].Image, "alpine:3") + } + if got.Steps[0].Container != "alpine:3" { + t.Errorf("Step.Container (mirrored from image): got %q want %q", got.Steps[0].Container, "alpine:3") + } +} + +// TestParseBatchSpec_v3_codingAgentStep ensures that a v3 spec with a +// codingAgent step parses with the expected typed step. +func TestParseBatchSpec_v3_codingAgentStep(t *testing.T) { + spec := []byte(` +version: 3 +name: test +description: test +on: + - repository: github.com/sourcegraph/sourcegraph +steps: + - codingAgent: + type: codex + prompt: do the thing + image: alpine:3 +changesetTemplate: + title: test + body: test + branch: test + commit: + message: test +`) + got, err := ParseBatchSpec(spec) + if err != nil { + t.Fatalf("ParseBatchSpec failed: %v", err) + } + if len(got.Steps) != 1 { + t.Fatalf("expected 1 step, got %d", len(got.Steps)) + } + step := got.Steps[0] + if step.CodingAgent == nil { + t.Fatal("expected step.CodingAgent to be set") + } + if step.CodingAgent.Type != CodingAgentTypeCodex { + t.Errorf("CodingAgent.Type: got %q want %q", step.CodingAgent.Type, CodingAgentTypeCodex) + } + if step.CodingAgent.Prompt != "do the thing" { + t.Errorf("CodingAgent.Prompt: got %q want %q", step.CodingAgent.Prompt, "do the thing") + } + if step.Container != "alpine:3" { + t.Errorf("Step.Container (mirrored from image): got %q want %q", step.Container, "alpine:3") + } +} diff --git a/lib/batches/codex/codex.go b/lib/batches/codex/codex.go index 6ce809dfa7..a20fc4ecf4 100644 --- a/lib/batches/codex/codex.go +++ b/lib/batches/codex/codex.go @@ -12,6 +12,10 @@ import ( const model = "gpt-5.4" +// pinnedVersion is the codex CLI release we test against. Bump in lockstep +// with the model_providers.* config keys below; codex CLI is pre-v1. +const pinnedVersion = "0.134.0" + var routes = []types.ModelProviderRoute{ {WirePath: "/responses", UpstreamPath: "/v1/completions/openai-responses"}, } @@ -20,11 +24,29 @@ type Agent struct{} func (Agent) Type() batcheslib.CodingAgentType { return batcheslib.CodingAgentTypeCodex } func (Agent) ModelProviderRoutes() []types.ModelProviderRoute { return routes } -func (Agent) ImageRequirements() []string { return []string{"codex"} } +func (Agent) ImageRequirements() []string { return []string{"curl", "tar"} } + +// InstallScript installs codex from GitHub Releases into types.InstallDir. +func (Agent) InstallScript() string { + return fmt.Sprintf(`_install_dir=%s +_version=%s +_codex_arch=$(uname -m) +case "$_codex_arch" in + x86_64) _codex_triple=x86_64-unknown-linux-musl ;; + aarch64) _codex_triple=aarch64-unknown-linux-musl ;; + *) echo "codingAgent codex: unsupported architecture: $_codex_arch" >&2; exit 1 ;; +esac +mkdir -p "$_install_dir" +curl -fsSL "https://github.com/openai/codex/releases/download/rust-v${_version}/codex-${_codex_triple}.tar.gz" | tar -xz -C "$_install_dir" +mv "$_install_dir/codex-${_codex_triple}" "$_install_dir/codex" +chmod +x "$_install_dir/codex" +"$_install_dir/codex" --version >/dev/null || { echo "codingAgent codex: install verification failed (cannot exec $_install_dir/codex)" >&2; exit 1; } +`, types.InstallDir, pinnedVersion) +} func (Agent) RunCommand(prompt, modelProviderURL string) string { return shellquote.Join( - "codex", + types.InstallDir+"/codex", "exec", "--json", "--sandbox", "danger-full-access", diff --git a/lib/batches/codingagent/codingagent.go b/lib/batches/codingagent/codingagent.go index 8256986d14..c36b391ff8 100644 --- a/lib/batches/codingagent/codingagent.go +++ b/lib/batches/codingagent/codingagent.go @@ -38,6 +38,7 @@ func RenderRunCommand(agentStep *batcheslib.CodingAgentStep, modelProviderURL st for _, binary := range a.ImageRequirements() { b.WriteString(failIfMissing(a.Type(), binary)) } + b.WriteString(a.InstallScript()) b.WriteString(a.RunCommand(renderedPrompt.String(), prefixed)) return b.String(), nil } @@ -47,8 +48,8 @@ func RenderRunCommand(agentStep *batcheslib.CodingAgentStep, modelProviderURL st // if binary isn't on PATH. func failIfMissing(agentType batcheslib.CodingAgentType, binary string) string { msg := fmt.Sprintf( - "codingAgent %q requires %q on PATH in the run container; ensure your image includes the %s binary", - agentType, binary, binary, + "codingAgent %q requires %q on PATH in the run container", + agentType, binary, ) return fmt.Sprintf("command -v %s >/dev/null 2>&1 || { echo %s >&2; exit 1; }\n", binary, diff --git a/lib/batches/codingagent/types/types.go b/lib/batches/codingagent/types/types.go index 8d11201551..050fdb88d4 100644 --- a/lib/batches/codingagent/types/types.go +++ b/lib/batches/codingagent/types/types.go @@ -14,6 +14,10 @@ const JobIDEnvVar = "SRC_BATCHES_JOB_ID" const JobIDHeaderName = "X-Sourcegraph-Job-ID" +// InstallDir is where each Agent installs its pinned binary; invoked by +// absolute path so we ignore any copy shipped by the image. +const InstallDir = "/tmp/sg-codingagent/bin" + type Agent interface { Type() batcheslib.CodingAgentType // RunCommand receives the already-templated prompt and MUST shell-quote it. @@ -21,9 +25,13 @@ type Agent interface { // ModelProviderRoutes returns routes whose WirePath is relative to the // agent type; the registry prefixes "/" before exposing them. ModelProviderRoutes() []ModelProviderRoute - // ImageRequirements returns binaries the agent expects on PATH in the run - // container. The registry emits a check before RunCommand. + // ImageRequirements lists binaries that must be on PATH in the run + // container (e.g. "curl" so InstallScript can fetch the agent). The + // registry emits a `command -v` check for each before InstallScript. ImageRequirements() []string + // InstallScript returns shell that installs the agent at a pinned + // version into a Sourcegraph-owned scratch dir and prepends it to PATH. + InstallScript() string } // ModelProviderRoute is one wire→upstream mapping served by the Sourcegraph diff --git a/lib/batches/schema/batch_spec_stringdata.go b/lib/batches/schema/batch_spec_stringdata.go index 31d59f1d74..5acf2223a5 100644 --- a/lib/batches/schema/batch_spec_stringdata.go +++ b/lib/batches/schema/batch_spec_stringdata.go @@ -11,6 +11,317 @@ const BatchSpecJSON = `{ "type": "object", "additionalProperties": false, "required": ["name"], + "allOf": [ + { + "if": { "properties": { "version": { "const": 3 } }, "required": ["version"] }, + "then": { + "properties": { + "steps": { + "items": { + "anyOf": [{ "required": ["run", "image"] }, { "required": ["codingAgent"] }] + } + } + } + }, + "else": { + "properties": { + "steps": { + "items": { + "required": ["run", "container"], + "not": { "required": ["codingAgent"] } + } + } + } + } + }, + { + "$comment": "The ` + "`" + `changesetHooks` + "`" + ` property is only allowed when ` + "`" + `version: 3` + "`" + ` is set.", + "if": { + "required": ["changesetHooks"] + }, + "then": { + "required": ["version"], + "properties": { + "version": { + "const": 3 + } + } + } + } + ], + "definitions": { + "OutputVariable": { + "title": "OutputVariable", + "type": "object", + "required": ["value"], + "properties": { + "value": { + "type": "string", + "description": "The value of the output, which can be a template string.", + "examples": ["hello world", "${{ step.stdout }}", "${{ repository.name }}"] + }, + "format": { + "type": "string", + "description": "The expected format of the output. If set, the output is being parsed in that format before being stored in the var. If not set, 'text' is assumed to the format.", + "enum": ["json", "yaml", "text"] + } + } + }, + "Mount": { + "title": "Mount", + "type": "object", + "additionalProperties": false, + "required": ["path", "mountpoint"], + "properties": { + "path": { + "type": "string", + "description": "The path on the local machine to mount. The path must be in the same directory or a subdirectory of the batch spec.", + "examples": ["local/path/to/file.text", "local/path/to/directory"] + }, + "mountpoint": { + "type": "string", + "description": "The path in the container to mount the path on the local machine to.", + "examples": ["path/to/file.txt", "path/to/directory"] + } + } + }, + "CodingAgent": { + "title": "CodingAgent", + "type": "object", + "description": "An out-of-the-box coding agent step that runs the given prompt inside a managed container. Mutually exclusive with run. Only supported in version 3 batch specs. Use the step's top-level container/image field to override the default agent image.", + "additionalProperties": false, + "required": ["type", "prompt"], + "properties": { + "type": { + "type": "string", + "description": "The coding agent to use.", + "enum": ["codex", "claude-code"] + }, + "prompt": { + "type": "string", + "description": "The prompt to send to the agent." + } + } + }, + "Step": { + "title": "Step", + "type": "object", + "description": "A command to run (as part of a sequence) in a repository branch to produce the required changes.", + "additionalProperties": false, + "properties": { + "run": { + "type": "string", + "description": "The shell command to run in the container. It can also be a multi-line shell script. The working directory is the root directory of the repository checkout." + }, + "container": { + "type": "string", + "description": "The Docker image used to launch the Docker container in which the shell command is run.", + "examples": ["alpine:3"] + }, + "image": { + "type": "string", + "description": "The Docker image used to launch the Docker container in which the shell command is run.", + "examples": ["alpine:3"] + }, + "codingAgent": { + "$ref": "#/definitions/CodingAgent" + }, + "outputs": { + "type": ["object", "null"], + "description": "Output variables of this step that can be referenced in the changesetTemplate or other steps via outputs.", + "additionalProperties": { + "$ref": "#/definitions/OutputVariable" + } + }, + "env": { + "description": "Environment variables to set in the step environment.", + "oneOf": [ + { + "type": "null" + }, + { + "type": "object", + "description": "Environment variables to set in the step environment.", + "additionalProperties": { + "type": "string" + } + }, + { + "type": "array", + "items": { + "oneOf": [ + { + "type": "string", + "description": "An environment variable to set in the step environment: the value will be passed through from the environment src is running within." + }, + { + "type": "object", + "description": "An environment variable to set in the step environment: the key is used as the environment variable name and the value as the value.", + "additionalProperties": { + "type": "string" + }, + "minProperties": 1, + "maxProperties": 1 + } + ] + } + } + ] + }, + "files": { + "type": ["object", "null"], + "description": "Files that should be mounted into or be created inside the Docker container.", + "additionalProperties": { + "type": "string" + } + }, + "if": { + "oneOf": [ + { + "type": "boolean" + }, + { + "type": "string" + }, + { + "type": "null" + } + ], + "description": "A condition to check before executing steps. Supports templating. The value 'true' is interpreted as true.", + "examples": [ + "true", + "${{ matches repository.name \"github.com/my-org/my-repo*\" }}", + "${{ outputs.goModFileExists }}", + "${{ eq previous_step.stdout \"success\" }}" + ] + }, + "mount": { + "description": "Files that are mounted to the Docker container.", + "type": ["array", "null"], + "items": { + "$ref": "#/definitions/Mount" + } + } + } + }, + "HookStep": { + "title": "HookStep", + "type": "object", + "description": "A command to run (as part of a sequence) inside a changeset hook action. Uses the version 3 step shape and supports additional hook-only fields such as maxAttempts.", + "additionalProperties": false, + "properties": { + "run": { + "type": "string", + "description": "The shell command to run in the container. It can also be a multi-line shell script. The working directory is the root directory of the repository checkout." + }, + "image": { + "type": "string", + "description": "The Docker image used to launch the Docker container in which the shell command is run.", + "examples": ["alpine:3"] + }, + "codingAgent": { + "$ref": "#/definitions/CodingAgent" + }, + "outputs": { + "type": ["object", "null"], + "description": "Output variables of this step that can be referenced in the changesetTemplate or other steps via outputs.", + "additionalProperties": { + "$ref": "#/definitions/OutputVariable" + } + }, + "env": { + "description": "Environment variables to set in the step environment.", + "oneOf": [ + { + "type": "null" + }, + { + "type": "object", + "description": "Environment variables to set in the step environment.", + "additionalProperties": { + "type": "string" + } + }, + { + "type": "array", + "items": { + "oneOf": [ + { + "type": "string", + "description": "An environment variable to set in the step environment: the value will be passed through from the environment src is running within." + }, + { + "type": "object", + "description": "An environment variable to set in the step environment: the key is used as the environment variable name and the value as the value.", + "additionalProperties": { + "type": "string" + }, + "minProperties": 1, + "maxProperties": 1 + } + ] + } + } + ] + }, + "files": { + "type": ["object", "null"], + "description": "Files that should be mounted into or be created inside the Docker container.", + "additionalProperties": { + "type": "string" + } + }, + "if": { + "oneOf": [ + { + "type": "boolean" + }, + { + "type": "string" + }, + { + "type": "null" + } + ], + "description": "A condition to check before executing steps. Supports templating. The value 'true' is interpreted as true.", + "examples": [ + "true", + "${{ matches repository.name \"github.com/my-org/my-repo*\" }}", + "${{ outputs.goModFileExists }}", + "${{ eq previous_step.stdout \"success\" }}" + ] + }, + "mount": { + "description": "Files that are mounted to the Docker container.", + "type": ["array", "null"], + "items": { + "$ref": "#/definitions/Mount" + } + }, + "maxAttempts": { + "type": "integer", + "description": "The maximum number of times this step will be attempted before the hook action is considered failed. Defaults to 1 (no retries).", + "minimum": 1 + } + } + }, + "ChangesetHookAction": { + "title": "ChangesetHookAction", + "type": "object", + "description": "A side-effect action to run at a changeset lifecycle event. Hook actions run in an executor.", + "additionalProperties": false, + "required": ["steps"], + "properties": { + "steps": { + "type": "array", + "description": "The sequence of steps to execute when the hook fires. Uses the version 3 step shape, with additional hook-only fields.", + "items": { + "$ref": "#/definitions/HookStep" + } + } + } + } + }, "properties": { "version": { "type": "number", @@ -28,20 +339,20 @@ const BatchSpecJSON = `{ }, "on": { "type": ["array", "null"], - "description": "The set of repositories (and branches) to run the batch change on, specified as a list of search queries (that match repositories) and/or specific repositories.", + "description": "The set of repositories to run the batch change on, specified as a list of search queries (that match repositories) and/or specific repositories. Repositories matched by repositoriesMatchingQuery use each repository's default branch unless overridden by an explicit repository entry with branch or branches.", "items": { "title": "OnQueryOrRepository", "oneOf": [ { "title": "OnQuery", "type": "object", - "description": "A Sourcegraph search query that matches a set of repositories (and branches). Each matched repository branch is added to the list of repositories that the batch change will be run on.", + "description": "A Sourcegraph search query that matches a set of repositories. Each matched repository is added to the list of repositories that the batch change will be run on using the repository's default branch.", "additionalProperties": false, "required": ["repositoriesMatchingQuery"], "properties": { "repositoriesMatchingQuery": { "type": "string", - "description": "A Sourcegraph search query that matches a set of repositories (and branches). If the query matches files, symbols, or some other object inside a repository, the object's repository is included.", + "description": "A Sourcegraph search query that matches a set of repositories. If the query matches files, symbols, or some other object inside a repository, the object's repository is included. Matched repositories are run on their default branch; use explicit repository entries with branch or branches to target non-default branches.", "examples": ["file:README.md"] } } @@ -124,154 +435,7 @@ const BatchSpecJSON = `{ "type": ["array", "null"], "description": "The sequence of commands to run (for each repository branch matched in the ` + "`" + `on` + "`" + ` property) to produce the workspace changes that will be included in the batch change.", "items": { - "title": "Step", - "type": "object", - "description": "A command to run (as part of a sequence) in a repository branch to produce the required changes.", - "additionalProperties": false, - "anyOf": [ - { "required": ["run", "container"] }, - { "required": ["run", "image"] }, - { "required": ["codingAgent", "container"] }, - { "required": ["codingAgent", "image"] } - ], - "properties": { - "run": { - "type": "string", - "description": "The shell command to run in the container. It can also be a multi-line shell script. The working directory is the root directory of the repository checkout." - }, - "container": { - "type": "string", - "description": "The Docker image used to launch the Docker container in which the shell command is run.", - "examples": ["alpine:3"] - }, - "image": { - "type": "string", - "description": "The Docker image used to launch the Docker container in which the shell command is run. Equivalent to 'container' in version 3 batch specs.", - "examples": ["alpine:3"] - }, - "codingAgent": { - "title": "CodingAgent", - "type": "object", - "description": "An out-of-the-box coding agent step that runs the given prompt inside a managed container. Mutually exclusive with run. Only supported in version 3 batch specs.", - "additionalProperties": false, - "required": ["type", "prompt"], - "properties": { - "type": { - "type": "string", - "description": "The coding agent to use.", - "enum": ["codex"] - }, - "prompt": { - "type": "string", - "description": "The prompt to send to the agent." - } - } - }, - "outputs": { - "type": ["object", "null"], - "description": "Output variables of this step that can be referenced in the changesetTemplate or other steps via outputs.", - "additionalProperties": { - "title": "OutputVariable", - "type": "object", - "required": ["value"], - "properties": { - "value": { - "type": "string", - "description": "The value of the output, which can be a template string.", - "examples": ["hello world", "${{ step.stdout }}", "${{ repository.name }}"] - }, - "format": { - "type": "string", - "description": "The expected format of the output. If set, the output is being parsed in that format before being stored in the var. If not set, 'text' is assumed to the format.", - "enum": ["json", "yaml", "text"] - } - } - } - }, - "env": { - "description": "Environment variables to set in the step environment.", - "oneOf": [ - { - "type": "null" - }, - { - "type": "object", - "description": "Environment variables to set in the step environment.", - "additionalProperties": { - "type": "string" - } - }, - { - "type": "array", - "items": { - "oneOf": [ - { - "type": "string", - "description": "An environment variable to set in the step environment: the value will be passed through from the environment src is running within." - }, - { - "type": "object", - "description": "An environment variable to set in the step environment: the key is used as the environment variable name and the value as the value.", - "additionalProperties": { - "type": "string" - }, - "minProperties": 1, - "maxProperties": 1 - } - ] - } - } - ] - }, - "files": { - "type": ["object", "null"], - "description": "Files that should be mounted into or be created inside the Docker container.", - "additionalProperties": { - "type": "string" - } - }, - "if": { - "oneOf": [ - { - "type": "boolean" - }, - { - "type": "string" - }, - { - "type": "null" - } - ], - "description": "A condition to check before executing steps. Supports templating. The value 'true' is interpreted as true.", - "examples": [ - "true", - "${{ matches repository.name \"github.com/my-org/my-repo*\" }}", - "${{ outputs.goModFileExists }}", - "${{ eq previous_step.stdout \"success\" }}" - ] - }, - "mount": { - "description": "Files that are mounted to the Docker container.", - "type": ["array", "null"], - "items": { - "type": "object", - "additionalProperties": false, - "required": ["path", "mountpoint"], - "properties": { - "path": { - "type": "string", - "description": "The path on the local machine to mount. The path must be in the same directory or a subdirectory of the batch spec.", - "examples": ["local/path/to/file.text", "local/path/to/directory"] - }, - "mountpoint": { - "type": "string", - "description": "The path in the container to mount the path on the local machine to.", - "examples": ["path/to/file.txt", "path/to/directory"] - } - } - } - } - } + "$ref": "#/definitions/Step" } }, "transformChanges": { @@ -434,6 +598,21 @@ const BatchSpecJSON = `{ ] } } + }, + "changesetHooks": { + "type": "object", + "description": "Side-effect actions to run at well-defined changeset lifecycle events. Only allowed when ` + "`" + `version: 3` + "`" + ` is set.", + "additionalProperties": false, + "properties": { + "onCIFailure": { + "description": "Action to run when the changeset's combined check state transitions into ` + "`" + `failed` + "`" + ` for a given head SHA.", + "$ref": "#/definitions/ChangesetHookAction" + }, + "onMergeConflict": { + "description": "Action to run when the changeset's external mergeability transitions into ` + "`" + `conflicting` + "`" + ` for a given (base, head) SHA pair.", + "$ref": "#/definitions/ChangesetHookAction" + } + } } } } From 7003f47b64af987b61d58d69c69264d48c5e2d2c Mon Sep 17 00:00:00 2001 From: Taras Yemets Date: Thu, 28 May 2026 14:01:45 +0300 Subject: [PATCH 03/11] feat/batch-changes: harden codingAgent v1 port Three real follow-ups on top of the initial v1 port, all surfaced while porting feedback from sourcegraph/sourcegraph#12503: - Stop emitting SRC_BATCHES_MODEL_PROVIDER_TOKEN in src-cli logs. The forwarded token is now redacted in StepStarted UI metadata (JSON-lines) and in the 'full command' debug log, but still passed verbatim to the docker -e flags so the agent CLI inside the container can use it. Server-side RedactedValues stays as a backstop. - Reject v3 codingAgent steps that omit image: at parse time, instead of failing later in the executor with an empty-image error. - Harden the codex install script: extract into a mktemp dir and mv into the install path so a failed/retried install can't leave a half-written binary behind, and assert that the installed binary's --version contains pinnedVersion (catches a stale binary surviving from a prior failed install). Also adds tests for the new redaction helpers, Step.MarshalJSON canonicalization, and the new codingAgent-requires-image validation. --- internal/batches/executor/run_steps.go | 44 +++++++++++++++- internal/batches/executor/run_steps_test.go | 46 ++++++++++++++++ lib/batches/batch_spec.go | 4 ++ lib/batches/batch_spec_test.go | 58 +++++++++++++++++++++ lib/batches/codex/codex.go | 30 +++++++---- 5 files changed, 170 insertions(+), 12 deletions(-) diff --git a/internal/batches/executor/run_steps.go b/internal/batches/executor/run_steps.go index eb22e153db..12f041d70d 100644 --- a/internal/batches/executor/run_steps.go +++ b/internal/batches/executor/run_steps.go @@ -338,7 +338,7 @@ func executeSingleStep( // ---------- // EXECUTION // ---------- - opts.UI.StepStarted(stepIdx+1, runScript, env) + opts.UI.StepStarted(stepIdx+1, runScript, redactSensitiveEnv(env)) workspaceOpts, err := workspace.DockerRunOpts(ctx, workDir) if err != nil { @@ -424,7 +424,7 @@ func executeSingleStep( } opts.Logger.Logf("[Step %d] run: %q, container: %q", stepIdx+1, step.Run, step.Container) - opts.Logger.Logf("[Step %d] full command: %q", stepIdx+1, strings.Join(cmd.Args, " ")) + opts.Logger.Logf("[Step %d] full command: %q", stepIdx+1, strings.Join(redactSensitiveArgs(cmd.Args), " ")) // Start the command. t0 := time.Now() @@ -619,6 +619,46 @@ func forwardCodingAgentEnv(globalEnv []string, stepEnv map[string]string) { } } +// sensitiveEnvKeys names env vars that get passed verbatim into the user +// container but must be scrubbed from UI sinks and log lines. +var sensitiveEnvKeys = map[string]struct{}{ + codingagenttypes.ModelProviderTokenEnvVar: {}, +} + +const redactedPlaceholder = "REDACTED" + +func redactSensitiveEnv(env map[string]string) map[string]string { + out := make(map[string]string, len(env)) + for k, v := range env { + if _, sensitive := sensitiveEnvKeys[k]; sensitive && v != "" { + out[k] = redactedPlaceholder + } else { + out[k] = v + } + } + return out +} + +// redactSensitiveArgs scrubs the value side of `-e KEY=VALUE` pairs whose +// KEY is sensitive, returning a copy of args suitable for logging. +func redactSensitiveArgs(args []string) []string { + out := make([]string, len(args)) + copy(out, args) + for i := 0; i+1 < len(out); i++ { + if out[i] != "-e" { + continue + } + key, _, ok := strings.Cut(out[i+1], "=") + if !ok { + continue + } + if _, sensitive := sensitiveEnvKeys[key]; sensitive { + out[i+1] = key + "=" + redactedPlaceholder + } + } + return out +} + // writeRunScriptFile writes a pre-rendered run script (e.g. a codingAgent // step desugared by the codingagent registry) verbatim to a temp file, with // the same permission semantics as createRunScriptFile. Unlike that helper, diff --git a/internal/batches/executor/run_steps_test.go b/internal/batches/executor/run_steps_test.go index 23ec6bfc3d..72d4714858 100644 --- a/internal/batches/executor/run_steps_test.go +++ b/internal/batches/executor/run_steps_test.go @@ -1,11 +1,57 @@ package executor import ( + "slices" + "strings" "testing" codingagenttypes "github.com/sourcegraph/sourcegraph/lib/batches/codingagent/types" ) +func TestRedactSensitiveEnv(t *testing.T) { + in := map[string]string{ + codingagenttypes.ModelProviderTokenEnvVar: "tok-abc", + codingagenttypes.JobIDEnvVar: "job-123", + "PATH": "/bin", + } + out := redactSensitiveEnv(in) + if got := out[codingagenttypes.ModelProviderTokenEnvVar]; got != redactedPlaceholder { + t.Errorf("token: got %q want %q", got, redactedPlaceholder) + } + if got := out[codingagenttypes.JobIDEnvVar]; got != "job-123" { + t.Errorf("job id should not be redacted: got %q", got) + } + if got := out["PATH"]; got != "/bin" { + t.Errorf("PATH should not be redacted: got %q", got) + } + if in[codingagenttypes.ModelProviderTokenEnvVar] != "tok-abc" { + t.Errorf("input must not be mutated") + } +} + +func TestRedactSensitiveArgs(t *testing.T) { + in := []string{ + "docker", "run", + "-e", codingagenttypes.ModelProviderTokenEnvVar + "=tok-abc", + "-e", codingagenttypes.JobIDEnvVar + "=job-123", + "-e", "PATH=/bin", + "--", "image:tag", "/script", + } + out := redactSensitiveArgs(in) + if slices.Contains(out, codingagenttypes.ModelProviderTokenEnvVar+"=tok-abc") { + t.Errorf("token value still present in args: %v", out) + } + if !slices.Contains(out, codingagenttypes.ModelProviderTokenEnvVar+"="+redactedPlaceholder) { + t.Errorf("token not redacted in args: %v", out) + } + if !slices.Contains(out, codingagenttypes.JobIDEnvVar+"=job-123") { + t.Errorf("job id should pass through: %v", out) + } + if strings.Contains(strings.Join(out, " "), "tok-abc") { + t.Errorf("token leaked anywhere in joined args: %v", out) + } +} + // TestForwardCodingAgentEnv verifies that the model-provider auth env vars // placed on the v1 CliStep by the Sourcegraph server are forwarded into the // user container env for codingAgent steps. diff --git a/lib/batches/batch_spec.go b/lib/batches/batch_spec.go index 83b5d9bb6f..99c7d76a9d 100644 --- a/lib/batches/batch_spec.go +++ b/lib/batches/batch_spec.go @@ -226,6 +226,10 @@ func parseBatchSpec(schema string, data []byte) (*BatchSpec, error) { if step.CodingAgent != nil && step.Run != "" { errs = errors.Append(errs, NewValidationError(errors.Newf("step %d: codingAgent and run cannot be combined in the same step", i+1))) } + if step.CodingAgent != nil && step.Container == "" { + // v3 image: is mirrored into Container above. + errs = errors.Append(errs, NewValidationError(errors.Newf("step %d: codingAgent step requires an image", i+1))) + } } return &spec, errs diff --git a/lib/batches/batch_spec_test.go b/lib/batches/batch_spec_test.go index 8cd0686bf0..adb512dc3b 100644 --- a/lib/batches/batch_spec_test.go +++ b/lib/batches/batch_spec_test.go @@ -1,9 +1,37 @@ package batches import ( + "encoding/json" + "strings" "testing" ) +// Step JSON must always emit `container` (never `image`) so prep-side cache +// keys computed by marshaling Step match src-cli's serialization. +func TestStep_MarshalJSON_canonicalizesImageToContainer(t *testing.T) { + v3FromImage, err := json.Marshal(Step{Image: "alpine:3", Run: "echo hi"}) + if err != nil { + t.Fatalf("marshal v3-shaped step: %v", err) + } + v1FromContainer, err := json.Marshal(Step{Container: "alpine:3", Run: "echo hi"}) + if err != nil { + t.Fatalf("marshal v1-shaped step: %v", err) + } + if string(v3FromImage) != string(v1FromContainer) { + t.Errorf("canonical JSON differs:\n v3 image: %s\n v1 container: %s", v3FromImage, v1FromContainer) + } + var out map[string]any + if err := json.Unmarshal(v3FromImage, &out); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if _, ok := out["image"]; ok { + t.Errorf("expected no image key in canonical JSON, got %s", v3FromImage) + } + if got, _ := out["container"].(string); got != "alpine:3" { + t.Errorf("container: got %v want alpine:3 (full=%s)", out["container"], v3FromImage) + } +} + // TestParseBatchSpec_v3_imageMirroredToContainer ensures that in v3 specs the // step-level `image:` field is mirrored into Step.Container so executor // consumers that read step.Container keep working. @@ -39,6 +67,36 @@ changesetTemplate: } } +// A v3 codingAgent step without image: must be rejected at parse time so +// it doesn't fail later with an opaque empty-image error in the executor. +func TestParseBatchSpec_v3_codingAgentRequiresImage(t *testing.T) { + spec := []byte(` +version: 3 +name: test +description: test +on: + - repository: github.com/sourcegraph/sourcegraph +steps: + - codingAgent: + type: codex + prompt: do the thing +changesetTemplate: + title: test + body: test + branch: test + commit: + message: test +`) + _, err := ParseBatchSpec(spec) + if err == nil { + t.Fatal("expected validation error, got nil") + } + if !strings.Contains(err.Error(), "requires an image") && + !strings.Contains(err.Error(), "Must validate") { + t.Errorf("error should mention missing image, got: %v", err) + } +} + // TestParseBatchSpec_v3_codingAgentStep ensures that a v3 spec with a // codingAgent step parses with the expected typed step. func TestParseBatchSpec_v3_codingAgentStep(t *testing.T) { diff --git a/lib/batches/codex/codex.go b/lib/batches/codex/codex.go index a20fc4ecf4..6b9ba1739a 100644 --- a/lib/batches/codex/codex.go +++ b/lib/batches/codex/codex.go @@ -26,21 +26,31 @@ func (Agent) Type() batcheslib.CodingAgentType { return batchesli func (Agent) ModelProviderRoutes() []types.ModelProviderRoute { return routes } func (Agent) ImageRequirements() []string { return []string{"curl", "tar"} } -// InstallScript installs codex from GitHub Releases into types.InstallDir. +// InstallScript installs codex at pinnedVersion into types.InstallDir. func (Agent) InstallScript() string { return fmt.Sprintf(`_install_dir=%s _version=%s -_codex_arch=$(uname -m) -case "$_codex_arch" in - x86_64) _codex_triple=x86_64-unknown-linux-musl ;; - aarch64) _codex_triple=aarch64-unknown-linux-musl ;; - *) echo "codingAgent codex: unsupported architecture: $_codex_arch" >&2; exit 1 ;; + +case "$(uname -m)" in + x86_64) _triple=x86_64-unknown-linux-musl ;; + aarch64) _triple=aarch64-unknown-linux-musl ;; + *) echo "codingAgent codex: unsupported architecture $(uname -m)" >&2; exit 1 ;; esac + +# Stage in a temp dir and mv into place so a failed retry can't leave a half-written binary behind. +_url="https://github.com/openai/codex/releases/download/rust-v${_version}/codex-${_triple}.tar.gz" +_tmp=$(mktemp -d "${TMPDIR:-/tmp}/sg-codex.XXXXXX") mkdir -p "$_install_dir" -curl -fsSL "https://github.com/openai/codex/releases/download/rust-v${_version}/codex-${_codex_triple}.tar.gz" | tar -xz -C "$_install_dir" -mv "$_install_dir/codex-${_codex_triple}" "$_install_dir/codex" -chmod +x "$_install_dir/codex" -"$_install_dir/codex" --version >/dev/null || { echo "codingAgent codex: install verification failed (cannot exec $_install_dir/codex)" >&2; exit 1; } +curl -fsSL "$_url" | tar -xz -C "$_tmp" || { echo "codingAgent codex: download/extract failed: $_url" >&2; exit 1; } +chmod +x "$_tmp/codex-${_triple}" +mv -f "$_tmp/codex-${_triple}" "$_install_dir/codex" +rm -rf "$_tmp" + +_actual=$("$_install_dir/codex" --version 2>&1) || { echo "codingAgent codex: cannot exec $_install_dir/codex: $_actual" >&2; exit 1; } +case "$_actual" in + *"$_version"*) ;; + *) echo "codingAgent codex: version mismatch: want $_version, got: $_actual" >&2; exit 1 ;; +esac `, types.InstallDir, pinnedVersion) } From 6a48767d13ab891c64120dfc7961bea802929912 Mon Sep 17 00:00:00 2001 From: Taras Yemets Date: Thu, 28 May 2026 15:02:01 +0300 Subject: [PATCH 04/11] feat/batch-changes: trim redundant comments in codingAgent port Drop test godocs that just paraphrase the test name, an inline block that duplicates a helper's own doc, and a single-line note restating a well-known Go idiom. --- internal/batches/executor/run_steps.go | 4 ---- internal/batches/executor/run_steps_test.go | 3 --- lib/batches/batch_spec.go | 1 - lib/batches/batch_spec_test.go | 9 --------- lib/batches/codingagent/codingagent_test.go | 4 ---- 5 files changed, 21 deletions(-) diff --git a/internal/batches/executor/run_steps.go b/internal/batches/executor/run_steps.go index 12f041d70d..2970e1a94c 100644 --- a/internal/batches/executor/run_steps.go +++ b/internal/batches/executor/run_steps.go @@ -325,10 +325,6 @@ func executeSingleStep( return bytes.Buffer{}, bytes.Buffer{}, err } - // For codingAgent steps, forward the model-provider auth env from the - // executor's environment (injected via CliStep.Env and JobTokenEnvVar on - // the server) into the user container so the agent CLI can talk to the - // /model-provider/batches proxy. if step.CodingAgent != nil { forwardCodingAgentEnv(opts.GlobalEnv, env) } diff --git a/internal/batches/executor/run_steps_test.go b/internal/batches/executor/run_steps_test.go index 72d4714858..89c1a1a0d2 100644 --- a/internal/batches/executor/run_steps_test.go +++ b/internal/batches/executor/run_steps_test.go @@ -52,9 +52,6 @@ func TestRedactSensitiveArgs(t *testing.T) { } } -// TestForwardCodingAgentEnv verifies that the model-provider auth env vars -// placed on the v1 CliStep by the Sourcegraph server are forwarded into the -// user container env for codingAgent steps. func TestForwardCodingAgentEnv(t *testing.T) { cases := []struct { name string diff --git a/lib/batches/batch_spec.go b/lib/batches/batch_spec.go index 99c7d76a9d..4c5858c188 100644 --- a/lib/batches/batch_spec.go +++ b/lib/batches/batch_spec.go @@ -121,7 +121,6 @@ type CodingAgentStep struct { // (which round-trips through src-cli) would not, producing divergent keys // and silent cache misses for any v3 spec. func (s Step) MarshalJSON() ([]byte, error) { - // Use an alias type to avoid infinite recursion through MarshalJSON. type stepAlias Step canon := stepAlias(s) if canon.Container == "" { diff --git a/lib/batches/batch_spec_test.go b/lib/batches/batch_spec_test.go index adb512dc3b..0399e746f8 100644 --- a/lib/batches/batch_spec_test.go +++ b/lib/batches/batch_spec_test.go @@ -6,8 +6,6 @@ import ( "testing" ) -// Step JSON must always emit `container` (never `image`) so prep-side cache -// keys computed by marshaling Step match src-cli's serialization. func TestStep_MarshalJSON_canonicalizesImageToContainer(t *testing.T) { v3FromImage, err := json.Marshal(Step{Image: "alpine:3", Run: "echo hi"}) if err != nil { @@ -32,9 +30,6 @@ func TestStep_MarshalJSON_canonicalizesImageToContainer(t *testing.T) { } } -// TestParseBatchSpec_v3_imageMirroredToContainer ensures that in v3 specs the -// step-level `image:` field is mirrored into Step.Container so executor -// consumers that read step.Container keep working. func TestParseBatchSpec_v3_imageMirroredToContainer(t *testing.T) { spec := []byte(` version: 3 @@ -67,8 +62,6 @@ changesetTemplate: } } -// A v3 codingAgent step without image: must be rejected at parse time so -// it doesn't fail later with an opaque empty-image error in the executor. func TestParseBatchSpec_v3_codingAgentRequiresImage(t *testing.T) { spec := []byte(` version: 3 @@ -97,8 +90,6 @@ changesetTemplate: } } -// TestParseBatchSpec_v3_codingAgentStep ensures that a v3 spec with a -// codingAgent step parses with the expected typed step. func TestParseBatchSpec_v3_codingAgentStep(t *testing.T) { spec := []byte(` version: 3 diff --git a/lib/batches/codingagent/codingagent_test.go b/lib/batches/codingagent/codingagent_test.go index dcf50e634e..7535fe4b48 100644 --- a/lib/batches/codingagent/codingagent_test.go +++ b/lib/batches/codingagent/codingagent_test.go @@ -22,10 +22,6 @@ func TestRenderRunCommand_unknownType(t *testing.T) { } } -// TestRenderRunCommand_promptShellQuoting ensures the rendered prompt is -// shell-quoted as a single argument even when it contains shell -// metacharacters (apostrophes, semicolons), so prompt text can't break out -// into the host shell. func TestRenderRunCommand_promptShellQuoting(t *testing.T) { const repoName = `github.com/sourcegraph/sourcegraph` prompt := "You're working in the ${{ repository.name }} repository.\n" + From c108b0153c0ff87272c473a0327fc99fb96381e4 Mon Sep 17 00:00:00 2001 From: Taras Yemets Date: Thu, 28 May 2026 15:13:31 +0300 Subject: [PATCH 05/11] batches: drop ModelProviderRoutes from codingagent Agent contract Sourcegraph now owns the wire-path/upstream-path mapping per agent type; src-cli only needs to construct its base URL as / and let the agent CLI append its protocol's wire path (codex CLI is hardcoded to POST /responses since wire_api=responses is now the only supported value, so the explicit -c model_providers.sourcegraph.wire_api line is also dropped). --- lib/batches/codex/codex.go | 22 ++++-------- lib/batches/codingagent/codingagent.go | 30 ++--------------- lib/batches/codingagent/types/types.go | 46 +++++++------------------- 3 files changed, 22 insertions(+), 76 deletions(-) diff --git a/lib/batches/codex/codex.go b/lib/batches/codex/codex.go index 6b9ba1739a..56155dbd9c 100644 --- a/lib/batches/codex/codex.go +++ b/lib/batches/codex/codex.go @@ -1,4 +1,4 @@ -// Package codex implements the codex coding agent run-command rewrite. +// Package codex implements the codex coding agent. package codex import ( @@ -10,23 +10,16 @@ import ( "github.com/sourcegraph/sourcegraph/lib/batches/codingagent/types" ) -const model = "gpt-5.4" - -// pinnedVersion is the codex CLI release we test against. Bump in lockstep -// with the model_providers.* config keys below; codex CLI is pre-v1. -const pinnedVersion = "0.134.0" - -var routes = []types.ModelProviderRoute{ - {WirePath: "/responses", UpstreamPath: "/v1/completions/openai-responses"}, -} +const ( + model = "gpt-5.4" + pinnedVersion = "0.134.0" +) type Agent struct{} -func (Agent) Type() batcheslib.CodingAgentType { return batcheslib.CodingAgentTypeCodex } -func (Agent) ModelProviderRoutes() []types.ModelProviderRoute { return routes } -func (Agent) ImageRequirements() []string { return []string{"curl", "tar"} } +func (Agent) Type() batcheslib.CodingAgentType { return batcheslib.CodingAgentTypeCodex } +func (Agent) ImageRequirements() []string { return []string{"curl", "tar"} } -// InstallScript installs codex at pinnedVersion into types.InstallDir. func (Agent) InstallScript() string { return fmt.Sprintf(`_install_dir=%s _version=%s @@ -69,7 +62,6 @@ func (Agent) RunCommand(prompt, modelProviderURL string) string { "-c", fmt.Sprintf(`model_providers.sourcegraph.base_url=%q`, modelProviderURL), "-c", fmt.Sprintf(`model_providers.sourcegraph.env_key=%q`, types.ModelProviderTokenEnvVar), "-c", fmt.Sprintf(`model_providers.sourcegraph.env_http_headers={%q=%q}`, types.JobIDHeaderName, types.JobIDEnvVar), - "-c", `model_providers.sourcegraph.wire_api="responses"`, prompt, ) } diff --git a/lib/batches/codingagent/codingagent.go b/lib/batches/codingagent/codingagent.go index c36b391ff8..4f3efd6aad 100644 --- a/lib/batches/codingagent/codingagent.go +++ b/lib/batches/codingagent/codingagent.go @@ -1,5 +1,5 @@ -// Package codingagent rewrites v3 batch spec coding-agent steps into the -// shell commands that drive them. Register new agents in the agents list. +// Package codingagent rewrites v3 batch spec coding-agent steps into shell +// commands. Register new agents in the agents list. package codingagent import ( @@ -16,13 +16,9 @@ import ( "github.com/sourcegraph/sourcegraph/lib/errors" ) -// ErrUnknownType is returned when a codingAgent step references a type that -// has no registered Agent. var ErrUnknownType = errors.New("unknown codingAgent type") -// RenderRunCommand returns the shell-quoted command that runs agentStep. The -// prompt is rendered before quoting; reversing would let templated values -// break out of the shell quoting. +// RenderRunCommand returns the shell command that runs agentStep. func RenderRunCommand(agentStep *batcheslib.CodingAgentStep, modelProviderURL string, stepCtx *template.StepContext) (string, error) { a, ok := agents[agentStep.Type] if !ok { @@ -43,9 +39,6 @@ func RenderRunCommand(agentStep *batcheslib.CodingAgentStep, modelProviderURL st return b.String(), nil } -// failIfMissing returns a shell snippet that, when prepended to the step -// script, writes a message to stderr and exits the container with status 1 -// if binary isn't on PATH. func failIfMissing(agentType batcheslib.CodingAgentType, binary string) string { msg := fmt.Sprintf( "codingAgent %q requires %q on PATH in the run container", @@ -69,20 +62,3 @@ var agents = func() map[batcheslib.CodingAgentType]types.Agent { } return out }() - -// RegisteredModelProviderRoutes returns the model-provider proxy routes -// contributed by every registered Agent, each WirePath prefixed with -// its agent type. -func RegisteredModelProviderRoutes() []types.ModelProviderRoute { - var out []types.ModelProviderRoute - for _, a := range agents { - prefix := "/" + string(a.Type()) - for _, route := range a.ModelProviderRoutes() { - out = append(out, types.ModelProviderRoute{ - WirePath: prefix + route.WirePath, - UpstreamPath: route.UpstreamPath, - }) - } - } - return out -} diff --git a/lib/batches/codingagent/types/types.go b/lib/batches/codingagent/types/types.go index 050fdb88d4..ffaf36c06f 100644 --- a/lib/batches/codingagent/types/types.go +++ b/lib/batches/codingagent/types/types.go @@ -1,48 +1,26 @@ -// Package types holds the contract shared between the codingagent registry -// and individual coding-agent implementations, in a leaf package to avoid an -// import cycle with the registry. +// Package types holds the codingagent contract shared with individual +// coding-agent implementations; split out to avoid an import cycle. package types import ( batcheslib "github.com/sourcegraph/sourcegraph/lib/batches" ) -const ModelProviderTokenEnvVar = "SRC_BATCHES_MODEL_PROVIDER_TOKEN" - -// JobIDEnvVar binds ModelProviderTokenEnvVar to its job; sent as JobIDHeaderName. -const JobIDEnvVar = "SRC_BATCHES_JOB_ID" - -const JobIDHeaderName = "X-Sourcegraph-Job-ID" - -// InstallDir is where each Agent installs its pinned binary; invoked by -// absolute path so we ignore any copy shipped by the image. -const InstallDir = "/tmp/sg-codingagent/bin" +const ( + ModelProviderTokenEnvVar = "SRC_BATCHES_MODEL_PROVIDER_TOKEN" + JobIDEnvVar = "SRC_BATCHES_JOB_ID" + JobIDHeaderName = "X-Sourcegraph-Job-ID" + InstallDir = "/tmp/sg-codingagent/bin" +) type Agent interface { Type() batcheslib.CodingAgentType - // RunCommand receives the already-templated prompt and MUST shell-quote it. + // RunCommand returns the shell command for the agent. The rendered + // prompt MUST be shell-quoted. RunCommand(renderedPrompt, modelProviderURL string) string - // ModelProviderRoutes returns routes whose WirePath is relative to the - // agent type; the registry prefixes "/" before exposing them. - ModelProviderRoutes() []ModelProviderRoute // ImageRequirements lists binaries that must be on PATH in the run - // container (e.g. "curl" so InstallScript can fetch the agent). The - // registry emits a `command -v` check for each before InstallScript. + // container before InstallScript runs. ImageRequirements() []string - // InstallScript returns shell that installs the agent at a pinned - // version into a Sourcegraph-owned scratch dir and prepends it to PATH. + // InstallScript installs the agent at a pinned version into InstallDir. InstallScript() string } - -// ModelProviderRoute is one wire→upstream mapping served by the Sourcegraph -// model-provider proxy (mounted under /.executors/model-provider/batches). -type ModelProviderRoute struct { - // WirePath is the proxy subpath the agent CLI POSTs to, relative to the - // agent type (e.g. "/responses"). The registry prefixes "/" - // before registering it on the router (e.g. "/codex/responses"). - WirePath string - // UpstreamPath is the path on the Sourcegraph Model Provider upstream - // (e.g. Cody Gateway) that serves WirePath, such as - // "/v1/completions/openai-responses". - UpstreamPath string -} From 0e9cd7de609c2f22b1c567d8d36fd1702abe1a67 Mon Sep 17 00:00:00 2001 From: Taras Yemets Date: Thu, 28 May 2026 15:23:14 +0300 Subject: [PATCH 06/11] feat/batch-changes: polish error message and tests in codingAgent port - Replace internal-type-leaking error message with user-facing wording in run_steps.go (codingAgent ModelProviderURL check). - Align temp-file error wording in writeRunScriptFile with the rest of the file ('writing to temporary file'). - Trim redundant assertions in TestRedactSensitiveEnv and TestRedactSensitiveArgs. - Tighten TestParseBatchSpec_v3_codingAgentRequiresImage to assert only the actual error path. - Extract v3SpecWith helper to deduplicate YAML scaffold across batch_spec_test.go cases. --- internal/batches/executor/run_steps.go | 4 +- internal/batches/executor/run_steps_test.go | 8 -- lib/batches/batch_spec_test.go | 81 +++++++-------------- 3 files changed, 27 insertions(+), 66 deletions(-) diff --git a/internal/batches/executor/run_steps.go b/internal/batches/executor/run_steps.go index 2970e1a94c..6b4721d271 100644 --- a/internal/batches/executor/run_steps.go +++ b/internal/batches/executor/run_steps.go @@ -281,7 +281,7 @@ func executeSingleStep( ) if step.CodingAgent != nil { if opts.Task.ModelProviderURL == "" { - err = errors.New("codingAgent step requires WorkspacesExecutionInput.ModelProviderURL to be set") + err = errors.New("codingAgent step requires a model-provider URL") opts.UI.StepPreparingFailed(stepIdx+1, err) return bytes.Buffer{}, bytes.Buffer{}, err } @@ -670,7 +670,7 @@ func writeRunScriptFile(tempDir, script string) (string, func(), error) { if _, err := runScriptFile.WriteString(script); err != nil { cleanup() - return "", nil, errors.Wrap(err, "writing temporary file") + return "", nil, errors.Wrap(err, "writing to temporary file") } if err := runScriptFile.Close(); err != nil { cleanup() diff --git a/internal/batches/executor/run_steps_test.go b/internal/batches/executor/run_steps_test.go index 89c1a1a0d2..112c3e9258 100644 --- a/internal/batches/executor/run_steps_test.go +++ b/internal/batches/executor/run_steps_test.go @@ -2,7 +2,6 @@ package executor import ( "slices" - "strings" "testing" codingagenttypes "github.com/sourcegraph/sourcegraph/lib/batches/codingagent/types" @@ -11,16 +10,12 @@ import ( func TestRedactSensitiveEnv(t *testing.T) { in := map[string]string{ codingagenttypes.ModelProviderTokenEnvVar: "tok-abc", - codingagenttypes.JobIDEnvVar: "job-123", "PATH": "/bin", } out := redactSensitiveEnv(in) if got := out[codingagenttypes.ModelProviderTokenEnvVar]; got != redactedPlaceholder { t.Errorf("token: got %q want %q", got, redactedPlaceholder) } - if got := out[codingagenttypes.JobIDEnvVar]; got != "job-123" { - t.Errorf("job id should not be redacted: got %q", got) - } if got := out["PATH"]; got != "/bin" { t.Errorf("PATH should not be redacted: got %q", got) } @@ -47,9 +42,6 @@ func TestRedactSensitiveArgs(t *testing.T) { if !slices.Contains(out, codingagenttypes.JobIDEnvVar+"=job-123") { t.Errorf("job id should pass through: %v", out) } - if strings.Contains(strings.Join(out, " "), "tok-abc") { - t.Errorf("token leaked anywhere in joined args: %v", out) - } } func TestForwardCodingAgentEnv(t *testing.T) { diff --git a/lib/batches/batch_spec_test.go b/lib/batches/batch_spec_test.go index 0399e746f8..500ba0aadc 100644 --- a/lib/batches/batch_spec_test.go +++ b/lib/batches/batch_spec_test.go @@ -2,10 +2,31 @@ package batches import ( "encoding/json" + "fmt" "strings" "testing" ) +// v3SpecWith wraps stepsYAML in the minimum scaffold needed for ParseBatchSpec +// to accept a v3 spec. +func v3SpecWith(stepsYAML string) []byte { + return []byte(fmt.Sprintf(` +version: 3 +name: test +description: test +on: + - repository: github.com/sourcegraph/sourcegraph +steps: +%s +changesetTemplate: + title: test + body: test + branch: test + commit: + message: test +`, stepsYAML)) +} + func TestStep_MarshalJSON_canonicalizesImageToContainer(t *testing.T) { v3FromImage, err := json.Marshal(Step{Image: "alpine:3", Run: "echo hi"}) if err != nil { @@ -31,23 +52,7 @@ func TestStep_MarshalJSON_canonicalizesImageToContainer(t *testing.T) { } func TestParseBatchSpec_v3_imageMirroredToContainer(t *testing.T) { - spec := []byte(` -version: 3 -name: test -description: test -on: - - repository: github.com/sourcegraph/sourcegraph -steps: - - run: echo hi - image: alpine:3 -changesetTemplate: - title: test - body: test - branch: test - commit: - message: test -`) - got, err := ParseBatchSpec(spec) + got, err := ParseBatchSpec(v3SpecWith(" - run: echo hi\n image: alpine:3")) if err != nil { t.Fatalf("ParseBatchSpec failed: %v", err) } @@ -63,53 +68,17 @@ changesetTemplate: } func TestParseBatchSpec_v3_codingAgentRequiresImage(t *testing.T) { - spec := []byte(` -version: 3 -name: test -description: test -on: - - repository: github.com/sourcegraph/sourcegraph -steps: - - codingAgent: - type: codex - prompt: do the thing -changesetTemplate: - title: test - body: test - branch: test - commit: - message: test -`) - _, err := ParseBatchSpec(spec) + _, err := ParseBatchSpec(v3SpecWith(" - codingAgent:\n type: codex\n prompt: do the thing")) if err == nil { t.Fatal("expected validation error, got nil") } - if !strings.Contains(err.Error(), "requires an image") && - !strings.Contains(err.Error(), "Must validate") { + if !strings.Contains(err.Error(), "requires an image") { t.Errorf("error should mention missing image, got: %v", err) } } func TestParseBatchSpec_v3_codingAgentStep(t *testing.T) { - spec := []byte(` -version: 3 -name: test -description: test -on: - - repository: github.com/sourcegraph/sourcegraph -steps: - - codingAgent: - type: codex - prompt: do the thing - image: alpine:3 -changesetTemplate: - title: test - body: test - branch: test - commit: - message: test -`) - got, err := ParseBatchSpec(spec) + got, err := ParseBatchSpec(v3SpecWith(" - codingAgent:\n type: codex\n prompt: do the thing\n image: alpine:3")) if err != nil { t.Fatalf("ParseBatchSpec failed: %v", err) } From 32b1b7a8cc0cbbe0321ee13fce89e7b0dad2573e Mon Sep 17 00:00:00 2001 From: Taras Yemets Date: Thu, 28 May 2026 15:34:40 +0300 Subject: [PATCH 07/11] batches: flag codingagent wire constants as cross-repo mirrored Adds a one-line note above the env-var / header constants pointing at the sibling block in sourcegraph/sourcegraph, since the two repos hold the contract independently. Splits InstallDir into its own const since it's src-cli-local and not part of the cross-repo contract. --- lib/batches/codingagent/types/types.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/batches/codingagent/types/types.go b/lib/batches/codingagent/types/types.go index ffaf36c06f..7be4be11ff 100644 --- a/lib/batches/codingagent/types/types.go +++ b/lib/batches/codingagent/types/types.go @@ -6,13 +6,16 @@ import ( batcheslib "github.com/sourcegraph/sourcegraph/lib/batches" ) +// Mirrored in sourcegraph/sourcegraph lib/batches/codingagent/codingagent.go; keep in sync. const ( ModelProviderTokenEnvVar = "SRC_BATCHES_MODEL_PROVIDER_TOKEN" JobIDEnvVar = "SRC_BATCHES_JOB_ID" JobIDHeaderName = "X-Sourcegraph-Job-ID" - InstallDir = "/tmp/sg-codingagent/bin" ) +// InstallDir is src-cli-local; not part of the cross-repo contract. +const InstallDir = "/tmp/sg-codingagent/bin" + type Agent interface { Type() batcheslib.CodingAgentType // RunCommand returns the shell command for the agent. The rendered From 02c89330e5b75c8c3ddf5c32ff058ea9e8cf0eeb Mon Sep 17 00:00:00 2001 From: Taras Yemets Date: Thu, 28 May 2026 15:51:20 +0300 Subject: [PATCH 08/11] feat/batch-changes: fix goimports in codingAgent env tests --- internal/batches/executor/run_steps_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/batches/executor/run_steps_test.go b/internal/batches/executor/run_steps_test.go index 112c3e9258..7022ba1b87 100644 --- a/internal/batches/executor/run_steps_test.go +++ b/internal/batches/executor/run_steps_test.go @@ -10,7 +10,7 @@ import ( func TestRedactSensitiveEnv(t *testing.T) { in := map[string]string{ codingagenttypes.ModelProviderTokenEnvVar: "tok-abc", - "PATH": "/bin", + "PATH": "/bin", } out := redactSensitiveEnv(in) if got := out[codingagenttypes.ModelProviderTokenEnvVar]; got != redactedPlaceholder { From 8a10375a0160c9f0f93a7224775e716b320f8d62 Mon Sep 17 00:00:00 2001 From: Taras Yemets Date: Thu, 28 May 2026 16:07:30 +0300 Subject: [PATCH 09/11] feat/batch-changes: drop unneeded v3 cache-key canonicalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Step.MarshalJSON canonicalization and v3 image→container parse-time mirroring are not required for server-side codingAgent execution: the server canonicalizes container on the wire, so src-cli receives container-only JSON and reads step.Container directly. These changes hardened src-cli's local handling of v3 specs (preview / validate / apply paths) but were already broken pre-PR; restoring that is out of scope here. Drop them to keep this PR focused on the codingAgent feature. --- lib/batches/batch_spec.go | 31 +---------------------- lib/batches/batch_spec_test.go | 45 ++-------------------------------- 2 files changed, 3 insertions(+), 73 deletions(-) diff --git a/lib/batches/batch_spec.go b/lib/batches/batch_spec.go index 4c5858c188..6fd6cde355 100644 --- a/lib/batches/batch_spec.go +++ b/lib/batches/batch_spec.go @@ -1,7 +1,6 @@ package batches import ( - "encoding/json" "fmt" "strings" @@ -113,23 +112,6 @@ type CodingAgentStep struct { Prompt string `json:"prompt,omitempty" yaml:"prompt"` } -// MarshalJSON canonicalizes the v3 `image:` field into `container:` on the -// wire. Both fields exist on Step for ergonomic reasons (v3 specs use -// `image:`, v1/v2 specs use `container:`), but src-cli's executor reads -// `Container`. Without canonicalization, the prep-side cache key — computed -// by JSON-marshaling Step — would include `image` while the executor side -// (which round-trips through src-cli) would not, producing divergent keys -// and silent cache misses for any v3 spec. -func (s Step) MarshalJSON() ([]byte, error) { - type stepAlias Step - canon := stepAlias(s) - if canon.Container == "" { - canon.Container = canon.Image - } - canon.Image = "" - return json.Marshal(canon) -} - func (s *Step) IfCondition() string { switch v := s.If.(type) { case bool: @@ -192,16 +174,6 @@ func parseBatchSpec(schema string, data []byte) (*BatchSpec, error) { return nil, err } - if spec.Version == 3 { - // Mirror v3 `image:` into `container:` so in-memory consumers that - // read step.Container (e.g. the executor transform) keep working. - // JSON serialization is canonicalized separately in Step.MarshalJSON - // so prep-side cache hashing matches src-cli/executor serialization. - for i := range spec.Steps { - spec.Steps[i].Container = spec.Steps[i].Image - } - } - var errs error if len(spec.Steps) != 0 && spec.ChangesetTemplate == nil { @@ -225,8 +197,7 @@ func parseBatchSpec(schema string, data []byte) (*BatchSpec, error) { if step.CodingAgent != nil && step.Run != "" { errs = errors.Append(errs, NewValidationError(errors.Newf("step %d: codingAgent and run cannot be combined in the same step", i+1))) } - if step.CodingAgent != nil && step.Container == "" { - // v3 image: is mirrored into Container above. + if step.CodingAgent != nil && step.Container == "" && step.Image == "" { errs = errors.Append(errs, NewValidationError(errors.Newf("step %d: codingAgent step requires an image", i+1))) } } diff --git a/lib/batches/batch_spec_test.go b/lib/batches/batch_spec_test.go index 500ba0aadc..55faea2e3c 100644 --- a/lib/batches/batch_spec_test.go +++ b/lib/batches/batch_spec_test.go @@ -1,7 +1,6 @@ package batches import ( - "encoding/json" "fmt" "strings" "testing" @@ -27,46 +26,6 @@ changesetTemplate: `, stepsYAML)) } -func TestStep_MarshalJSON_canonicalizesImageToContainer(t *testing.T) { - v3FromImage, err := json.Marshal(Step{Image: "alpine:3", Run: "echo hi"}) - if err != nil { - t.Fatalf("marshal v3-shaped step: %v", err) - } - v1FromContainer, err := json.Marshal(Step{Container: "alpine:3", Run: "echo hi"}) - if err != nil { - t.Fatalf("marshal v1-shaped step: %v", err) - } - if string(v3FromImage) != string(v1FromContainer) { - t.Errorf("canonical JSON differs:\n v3 image: %s\n v1 container: %s", v3FromImage, v1FromContainer) - } - var out map[string]any - if err := json.Unmarshal(v3FromImage, &out); err != nil { - t.Fatalf("unmarshal: %v", err) - } - if _, ok := out["image"]; ok { - t.Errorf("expected no image key in canonical JSON, got %s", v3FromImage) - } - if got, _ := out["container"].(string); got != "alpine:3" { - t.Errorf("container: got %v want alpine:3 (full=%s)", out["container"], v3FromImage) - } -} - -func TestParseBatchSpec_v3_imageMirroredToContainer(t *testing.T) { - got, err := ParseBatchSpec(v3SpecWith(" - run: echo hi\n image: alpine:3")) - if err != nil { - t.Fatalf("ParseBatchSpec failed: %v", err) - } - if len(got.Steps) != 1 { - t.Fatalf("expected 1 step, got %d", len(got.Steps)) - } - if got.Steps[0].Image != "alpine:3" { - t.Errorf("Step.Image: got %q want %q", got.Steps[0].Image, "alpine:3") - } - if got.Steps[0].Container != "alpine:3" { - t.Errorf("Step.Container (mirrored from image): got %q want %q", got.Steps[0].Container, "alpine:3") - } -} - func TestParseBatchSpec_v3_codingAgentRequiresImage(t *testing.T) { _, err := ParseBatchSpec(v3SpecWith(" - codingAgent:\n type: codex\n prompt: do the thing")) if err == nil { @@ -95,7 +54,7 @@ func TestParseBatchSpec_v3_codingAgentStep(t *testing.T) { if step.CodingAgent.Prompt != "do the thing" { t.Errorf("CodingAgent.Prompt: got %q want %q", step.CodingAgent.Prompt, "do the thing") } - if step.Container != "alpine:3" { - t.Errorf("Step.Container (mirrored from image): got %q want %q", step.Container, "alpine:3") + if step.Image != "alpine:3" { + t.Errorf("Step.Image: got %q want %q", step.Image, "alpine:3") } } From 18a223fa7af0f6b2cc54148b87084b57317ea477 Mon Sep 17 00:00:00 2001 From: Taras Yemets Date: Thu, 28 May 2026 16:14:04 +0300 Subject: [PATCH 10/11] feat/batch-changes: fix codingAgent prompt-quoting test The previous assertion round-tripped the rendered shell script through shellquote.Split and compared the last token to the expected prompt. That tokenizer does not understand POSIX shell comments, so the apostrophe in the install script's "can't" comment was read as an unterminated single-quoted string and swallowed everything to EOF as one token. Production /bin/sh handles the comment correctly; only the test was broken. Assert on the shell-quoted suffix instead. --- lib/batches/codingagent/codingagent_test.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/batches/codingagent/codingagent_test.go b/lib/batches/codingagent/codingagent_test.go index 7535fe4b48..5fc1e75723 100644 --- a/lib/batches/codingagent/codingagent_test.go +++ b/lib/batches/codingagent/codingagent_test.go @@ -2,6 +2,7 @@ package codingagent_test import ( "errors" + "strings" "testing" "github.com/kballard/go-shellquote" @@ -36,13 +37,20 @@ func TestRenderRunCommand_promptShellQuoting(t *testing.T) { t.Fatal(err) } - tokens, err := shellquote.Split(cmd) - if err != nil { - t.Fatal(err) - } + // The rendered command appends the shell-quoted prompt as its final + // argument. Round-tripping via shellquote.Split is unreliable here + // because the install script contains POSIX shell comments with + // apostrophes (e.g. "can't"), which shellquote does not understand + // and would treat as unterminated quoted strings. Assert on the + // suffix instead. wantPrompt := "You're working in the " + repoName + " repository.\n" + "Add a README section describing the project; don't touch existing files." - if got := tokens[len(tokens)-1]; got != wantPrompt { - t.Fatalf("prompt mismatch:\n got: %q\n want: %q", got, wantPrompt) + wantQuoted := shellquote.Join(wantPrompt) + if !strings.HasSuffix(cmd, wantQuoted) { + tail := cmd + if n := len(wantQuoted) + 50; len(cmd) > n { + tail = cmd[len(cmd)-n:] + } + t.Fatalf("rendered cmd does not end with shell-quoted prompt:\n want suffix: %q\n cmd tail: %q", wantQuoted, tail) } } From 1f6c80b361a75fc0ba71bb4e7e460e31c78655ab Mon Sep 17 00:00:00 2001 From: Taras Yemets Date: Thu, 28 May 2026 16:35:30 +0300 Subject: [PATCH 11/11] feat/batch-changes: shorten codingAgent helper doc comments --- internal/batches/executor/run_steps.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/internal/batches/executor/run_steps.go b/internal/batches/executor/run_steps.go index 6b4721d271..076e269e8b 100644 --- a/internal/batches/executor/run_steps.go +++ b/internal/batches/executor/run_steps.go @@ -600,10 +600,8 @@ func createRunScriptFile(ctx context.Context, tempDir string, stepRun string, st } // forwardCodingAgentEnv copies the model-provider auth env vars -// (SRC_BATCHES_MODEL_PROVIDER_TOKEN, SRC_BATCHES_JOB_ID) from globalEnv into -// stepEnv. These are placed on the v1 CliStep that runs `src batch exec` by -// the Sourcegraph server; the agent CLI in the user container needs them to -// reach the /.executors/model-provider/batches proxy. +// (SRC_BATCHES_MODEL_PROVIDER_TOKEN, SRC_BATCHES_JOB_ID) from globalEnv +// into stepEnv so they reach the user container. func forwardCodingAgentEnv(globalEnv []string, stepEnv map[string]string) { for _, key := range []string{codingagenttypes.ModelProviderTokenEnvVar, codingagenttypes.JobIDEnvVar} { for _, e := range globalEnv { @@ -655,12 +653,10 @@ func redactSensitiveArgs(args []string) []string { return out } -// writeRunScriptFile writes a pre-rendered run script (e.g. a codingAgent -// step desugared by the codingagent registry) verbatim to a temp file, with -// the same permission semantics as createRunScriptFile. Unlike that helper, -// this one does NOT pass the content through template.RenderStepTemplate: -// the script is treated as opaque shell text so embedded sequences like -// `{{` inside a shell-quoted prompt are not re-parsed as templates. +// writeRunScriptFile writes a pre-rendered run script verbatim to a temp +// file. Unlike createRunScriptFile it does NOT pass the content through +// template.RenderStepTemplate, so embedded `{{` sequences in a +// shell-quoted prompt are not re-parsed as templates. func writeRunScriptFile(tempDir, script string) (string, func(), error) { runScriptFile, err := os.CreateTemp(tempDir, "") if err != nil {