From 1e7da7f663c5eec6c45c759356a0c521e9441a8e Mon Sep 17 00:00:00 2001 From: Christopher Maher Date: Thu, 9 Jul 2026 19:26:56 -0700 Subject: [PATCH 1/3] feat(foreman): integrate + reconcile AgenticTask kinds + slice payload Add the integrate and reconcile AgenticTaskKind values (enum widened on both AgenticTask.spec.kind and Workload pipeline steps) and the payload fields the sliced-workload pipeline needs: Slices ([]SliceRef of name/branch/files), Contract, and SharedIdentifiers ([]SharedIdentifier of id/definedBy/ referencedBy). integrate reads slices[].branch + baseBranch; reconcile reads sharedIdentifiers + slices[].files + contract. Base and integration branch reuse the existing baseBranch/branch fields. API surface only: no executor handling yet (a follow-up wires the two kinds to the slicer libraries). Regenerated deepcopy + CRD manifests + chart CRDs; the AgenticTask webhook validates only agentRef, so no webhook change. Part of #1033 Signed-off-by: Christopher Maher --- api/foreman/v1alpha1/agentictask_types.go | 72 ++++++++++++++++++- api/foreman/v1alpha1/zz_generated.deepcopy.go | 54 ++++++++++++++ .../foreman/templates/crds/agentictasks.yaml | 71 ++++++++++++++++++ charts/foreman/templates/crds/workloads.yaml | 71 ++++++++++++++++++ .../foreman.llmkube.dev_agentictasks.yaml | 71 ++++++++++++++++++ .../bases/foreman.llmkube.dev_workloads.yaml | 71 ++++++++++++++++++ 6 files changed, 409 insertions(+), 1 deletion(-) 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/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: |- From a82f8c32fa8ef9a42835619d1249074c84ea9c35 Mon Sep 17 00:00:00 2001 From: Christopher Maher Date: Thu, 9 Jul 2026 21:12:13 -0700 Subject: [PATCH 2/3] feat(foreman): run_integrate tool for sliced-workload union Task 4a of the Sliced Workloads epic (#1033): the deterministic run_integrate tool that a sliced Workload's integrate step dispatches. It fetches the base from upstream and each slice branch from origin into local refs, unions them via slicer.Integrate, and pushes the integration branch back to origin for the downstream reconcile step + PR. Verdict mapping: clean union -> GATE-PASS; overlap / stale-base apply-conflict / empty union -> GATE-FAIL (a slice violated its file scope, real signal); a fetch/push fault -> GATE-ERROR (retryable). Git auth comes from repo.TokenFromEnvOrFile (same source as fetch_issue), so the tool needs no new plumbing in makeRegistryFactory. buildDeterministicArgs surfaces the slice plan (slices, upstreamURL, and the reconcile fields) only for the integrate/reconcile kinds, so the gate's args stay byte-identical. Real-git tests cover the clean union (pushed, carries both slices), overlap -> GATE-FAIL, and bad-args -> GATE-ERROR. Part of #1033 Signed-off-by: Christopher Maher --- cmd/foreman-agent/main.go | 7 + pkg/foreman/agent/executor_native.go | 12 + pkg/foreman/agent/tools/run_integrate.go | 226 ++++++++++++++++++ pkg/foreman/agent/tools/run_integrate_test.go | 152 ++++++++++++ 4 files changed, 397 insertions(+) create mode 100644 pkg/foreman/agent/tools/run_integrate.go create mode 100644 pkg/foreman/agent/tools/run_integrate_test.go 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/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..b9fc47a8 --- /dev/null +++ b/pkg/foreman/agent/tools/run_integrate.go @@ -0,0 +1,226 @@ +/* +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._-]+`) + +// 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 + } + base := a.BaseBranch + if base == "" { + base = "main" + } + + 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 s.Branch == "" { + return t.gateError(fmt.Sprintf("slice %q has no branch to integrate", s.Name)), 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", "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 { + if err := t.runGit(ctx, env, "fetch", 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..3e2e19d9 --- /dev/null +++ b/pkg/foreman/agent/tools/run_integrate_test.go @@ -0,0 +1,152 @@ +/* +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) + } +} From 4981f516bfb50a48c1fc11549736a5acebbdbbfb Mon Sep 17 00:00:00 2001 From: Christopher Maher Date: Thu, 9 Jul 2026 21:16:14 -0700 Subject: [PATCH 3/3] fix(foreman): validate git refs/URLs in run_integrate against argv injection The tool passed user-influenced values (integration branch, base, slice branches, upstream URL) straight to git argv, so a value beginning with '-' could be smuggled in as a git flag (e.g. fetch --upload-pack= executes a command). Validate every such value up front with refSafe/gitURLSafe (mirroring repo.gitRefSafe: no leading '-', no '..', git-ref-safe charset; URL scheme allowlist) and reject with GATE-ERROR, and pass --end-of-options on fetch/push as defense in depth. Adds a test asserting option-looking refs/URLs are rejected. Flagged by the commit security review. Signed-off-by: Christopher Maher --- pkg/foreman/agent/tools/run_integrate.go | 47 +++++++++++++++++-- pkg/foreman/agent/tools/run_integrate_test.go | 18 +++++++ 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/pkg/foreman/agent/tools/run_integrate.go b/pkg/foreman/agent/tools/run_integrate.go index b9fc47a8..bfcc05b1 100644 --- a/pkg/foreman/agent/tools/run_integrate.go +++ b/pkg/foreman/agent/tools/run_integrate.go @@ -102,6 +102,32 @@ func (RunIntegrateTool) Schema() oai.ToolSchemaDef { 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 @@ -114,10 +140,21 @@ func (t *RunIntegrateTool) Execute(ctx context.Context, raw json.RawMessage) (*a 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 { @@ -142,8 +179,8 @@ func (t *RunIntegrateTool) Execute(ctx context.Context, raw json.RawMessage) (*a // 2. Fetch each slice branch from origin into a local ref. sliceRefs := make([]string, 0, len(a.Slices)) for _, s := range a.Slices { - if s.Branch == "" { - return t.gateError(fmt.Sprintf("slice %q has no branch to integrate", s.Name)), nil + 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 { @@ -175,7 +212,7 @@ func (t *RunIntegrateTool) Execute(ctx context.Context, raw json.RawMessage) (*a } // 4. Push the integration branch to origin for the reconcile step + PR. - if err := t.runGit(ctx, authEnv, "push", "--force-with-lease", "origin", a.Branch); err != nil { + 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 } @@ -195,7 +232,9 @@ func (t *RunIntegrateTool) Execute(ctx context.Context, raw json.RawMessage) (*a func (t *RunIntegrateTool) fetchToLocal( ctx context.Context, env []string, remote, remoteRef, localBranch string, ) *agent.ToolResult { - if err := t.runGit(ctx, env, "fetch", remote, remoteRef); err != nil { + // --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 { diff --git a/pkg/foreman/agent/tools/run_integrate_test.go b/pkg/foreman/agent/tools/run_integrate_test.go index 3e2e19d9..5a1cd3e9 100644 --- a/pkg/foreman/agent/tools/run_integrate_test.go +++ b/pkg/foreman/agent/tools/run_integrate_test.go @@ -150,3 +150,21 @@ func TestRunIntegrate_BadArgsIsGateError(t *testing.T) { 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) + } + } +}