diff --git a/api/foreman/v1alpha1/agentictask_types.go b/api/foreman/v1alpha1/agentictask_types.go index 38cd5d48..c5f9dd7f 100644 --- a/api/foreman/v1alpha1/agentictask_types.go +++ b/api/foreman/v1alpha1/agentictask_types.go @@ -24,7 +24,7 @@ import ( // AgenticTaskKind is the unit of work the task performs. Each kind has a // payload shape, scheduler routing, and lifecycle. -// +kubebuilder:validation:Enum=issue-fix;verify;review;freeform +// +kubebuilder:validation:Enum=issue-fix;verify;review;freeform;integrate;reconcile type AgenticTaskKind string const ( @@ -47,6 +47,19 @@ const ( AgenticTaskKindReview AgenticTaskKind = "review" // AgenticTaskKindFreeform passes an arbitrary prompt to a named agent. AgenticTaskKindFreeform AgenticTaskKind = "freeform" + // AgenticTaskKindIntegrate unions the disjoint slice branches of a sliced + // Workload onto the current base on a fresh integration branch, gated by a + // build. Scheduled after all slice issue-fix tasks Succeed; its payload + // carries the slice branches (payload.slices[].branch) and the base + // (payload.baseBranch). Part of Sliced Workloads (#1033). + AgenticTaskKindIntegrate AgenticTaskKind = "integrate" + // AgenticTaskKindReconcile checks the integrated union against the slice + // plan's pinned shared identifiers, catching cross-slice interface drift a + // build cannot see. A pinned-missing drift is authoritative (GATE-FAIL); an + // llm-flagged-only drift is advisory. Its payload carries the pins + // (payload.sharedIdentifiers), the slices' files (payload.slices[].files), + // and the contract (payload.contract). Part of Sliced Workloads (#1033). + AgenticTaskKindReconcile AgenticTaskKind = "reconcile" ) // BranchStrategy controls how the executor cuts an issue-fix task's working @@ -351,6 +364,63 @@ type AgenticTaskPayload struct { // produced no advisories or has not yet completed. // +optional GateAdvisories []GateAdvisory `json:"gateAdvisories,omitempty"` + + // Slices are the disjoint slices of a sliced Workload. The integrate task + // unions their branches; the reconcile task checks pins against their + // files. Set on integrate and reconcile tasks. Part of #1033. + // +optional + Slices []SliceRef `json:"slices,omitempty"` + + // Contract is the slice plan's prose contract, handed to a reconcile + // task's LLM sweep as the description of the intended shared interfaces. + // Reconcile only. Part of #1033. + // +optional + Contract string `json:"contract,omitempty"` + + // SharedIdentifiers pins the exact cross-slice strings a reconcile task + // verifies against the integrated union. Reconcile only. Part of #1033. + // +optional + SharedIdentifiers []SharedIdentifier `json:"sharedIdentifiers,omitempty"` +} + +// SliceRef describes one disjoint slice of a sliced Workload. The integrate +// task unions each slice's Branch; the reconcile task verifies pinned +// identifiers appear in each slice's Files. Part of Sliced Workloads (#1033). +type SliceRef struct { + // Name is the slice's name within the plan. + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + Name string `json:"name"` + + // Branch is the git ref the slice's coder pushed. Required for the + // integrate task, which applies each slice branch's diff onto the base. + // +optional + Branch string `json:"branch,omitempty"` + + // Files are the repo-relative paths this slice owns, so a reconcile task + // looks for each pinned identifier in the right slice. + // +optional + Files []string `json:"files,omitempty"` +} + +// SharedIdentifier pins one exact string that crosses a slice boundary (a +// metric name, config key, or CRD field). The reconcile task asserts the ID +// appears verbatim in the DefinedBy slice and every ReferencedBy slice; a +// missing pin is an authoritative drift. Part of Sliced Workloads (#1033). +type SharedIdentifier struct { + // ID is the exact string every listed slice must contain verbatim. + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + ID string `json:"id"` + + // DefinedBy is the slice name that must produce the identifier. + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + DefinedBy string `json:"definedBy"` + + // ReferencedBy are the slice names that must consume the identifier. + // +optional + ReferencedBy []string `json:"referencedBy,omitempty"` } // AgenticTaskSpec defines the desired state of an AgenticTask. diff --git a/api/foreman/v1alpha1/zz_generated.deepcopy.go b/api/foreman/v1alpha1/zz_generated.deepcopy.go index 611f73db..4c68d59a 100644 --- a/api/foreman/v1alpha1/zz_generated.deepcopy.go +++ b/api/foreman/v1alpha1/zz_generated.deepcopy.go @@ -393,6 +393,20 @@ func (in *AgenticTaskPayload) DeepCopyInto(out *AgenticTaskPayload) { *out = make([]GateAdvisory, len(*in)) copy(*out, *in) } + if in.Slices != nil { + in, out := &in.Slices, &out.Slices + *out = make([]SliceRef, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.SharedIdentifiers != nil { + in, out := &in.SharedIdentifiers, &out.SharedIdentifiers + *out = make([]SharedIdentifier, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AgenticTaskPayload. @@ -968,6 +982,46 @@ func (in *ResolvedGate) DeepCopy() *ResolvedGate { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SharedIdentifier) DeepCopyInto(out *SharedIdentifier) { + *out = *in + if in.ReferencedBy != nil { + in, out := &in.ReferencedBy, &out.ReferencedBy + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SharedIdentifier. +func (in *SharedIdentifier) DeepCopy() *SharedIdentifier { + if in == nil { + return nil + } + out := new(SharedIdentifier) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SliceRef) DeepCopyInto(out *SliceRef) { + *out = *in + if in.Files != nil { + in, out := &in.Files, &out.Files + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SliceRef. +func (in *SliceRef) DeepCopy() *SliceRef { + if in == nil { + return nil + } + out := new(SliceRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *StuckLoopDetectionSpec) DeepCopyInto(out *StuckLoopDetectionSpec) { *out = *in diff --git a/charts/foreman/templates/crds/agentictasks.yaml b/charts/foreman/templates/crds/agentictasks.yaml index 1b15875b..1bc02088 100644 --- a/charts/foreman/templates/crds/agentictasks.yaml +++ b/charts/foreman/templates/crds/agentictasks.yaml @@ -147,6 +147,8 @@ spec: - verify - review - freeform + - integrate + - reconcile type: string mcpEnabled: description: |- @@ -216,6 +218,12 @@ spec: - reset - rebase type: string + contract: + description: |- + Contract is the slice plan's prose contract, handed to a reconcile + task's LLM sweep as the description of the intended shared interfaces. + Reconcile only. Part of #1033. + type: string gateAdvisories: description: |- GateAdvisories holds non-blocking findings copied from the upstream @@ -286,6 +294,69 @@ spec: executor logs and falls back to branching from BaseBranch rather than failing. type: string + sharedIdentifiers: + description: |- + SharedIdentifiers pins the exact cross-slice strings a reconcile task + verifies against the integrated union. Reconcile only. Part of #1033. + items: + description: |- + SharedIdentifier pins one exact string that crosses a slice boundary (a + metric name, config key, or CRD field). The reconcile task asserts the ID + appears verbatim in the DefinedBy slice and every ReferencedBy slice; a + missing pin is an authoritative drift. Part of Sliced Workloads (#1033). + properties: + definedBy: + description: DefinedBy is the slice name that must produce + the identifier. + minLength: 1 + type: string + id: + description: ID is the exact string every listed slice must + contain verbatim. + minLength: 1 + type: string + referencedBy: + description: ReferencedBy are the slice names that must + consume the identifier. + items: + type: string + type: array + required: + - definedBy + - id + type: object + type: array + slices: + description: |- + Slices are the disjoint slices of a sliced Workload. The integrate task + unions their branches; the reconcile task checks pins against their + files. Set on integrate and reconcile tasks. Part of #1033. + items: + description: |- + SliceRef describes one disjoint slice of a sliced Workload. The integrate + task unions each slice's Branch; the reconcile task verifies pinned + identifiers appear in each slice's Files. Part of Sliced Workloads (#1033). + properties: + branch: + description: |- + Branch is the git ref the slice's coder pushed. Required for the + integrate task, which applies each slice branch's diff onto the base. + type: string + files: + description: |- + Files are the repo-relative paths this slice owns, so a reconcile task + looks for each pinned identifier in the right slice. + items: + type: string + type: array + name: + description: Name is the slice's name within the plan. + minLength: 1 + type: string + required: + - name + type: object + type: array type: object priority: description: |- diff --git a/charts/foreman/templates/crds/workloads.yaml b/charts/foreman/templates/crds/workloads.yaml index ebbda790..ad22c0b8 100644 --- a/charts/foreman/templates/crds/workloads.yaml +++ b/charts/foreman/templates/crds/workloads.yaml @@ -421,6 +421,8 @@ spec: - verify - review - freeform + - integrate + - reconcile type: string name: description: |- @@ -482,6 +484,12 @@ spec: - reset - rebase type: string + contract: + description: |- + Contract is the slice plan's prose contract, handed to a reconcile + task's LLM sweep as the description of the intended shared interfaces. + Reconcile only. Part of #1033. + type: string gateAdvisories: description: |- GateAdvisories holds non-blocking findings copied from the upstream @@ -553,6 +561,69 @@ spec: executor logs and falls back to branching from BaseBranch rather than failing. type: string + sharedIdentifiers: + description: |- + SharedIdentifiers pins the exact cross-slice strings a reconcile task + verifies against the integrated union. Reconcile only. Part of #1033. + items: + description: |- + SharedIdentifier pins one exact string that crosses a slice boundary (a + metric name, config key, or CRD field). The reconcile task asserts the ID + appears verbatim in the DefinedBy slice and every ReferencedBy slice; a + missing pin is an authoritative drift. Part of Sliced Workloads (#1033). + properties: + definedBy: + description: DefinedBy is the slice name that must + produce the identifier. + minLength: 1 + type: string + id: + description: ID is the exact string every listed slice + must contain verbatim. + minLength: 1 + type: string + referencedBy: + description: ReferencedBy are the slice names that + must consume the identifier. + items: + type: string + type: array + required: + - definedBy + - id + type: object + type: array + slices: + description: |- + Slices are the disjoint slices of a sliced Workload. The integrate task + unions their branches; the reconcile task checks pins against their + files. Set on integrate and reconcile tasks. Part of #1033. + items: + description: |- + SliceRef describes one disjoint slice of a sliced Workload. The integrate + task unions each slice's Branch; the reconcile task verifies pinned + identifiers appear in each slice's Files. Part of Sliced Workloads (#1033). + properties: + branch: + description: |- + Branch is the git ref the slice's coder pushed. Required for the + integrate task, which applies each slice branch's diff onto the base. + type: string + files: + description: |- + Files are the repo-relative paths this slice owns, so a reconcile task + looks for each pinned identifier in the right slice. + items: + type: string + type: array + name: + description: Name is the slice's name within the plan. + minLength: 1 + type: string + required: + - name + type: object + type: array type: object priority: description: |- diff --git a/cmd/foreman-agent/main.go b/cmd/foreman-agent/main.go index 9e5cd477..46d6b206 100644 --- a/cmd/foreman-agent/main.go +++ b/cmd/foreman-agent/main.go @@ -748,6 +748,13 @@ func makeRegistryFactory( Fetcher: githubissue.NewClient(), Token: repo.TokenFromEnvOrFile, }, + // run_integrate: deterministic tool for a sliced Workload's + // integrate step. Unions the disjoint slice branches onto the + // current base and pushes the integration branch (#1033). + &foremantools.RunIntegrateTool{ + Workspace: workspace, + Token: repo.TokenFromEnvOrFile, + }, } var mcpTools []foremantools.Tool diff --git a/config/crd/bases/foreman.llmkube.dev_agentictasks.yaml b/config/crd/bases/foreman.llmkube.dev_agentictasks.yaml index 50a48753..7599ecf2 100644 --- a/config/crd/bases/foreman.llmkube.dev_agentictasks.yaml +++ b/config/crd/bases/foreman.llmkube.dev_agentictasks.yaml @@ -146,6 +146,8 @@ spec: - verify - review - freeform + - integrate + - reconcile type: string mcpEnabled: description: |- @@ -215,6 +217,12 @@ spec: - reset - rebase type: string + contract: + description: |- + Contract is the slice plan's prose contract, handed to a reconcile + task's LLM sweep as the description of the intended shared interfaces. + Reconcile only. Part of #1033. + type: string gateAdvisories: description: |- GateAdvisories holds non-blocking findings copied from the upstream @@ -285,6 +293,69 @@ spec: executor logs and falls back to branching from BaseBranch rather than failing. type: string + sharedIdentifiers: + description: |- + SharedIdentifiers pins the exact cross-slice strings a reconcile task + verifies against the integrated union. Reconcile only. Part of #1033. + items: + description: |- + SharedIdentifier pins one exact string that crosses a slice boundary (a + metric name, config key, or CRD field). The reconcile task asserts the ID + appears verbatim in the DefinedBy slice and every ReferencedBy slice; a + missing pin is an authoritative drift. Part of Sliced Workloads (#1033). + properties: + definedBy: + description: DefinedBy is the slice name that must produce + the identifier. + minLength: 1 + type: string + id: + description: ID is the exact string every listed slice must + contain verbatim. + minLength: 1 + type: string + referencedBy: + description: ReferencedBy are the slice names that must + consume the identifier. + items: + type: string + type: array + required: + - definedBy + - id + type: object + type: array + slices: + description: |- + Slices are the disjoint slices of a sliced Workload. The integrate task + unions their branches; the reconcile task checks pins against their + files. Set on integrate and reconcile tasks. Part of #1033. + items: + description: |- + SliceRef describes one disjoint slice of a sliced Workload. The integrate + task unions each slice's Branch; the reconcile task verifies pinned + identifiers appear in each slice's Files. Part of Sliced Workloads (#1033). + properties: + branch: + description: |- + Branch is the git ref the slice's coder pushed. Required for the + integrate task, which applies each slice branch's diff onto the base. + type: string + files: + description: |- + Files are the repo-relative paths this slice owns, so a reconcile task + looks for each pinned identifier in the right slice. + items: + type: string + type: array + name: + description: Name is the slice's name within the plan. + minLength: 1 + type: string + required: + - name + type: object + type: array type: object priority: description: |- diff --git a/config/crd/bases/foreman.llmkube.dev_workloads.yaml b/config/crd/bases/foreman.llmkube.dev_workloads.yaml index 9951caa6..cdbf2b0e 100644 --- a/config/crd/bases/foreman.llmkube.dev_workloads.yaml +++ b/config/crd/bases/foreman.llmkube.dev_workloads.yaml @@ -420,6 +420,8 @@ spec: - verify - review - freeform + - integrate + - reconcile type: string name: description: |- @@ -481,6 +483,12 @@ spec: - reset - rebase type: string + contract: + description: |- + Contract is the slice plan's prose contract, handed to a reconcile + task's LLM sweep as the description of the intended shared interfaces. + Reconcile only. Part of #1033. + type: string gateAdvisories: description: |- GateAdvisories holds non-blocking findings copied from the upstream @@ -552,6 +560,69 @@ spec: executor logs and falls back to branching from BaseBranch rather than failing. type: string + sharedIdentifiers: + description: |- + SharedIdentifiers pins the exact cross-slice strings a reconcile task + verifies against the integrated union. Reconcile only. Part of #1033. + items: + description: |- + SharedIdentifier pins one exact string that crosses a slice boundary (a + metric name, config key, or CRD field). The reconcile task asserts the ID + appears verbatim in the DefinedBy slice and every ReferencedBy slice; a + missing pin is an authoritative drift. Part of Sliced Workloads (#1033). + properties: + definedBy: + description: DefinedBy is the slice name that must + produce the identifier. + minLength: 1 + type: string + id: + description: ID is the exact string every listed slice + must contain verbatim. + minLength: 1 + type: string + referencedBy: + description: ReferencedBy are the slice names that + must consume the identifier. + items: + type: string + type: array + required: + - definedBy + - id + type: object + type: array + slices: + description: |- + Slices are the disjoint slices of a sliced Workload. The integrate task + unions their branches; the reconcile task checks pins against their + files. Set on integrate and reconcile tasks. Part of #1033. + items: + description: |- + SliceRef describes one disjoint slice of a sliced Workload. The integrate + task unions each slice's Branch; the reconcile task verifies pinned + identifiers appear in each slice's Files. Part of Sliced Workloads (#1033). + properties: + branch: + description: |- + Branch is the git ref the slice's coder pushed. Required for the + integrate task, which applies each slice branch's diff onto the base. + type: string + files: + description: |- + Files are the repo-relative paths this slice owns, so a reconcile task + looks for each pinned identifier in the right slice. + items: + type: string + type: array + name: + description: Name is the slice's name within the plan. + minLength: 1 + type: string + required: + - name + type: object + type: array type: object priority: description: |- diff --git a/pkg/foreman/agent/executor_native.go b/pkg/foreman/agent/executor_native.go index a3d9a323..e2718f30 100644 --- a/pkg/foreman/agent/executor_native.go +++ b/pkg/foreman/agent/executor_native.go @@ -1108,6 +1108,18 @@ func buildDeterministicArgs(task *foremanv1alpha1.AgenticTask, branch, cloneURL args["commands"] = cmds } + // Sliced-workload steps (#1033) carry the slice plan for the integrate / + // reconcile tools: the slices to union or check, the upstream source of the + // base, and (for reconcile) the pinned identifiers + contract. Added only + // for those kinds so the gate's args stay byte-identical. + if task.Spec.Kind == foremanv1alpha1.AgenticTaskKindIntegrate || + task.Spec.Kind == foremanv1alpha1.AgenticTaskKindReconcile { + args["slices"] = task.Spec.Payload.Slices + args["upstreamURL"] = upstreamURLForRepo(task.Spec.Payload.Repo) + args["sharedIdentifiers"] = task.Spec.Payload.SharedIdentifiers + args["contract"] = task.Spec.Payload.Contract + } + out, _ := json.Marshal(args) return out } diff --git a/pkg/foreman/agent/tools/run_integrate.go b/pkg/foreman/agent/tools/run_integrate.go new file mode 100644 index 00000000..bfcc05b1 --- /dev/null +++ b/pkg/foreman/agent/tools/run_integrate.go @@ -0,0 +1,265 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tools + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os/exec" + "regexp" + "strings" + + "github.com/defilantech/llmkube/pkg/foreman/agent" + "github.com/defilantech/llmkube/pkg/foreman/agent/oai" + "github.com/defilantech/llmkube/pkg/foreman/agent/repo" + "github.com/defilantech/llmkube/pkg/foreman/slicer" +) + +// RunIntegrateTool unions a sliced Workload's disjoint slice branches onto the +// current base on a fresh integration branch and pushes it, so a downstream +// reconcile task can check the union for cross-slice interface drift. It runs +// on the deterministic (no-LLM) path: the executor dispatches it directly and +// its Terminal verdict drives the AgenticTask status. Part of Sliced Workloads +// (#1033). +// +// The slice branches live on origin (the fork the coders pushed to); the base +// lives on the upstream project (the #813 base resolution). So the tool fetches +// from two remotes into local refs before the local union, then pushes the +// integration branch back to origin. +type RunIntegrateTool struct { + // Workspace is the cloned fork working tree (origin = the fork). The union + // and the fetches happen here. + Workspace string + // Token sources the git token for the fetches + push. Production wires + // repo.TokenFromEnvOrFile; a nil Token skips auth (public repos, tests + // against local remotes). + Token func() (string, error) + // Run overrides the git transport for the local union (slicer.Integrate). + // Nil uses slicer's exec default. Tests inject a runner; the fetch/push the + // tool does itself always shell real git in Workspace. + Run slicer.GitRunner +} + +// integrateSlice is one slice's dispatch view: its pushed branch and the files +// it owns (files are unused by integrate but travel in the same payload shape +// the reconcile step consumes). +type integrateSlice struct { + Name string `json:"name"` + Branch string `json:"branch"` + Files []string `json:"files"` +} + +type integrateArgs struct { + BaseBranch string `json:"baseBranch"` + Branch string `json:"branch"` + UpstreamURL string `json:"upstreamURL"` + Slices []integrateSlice `json:"slices"` +} + +func (RunIntegrateTool) Name() string { return "run_integrate" } + +// Schema advertises the tool. The deterministic executor path never shows this +// to a model, but a future LLM-driven wrapper would, so it stays faithful. +func (RunIntegrateTool) Schema() oai.ToolSchemaDef { + return oai.ToolSchemaDef{ + Name: "run_integrate", + Description: "Union the disjoint slice branches of a sliced workload onto the current base " + + "on a fresh integration branch and push it. Returns GATE-PASS on a clean union, " + + "GATE-FAIL when slices overlap or a slice does not apply onto the base, or " + + "GATE-ERROR on a fetch/push failure.", + Parameters: json.RawMessage(`{ +"type": "object", +"properties": { + "branch": {"type": "string", "description": "integration branch to create and push"}, + "baseBranch": {"type": "string", "description": "base ref to union onto (default main)"}, + "upstreamURL": {"type": "string", "description": "git URL the base ref is fetched from"}, + "slices": {"type": "array", "items": {"type": "object", "properties": { + "name": {"type": "string"}, + "branch": {"type": "string", "description": "the slice's pushed branch on origin"}, + "files": {"type": "array", "items": {"type": "string"}} + }}, "description": "the disjoint slices to union"} +}, +"required": ["branch", "slices"] +}`), + } +} + +var localRefUnsafe = regexp.MustCompile(`[^A-Za-z0-9._-]+`) + +var refAllowed = regexp.MustCompile(`^[A-Za-z0-9._/-]+$`) + +// refSafe reports whether s is safe to pass to git as a ref/positional +// argument: non-empty, not an option (no leading '-'), no ".." traversal, and +// only git-ref-safe characters. Mirrors repo.gitRefSafe so a value beginning +// with '-' cannot be smuggled in as a git flag (argv injection, e.g. +// --upload-pack=). Shared with run_reconcile. +func refSafe(s string) bool { + return s != "" && !strings.HasPrefix(s, "-") && + !strings.Contains(s, "..") && refAllowed.MatchString(s) +} + +// gitURLSafe reports whether s is a safe git remote URL: not an option and a +// known scheme, so it cannot be read as a git flag. +func gitURLSafe(s string) bool { + if s == "" || strings.HasPrefix(s, "-") { + return false + } + for _, scheme := range []string{"https://", "http://", "ssh://", "git://", "git@", "file://", "/"} { + if strings.HasPrefix(s, scheme) { + return true + } + } + return false +} + +// Execute fetches the base + each slice into local refs, unions them via +// slicer.Integrate, and pushes the integration branch. It never returns a +// non-nil error for a content/transport failure: those map to a Terminal +// GATE-FAIL/GATE-ERROR verdict so the AgenticTask reaches a terminal state. +func (t *RunIntegrateTool) Execute(ctx context.Context, raw json.RawMessage) (*agent.ToolResult, error) { + var a integrateArgs + if err := json.Unmarshal(raw, &a); err != nil { + return t.gateError("bad args: " + err.Error()), nil + } + if a.Branch == "" || len(a.Slices) == 0 { + return t.gateError("run_integrate requires branch and at least one slice"), nil + } + // Validate every user-influenced value before it reaches git argv, so a + // value beginning with '-' cannot be smuggled in as a git flag. + if !refSafe(a.Branch) { + return t.gateError(fmt.Sprintf("unsafe integration branch %q", a.Branch)), nil + } + base := a.BaseBranch + if base == "" { + base = "main" + } + if !refSafe(base) { + return t.gateError(fmt.Sprintf("unsafe base branch %q", base)), nil + } + if a.UpstreamURL != "" && !gitURLSafe(a.UpstreamURL) { + return t.gateError(fmt.Sprintf("unsafe upstream url %q", a.UpstreamURL)), nil + } + + var authEnv []string + if t.Token != nil { + if tok, err := t.Token(); err == nil && tok != "" { + if auth, aerr := repo.NewAuth(tok); aerr == nil { + authEnv = auth.Env() + } + } + } + + // 1. Fetch the base from the upstream project into a local ref (origin is + // the fork; the base lives upstream). An empty upstreamURL means the base + // is already a local ref (tests, or a same-repo flow). + const baseLocal = "_slicer_base" + if a.UpstreamURL != "" { + if v := t.fetchToLocal(ctx, authEnv, a.UpstreamURL, base, baseLocal); v != nil { + return v, nil + } + base = baseLocal + } + + // 2. Fetch each slice branch from origin into a local ref. + sliceRefs := make([]string, 0, len(a.Slices)) + for _, s := range a.Slices { + if !refSafe(s.Branch) { + return t.gateError(fmt.Sprintf("slice %q has an unsafe or empty branch %q", s.Name, s.Branch)), nil + } + local := "_slicer_slice_" + localRefUnsafe.ReplaceAllString(s.Name, "-") + if v := t.fetchToLocal(ctx, authEnv, "origin", s.Branch, local); v != nil { + return v, nil + } + sliceRefs = append(sliceRefs, local) + } + + // 3. Union onto the base. slicer.Integrate proves disjointness first, then + // applies each slice's diff. + res, err := slicer.Integrate(ctx, slicer.IntegrateOptions{ + RepoDir: t.Workspace, + Base: base, + Branch: a.Branch, + Slices: sliceRefs, + Run: t.Run, + }) + if err != nil { + var overlap *slicer.OverlapError + var apply *slicer.ApplyError + var empty *slicer.EmptyUnionError + if errors.As(err, &overlap) || errors.As(err, &apply) || errors.As(err, &empty) { + // A slice violated its file scope, was cut from a stale base, or the + // union is empty: a real content failure the coder/planner must fix. + return t.gateFail(err.Error()), nil + } + // Anything else is a git/transport fault: retryable infrastructure. + return t.gateError("integrate: " + err.Error()), nil + } + + // 4. Push the integration branch to origin for the reconcile step + PR. + if err := t.runGit(ctx, authEnv, "push", "--force-with-lease", "--end-of-options", "origin", a.Branch); err != nil { + return t.gateError("push integration branch: " + err.Error()), nil + } + + return &agent.ToolResult{ + Terminal: true, + Verdict: VerdictGatePass, + Summary: fmt.Sprintf("integrated %d slices into %s", len(sliceRefs), a.Branch), + Output: map[string]any{ + "branch": res.Branch, + "owners": res.Owners, + }, + }, nil +} + +// fetchToLocal fetches remoteRef from remote and pins it to a local branch. +// Returns a non-nil GATE-ERROR ToolResult on failure, nil on success. +func (t *RunIntegrateTool) fetchToLocal( + ctx context.Context, env []string, remote, remoteRef, localBranch string, +) *agent.ToolResult { + // --end-of-options is defense in depth on top of the refSafe/gitURLSafe + // validation in Execute: even a validated value is treated as positional. + if err := t.runGit(ctx, env, "fetch", "--end-of-options", remote, remoteRef); err != nil { + return t.gateError(fmt.Sprintf("fetch %s %s: %s", remote, remoteRef, err.Error())) + } + if err := t.runGit(ctx, env, "branch", "-f", localBranch, "FETCH_HEAD"); err != nil { + return t.gateError(fmt.Sprintf("pin %s: %s", localBranch, err.Error())) + } + return nil +} + +// runGit shells one git subcommand in the workspace with the extra env. +func (t *RunIntegrateTool) runGit(ctx context.Context, extraEnv []string, args ...string) error { + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = t.Workspace + cmd.Env = append(cmd.Environ(), extraEnv...) + if out, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("%w: %s", err, strings.TrimSpace(string(out))) + } + return nil +} + +func (t *RunIntegrateTool) gateFail(msg string) *agent.ToolResult { + return &agent.ToolResult{Terminal: true, Verdict: VerdictGateFail, Summary: msg, + Output: map[string]any{"error": msg}} +} + +func (t *RunIntegrateTool) gateError(msg string) *agent.ToolResult { + return &agent.ToolResult{Terminal: true, Verdict: VerdictGateError, Summary: msg, + Output: map[string]any{"error": msg}} +} diff --git a/pkg/foreman/agent/tools/run_integrate_test.go b/pkg/foreman/agent/tools/run_integrate_test.go new file mode 100644 index 00000000..5a1cd3e9 --- /dev/null +++ b/pkg/foreman/agent/tools/run_integrate_test.go @@ -0,0 +1,170 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tools + +import ( + "context" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func git(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %s (in %s): %v\n%s", strings.Join(args, " "), dir, err, out) + } + return string(out) +} + +func writeFile(t *testing.T, dir, path, content string) { + t.Helper() + full := filepath.Join(dir, path) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +// integrateFixture builds a bare "origin" repo with a base commit on main and +// the given slice branches, and clones it into a workspace with a git identity. +// Returns (originPath, workspacePath). +func integrateFixture(t *testing.T, slices map[string]map[string]string) (string, string) { + t.Helper() + root := t.TempDir() + origin := filepath.Join(root, "origin.git") + git(t, root, "init", "-q", "--bare", "-b", "main", origin) + + // seed: base commit on main. + seed := filepath.Join(root, "seed") + git(t, root, "clone", "-q", origin, seed) + git(t, seed, "config", "user.email", "t@e.com") + git(t, seed, "config", "user.name", "T") + git(t, seed, "config", "commit.gpgsign", "false") + writeFile(t, seed, "base.txt", "base\n") + git(t, seed, "add", "-A") + git(t, seed, "commit", "-qm", "base") + git(t, seed, "push", "-q", "origin", "main") + + // each slice branch off main. + for name, files := range slices { + git(t, seed, "checkout", "-q", "-B", name, "main") + for p, c := range files { + writeFile(t, seed, p, c) + } + git(t, seed, "add", "-A") + git(t, seed, "commit", "-qm", name) + git(t, seed, "push", "-q", "origin", name) + git(t, seed, "checkout", "-q", "main") + } + + // the executor's clone (origin = the bare repo). + ws := filepath.Join(root, "ws") + git(t, root, "clone", "-q", origin, ws) + git(t, ws, "config", "user.email", "t@e.com") + git(t, ws, "config", "user.name", "T") + git(t, ws, "config", "commit.gpgsign", "false") + return origin, ws +} + +func TestRunIntegrate_CleanUnionPushed(t *testing.T) { + origin, ws := integrateFixture(t, map[string]map[string]string{ + "foreman/s/a": {"a/new.txt": "AAA\n"}, + "foreman/s/b": {"b/new.txt": "BBB\n"}, + }) + tool := &RunIntegrateTool{Workspace: ws} + args, _ := json.Marshal(map[string]any{ + "branch": "foreman/s/integ", + "baseBranch": "main", + "upstreamURL": origin, // base fetched from here; slices from origin remote + "slices": []map[string]any{ + {"name": "a", "branch": "foreman/s/a"}, + {"name": "b", "branch": "foreman/s/b"}, + }, + }) + res, err := tool.Execute(context.Background(), args) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Verdict != VerdictGatePass { + t.Fatalf("verdict = %q (%s), want GATE-PASS", res.Verdict, res.Summary) + } + // the integration branch was pushed to origin and carries both slices. + git(t, ws, "fetch", "-q", "origin", "foreman/s/integ") + names := git(t, ws, "ls-tree", "-r", "--name-only", "FETCH_HEAD") + if !strings.Contains(names, "a/new.txt") || !strings.Contains(names, "b/new.txt") { + t.Fatalf("integration branch missing a slice's file:\n%s", names) + } +} + +func TestRunIntegrate_OverlapIsGateFail(t *testing.T) { + origin, ws := integrateFixture(t, map[string]map[string]string{ + "foreman/s/a": {"shared.txt": "from-a\n"}, + "foreman/s/b": {"shared.txt": "from-b\n"}, + }) + tool := &RunIntegrateTool{Workspace: ws} + args, _ := json.Marshal(map[string]any{ + "branch": "foreman/s/integ", + "baseBranch": "main", + "upstreamURL": origin, + "slices": []map[string]any{ + {"name": "a", "branch": "foreman/s/a"}, + {"name": "b", "branch": "foreman/s/b"}, + }, + }) + res, err := tool.Execute(context.Background(), args) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Verdict != VerdictGateFail { + t.Fatalf("verdict = %q (%s), want GATE-FAIL for overlapping slices", res.Verdict, res.Summary) + } +} + +func TestRunIntegrate_BadArgsIsGateError(t *testing.T) { + tool := &RunIntegrateTool{Workspace: t.TempDir()} + res, _ := tool.Execute(context.Background(), json.RawMessage(`{"branch":"x"}`)) // no slices + if res.Verdict != VerdictGateError { + t.Fatalf("verdict = %q, want GATE-ERROR for missing slices", res.Verdict) + } +} + +// A branch/ref that begins with '-' (or an option-looking URL) must be rejected +// before it reaches git argv, closing the flag-smuggling vector. +func TestRunIntegrate_ArgvInjectionRejected(t *testing.T) { + tool := &RunIntegrateTool{Workspace: t.TempDir()} + cases := []string{ + `{"branch":"--upload-pack=touch /tmp/pwn","slices":[{"name":"a","branch":"foreman/s/a"}]}`, + `{"branch":"integ","slices":[{"name":"a","branch":"--output=/etc/x"}]}`, + `{"branch":"integ","baseBranch":"-x","slices":[{"name":"a","branch":"foreman/s/a"}]}`, + `{"branch":"integ","upstreamURL":"--upload-pack=x","slices":[{"name":"a","branch":"foreman/s/a"}]}`, + } + for _, c := range cases { + res, _ := tool.Execute(context.Background(), json.RawMessage(c)) + if res.Verdict != VerdictGateError { + t.Fatalf("verdict = %q for %s, want GATE-ERROR", res.Verdict, c) + } + } +}