Merge branch 'main' into bahrmichael/dynamic-containers

Author: bahrmichael

lib/README.md

@@ -5,8 +5,8 @@ This directory contains vendored packages from `github.com/sourcegraph/sourcegra
 ## Source
 
 - **Repository**: https://github.com/sourcegraph/sourcegraph
-- **Commit**: 2ee2b8e77de9663b08ce5f6e5a2c7d2217ce721a
-- **Date**: 2025-11-17 19:49:42 -0800
+- **Commit**: bdc2f4bb8b59f78f4fa8868b2690b673b41948d4
+- **Date**: 2026-06-01 07:34:50 +0100
 
 ## Updating
 

lib/batches/batch_spec.go

@@ -1,6 +1,7 @@
 package batches
 
 import (
+	"encoding/json"
 	"fmt"
 	"strings"
 
@@ -38,8 +39,31 @@ type BatchSpec struct {
 	TransformChanges  *TransformChanges        `json:"transformChanges,omitempty" yaml:"transformChanges,omitempty"`
 	ImportChangesets  []ImportChangeset        `json:"importChangesets,omitempty" yaml:"importChangesets"`
 	ChangesetTemplate *ChangesetTemplate       `json:"changesetTemplate,omitempty" yaml:"changesetTemplate"`
+	ChangesetHooks    *ChangesetHooks          `json:"changesetHooks,omitempty" yaml:"hooks,omitempty"`
 }
 
+// Hooks declares side-effect actions to run at well-defined changeset
+// lifecycle events. Only allowed when Version is 3.
+type ChangesetHooks struct {
+	OnCIFailure     ChangesetHookAction `json:"onCIFailure,omitempty" yaml:"onCIFailure,omitempty"`
+	OnMergeConflict ChangesetHookAction `json:"onMergeConflict,omitempty" yaml:"onMergeConflict,omitempty"`
+}
+
+// HookAction is a single action attached to a changeset lifecycle event.
+//
+// Hook actions reuse the Step shape from the top-level steps block.
+type ChangesetHookAction struct {
+	Steps []Step `json:"steps,omitempty" yaml:"steps,omitempty"`
+}
+
+type changesetHookEvent string
+
+// Hook event names. Kept here so callers don't pass typoed strings.
+const (
+	ChangesetHookEventOnCIFailure     changesetHookEvent = "onCIFailure"
+	ChangesetHookEventOnMergeConflict changesetHookEvent = "onMergeConflict"
+)
+
 type ChangesetTemplate struct {
 	Title     string                       `json:"title,omitempty" yaml:"title"`
 	Body      string                       `json:"body,omitempty" yaml:"body"`
@@ -90,13 +114,46 @@ 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"`
+	BuildImage  *BuildImageStep   `json:"buildImage,omitempty" yaml:"buildImage,omitempty"`
+	Container   string            `json:"container,omitempty" yaml:"container"`
+	Image       string            `json:"image,omitempty" yaml:"image"`
+	MaxAttempts int               `json:"maxAttempts,omitempty" yaml:"maxAttempts,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"`
+}
+
+type CodingAgentStep struct {
+	Type   string `json:"type,omitempty" yaml:"type"`
+	Prompt string `json:"prompt,omitempty" yaml:"prompt"`
+}
+
+type BuildImageStep struct {
+	Run       string `json:"run" yaml:"run"`
+	BaseImage string `json:"baseImage" yaml:"baseImage"`
+}
+
+// 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 Step has only
+// `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. See the regression test in
+// lib/batches/execution/cache.
+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 {
@@ -161,33 +218,97 @@ func parseBatchSpec(schema string, data []byte) (*BatchSpec, error) {
 		return nil, err
 	}
 
-	var errs error
+	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 {
 		errs = errors.Append(errs, NewValidationError(errors.New("batch spec includes steps but no changesetTemplate")))
 	}
 
+	// v3 specs do not support changesetTemplate.published — publication is
+	// driven exclusively via the batchchangeagent tools. Reject the field at
+	// parse time.
+	if spec.Version == 3 && spec.ChangesetTemplate != nil && spec.ChangesetTemplate.Published != nil {
+		errs = errors.Append(errs, NewValidationError(errors.New("changesetTemplate.published is not supported in batch spec version 3; drive publication via the publish_changesets tool instead")))
+	}
+
 	for i, step := range spec.Steps {
 		for _, mount := range step.Mount {
-			if strings.ContainsAny(mount.Path, invalidMountCharacters) {
+			if strings.Contains(mount.Path, invalidMountCharacters) {
 				errs = errors.Append(errs, NewValidationError(errors.Newf("step %d mount path contains invalid characters", i+1)))
 			}
-			if strings.ContainsAny(mount.Mountpoint, invalidMountCharacters) {
+			if strings.Contains(mount.Mountpoint, invalidMountCharacters) {
 				errs = errors.Append(errs, NewValidationError(errors.Newf("step %d mount mountpoint 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)))
+		}
+		if step.BuildImage != nil && step.Run != "" {
+			errs = errors.Append(errs, NewValidationError(errors.Newf("step %d: buildImage and run cannot be combined in the same step", i+1)))
+		}
 		for name := range step.Files {
-			if strings.ContainsAny(name, invalidMountCharacters) {
+			if strings.Contains(name, invalidMountCharacters) {
 				errs = errors.Append(errs, NewValidationError(errors.Newf("step %d files target path contains invalid characters", i+1)))
 			}
 		}
 	}
 
+	if hookErr := validateHooks(&spec); hookErr != nil {
+		errs = errors.Append(errs, hookErr)
+	}
+
 	return &spec, errs
 }
 
-// docker uses Golang's `encoding/csv` library to parse arguments passed to `--mount`
-const invalidMountCharacters = ",\"\n\r"
+// validateHooks performs Go-level validation of spec.Hooks beyond what the
+// JSON schema enforces. The schema already gates `hooks:` on `version: 3` and
+// rejects unknown event names. We re-check the version invariant here so
+// non-schema callers (and any future schema drift) still fail safely, and we
+// run the per-step mount-character validator that the schema cannot express.
+func validateHooks(spec *BatchSpec) error {
+	if spec.ChangesetHooks == nil {
+		return nil
+	}
+
+	var errs error
+
+	if spec.Version != 3 {
+		errs = errors.Append(errs, NewValidationError(errors.New("batch spec hooks require version: 3")))
+	}
+
+	validate := func(event changesetHookEvent, action ChangesetHookAction) {
+		for i, step := range action.Steps {
+			for _, mount := range step.Mount {
+				if strings.Contains(mount.Path, invalidMountCharacters) {
+					errs = errors.Append(errs, NewValidationError(errors.Newf(
+						"hooks.%s step %d mount path contains invalid characters", event, i+1,
+					)))
+				}
+				if strings.Contains(mount.Mountpoint, invalidMountCharacters) {
+					errs = errors.Append(errs, NewValidationError(errors.Newf(
+						"hooks.%s step %d mount mountpoint contains invalid characters", event, i+1,
+					)))
+				}
+			}
+		}
+	}
+
+	validate(ChangesetHookEventOnCIFailure, spec.ChangesetHooks.OnCIFailure)
+	validate(ChangesetHookEventOnMergeConflict, spec.ChangesetHooks.OnMergeConflict)
+
+	return errs
+}
+
+const invalidMountCharacters = ","
 
 func (on *OnQueryOrRepository) String() string {
 	if on.RepositoriesMatchingQuery != "" {
@@ -212,10 +333,6 @@ func (e BatchSpecValidationError) Error() string {
 	return e.err.Error()
 }
 
-func IsValidationError(err error) bool {
-	return errors.HasType[*BatchSpecValidationError](err)
-}
-
 // SkippedStepsForRepo calculates the steps required to run on the given repo.
 func SkippedStepsForRepo(spec *BatchSpec, repoName string, fileMatches []string) (skipped map[int]struct{}, err error) {
 	skipped = map[int]struct{}{}

lib/batches/changeset_spec.go

@@ -203,12 +203,6 @@ func (d *ChangesetSpec) IsImportingExisting() bool {
 	return d.Type() == ChangesetSpecDescriptionTypeExisting
 }
 
-// IsBranch returns whether the description is of type
-// ChangesetSpecDescriptionTypeBranch.
-func (d *ChangesetSpec) IsBranch() bool {
-	return d.Type() == ChangesetSpecDescriptionTypeBranch
-}
-
 // ChangesetSpecDescriptionType tells the consumer what the type of a
 // ChangesetSpecDescription is without having to look into the description.
 // Useful in the GraphQL when a HiddenChangesetSpec is returned.

lib/batches/overridable/bool.go

@@ -7,13 +7,6 @@ type Bool struct {
 	rules rules
 }
 
-// FromBool creates a Bool representing a static, scalar value.
-func FromBool(b bool) Bool {
-	return Bool{
-		rules: rules{simpleRule(b)},
-	}
-}
-
 // Value returns the bool value for the given repository.
 func (b *Bool) Value(name string) bool {
 	v := b.rules.Match(name)

lib/batches/template/partial_eval.go

@@ -89,7 +89,6 @@ func parseAndPartialEval(input string, ctx *StepContext) (*template.Template, er
 		Funcs(builtins).
 		Funcs(ctx.ToFuncMap()).
 		Parse(input)
-
 	if err != nil {
 		return nil, err
 	}
@@ -349,7 +348,7 @@ func isTrue(val reflect.Value) (truth bool) {
 		return val.Bool()
 	case reflect.Complex64, reflect.Complex128:
 		return val.Complex() != 0
-	case reflect.Chan, reflect.Func, reflect.Ptr, reflect.Interface:
+	case reflect.Chan, reflect.Func, reflect.Pointer, reflect.Interface:
 		return !val.IsNil()
 	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
 		return val.Int() != 0

lib/batches/template/template.go

@@ -1,6 +1,9 @@
 package template
 
-import "text/template"
+import (
+	"strings"
+	"text/template"
+)
 
 func New(name, tmpl, option string, ctxs ...template.FuncMap) (*template.Template, error) {
 	t := template.New(name).Delims(startDelim, endDelim)
@@ -16,3 +19,7 @@ func New(name, tmpl, option string, ctxs ...template.FuncMap) (*template.Templat
 
 	return t.Parse(tmpl)
 }
+
+func ContainsTemplateAction(tmpl string) bool {
+	return strings.Contains(tmpl, startDelim)
+}

lib/batches/template/templating.go

@@ -16,8 +16,10 @@ import (
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 )
 
-const startDelim = "${{"
-const endDelim = "}}"
+const (
+	startDelim = "${{"
+	endDelim   = "}}"
+)
 
 var builtins = template.FuncMap{
 	"join":    strings.Join,
@@ -74,7 +76,6 @@ func ValidateBatchSpecTemplate(spec string) (bool, error) {
 	// option "missingkey=error". See https://pkg.go.dev/text/template#Template.Option for
 	// more.
 	t, err := New("validateBatchSpecTemplate", spec, "missingkey=error", sfm, cstfm)
-
 	if err != nil {
 		// Attempt to extract the specific template variable field that caused the error
 		// to provide a clearer message.

lib/codeintel/upload/indexer_name.go

@@ -20,9 +20,6 @@ import (
 // is 10639 characters long.
 const MaxBufferSize = 128 * 1024
 
-// ErrMetadataExceedsBuffer occurs when the first line of an LSIF index is too long to read.
-var ErrMetadataExceedsBuffer = errors.New("metaData vertex exceeds buffer")
-
 // ErrInvalidMetaDataVertex occurs when the first line of an LSIF index is not a valid metadata vertex.
 var ErrInvalidMetaDataVertex = errors.New("invalid metaData vertex")
 

lib/codeintel/upload/request.go

@@ -30,6 +30,9 @@ type uploadRequestOptions struct {
 // ErrUnauthorized occurs when the upload endpoint returns a 401 response.
 var ErrUnauthorized = errors.New("unauthorized upload")
 
+// ErrForbidden occurs when the upload endpoint returns a 403 response.
+var ErrForbidden = errors.New("insufficient permissions")
+
 // performUploadRequest performs an HTTP POST to the upload endpoint. The query string of the request
 // is constructed from the given request options and the body of the request is the unmodified reader.
 // If target is a non-nil pointer, it will be assigned the value of the upload identifier present
@@ -105,17 +108,17 @@ func performRequest(ctx context.Context, req *http.Request, httpClient Client, l
 // returns a boolean flag indicating if the function can be retried on failure (error-dependent).
 func decodeUploadPayload(resp *http.Response, body []byte, target *int) (bool, error) {
 	if resp.StatusCode >= 300 {
+		detail := extractBodyDetail(body)
+
 		if resp.StatusCode == http.StatusUnauthorized {
-			return false, ErrUnauthorized
+			return false, errors.Wrap(ErrUnauthorized, detail)
 		}
-
-		suffix := ""
-		if !bytes.HasPrefix(bytes.TrimSpace(body), []byte{'<'}) {
-			suffix = fmt.Sprintf(" (%s)", bytes.TrimSpace(body))
+		if resp.StatusCode == http.StatusForbidden {
+			return false, errors.Wrap(ErrForbidden, detail)
 		}
 
 		// Do not retry client errors
-		return resp.StatusCode >= 500, errors.Errorf("unexpected status code: %d%s", resp.StatusCode, suffix)
+		return resp.StatusCode >= 500, errors.Errorf("unexpected status code: %d: %s", resp.StatusCode, detail)
 	}
 
 	if target == nil {
@@ -199,6 +202,16 @@ func makeUploadURL(opts uploadRequestOptions) (*url.URL, error) {
 	return parsedUrl, nil
 }
 
+// extractBodyDetail returns the response body as a suffix string for error messages.
+// Returns an empty string if the body is empty or appears to be HTML.
+func extractBodyDetail(body []byte) string {
+	trimmed := bytes.TrimSpace(body)
+	if len(trimmed) == 0 || bytes.HasPrefix(trimmed, []byte{'<'}) {
+		return ""
+	}
+	return string(trimmed)
+}
+
 func formatInt(v int) string {
 	return strconv.FormatInt(int64(v), 10)
 }

lib/codeintel/upload/upload.go

@@ -218,8 +218,6 @@ func uploadMultipartIndexParts(ctx context.Context, httpClient Client, opts Uplo
 	}
 
 	for i, reader := range readers {
-		i, reader := i, reader
-
 		pool.Go(func(ctx context.Context) error {
 			// Determine size of this reader. If we're not the last reader in the slice,
 			// then we're the maximum payload size. Otherwise, we're whatever is left.