Fix | Stream branch root for local Helm charts with out-of-chart value files

AGENTS.md

@@ -47,6 +47,77 @@ make update-integration-tests 2>&1 | tail -50
 make run-integration-tests-go 2>&1 | tail -50
 ```
 
+## How to Create an Integration Test
+
+This is the stuff that would have saved time when getting started. An integration
+test runs the real tool against a real kind cluster + Argo CD and compares the
+generated diff against a saved "golden" file.
+
+### How it works (the mental model)
+
+- Each test case clones **two real Git branches** from the upstream repo
+  (`dag-andersen/argocd-diff-preview`): a `base` branch and a `target` branch.
+  These branches are full snapshots of the whole repo. The tool diffs them.
+- The test data lives in branches named `integration-test/branch-N/base` and
+  `integration-test/branch-N/target`. The example apps the tool renders live
+  under `examples/` on those branches.
+- The expected output lives **in this repo** (not on the data branches), under
+  `integration-test/branch-N/target<suffix>/{output.md,output.html}`. This is
+  the golden file the actual run is compared against.
+- Test cases are defined as `TestCase` structs in
+  `integration-test/integration_test.go` (see the big `testCases` slice).
+
+### Steps to add a new test
+
+1. **Pick the next free branch number.** Run
+   `git ls-remote --heads origin 'refs/heads/integration-test/*'` and pick the
+   next `branch-N`. (Note: some numbers may exist on the remote but not in
+   `testCases` - check both.)
+
+2. **Create the two data branches from `main`, then edit the example files.**
+   The branches are just `main` with example files changed:
+   ```bash
+   git checkout main
+   git checkout -b integration-test/branch-N/base
+   # add/modify files under examples/ that the test should render
+   git commit -am "Tests | Add branch-N base: <what it tests>"
+
+   git checkout -b integration-test/branch-N/target   # branches off base
+   # change ONLY the file(s) whose diff you want to verify
+   git commit -am "Tests | Add branch-N target: <what changed>"
+
+   git push -u origin integration-test/branch-N/base integration-test/branch-N/target
+   ```
+   Keep `base` and `target` nearly identical - they should differ only in the
+   file(s) under test, so the diff output stays small and focused. (Look at how
+   an existing pair like `branch-16` differs by a single file.)
+
+3. **Add the `TestCase`** to the `testCases` slice in `integration_test.go`.
+   The expected-output directory is derived from `TargetBranch` + `Suffix`
+   (e.g. `TargetBranch: integration-test/branch-N/target`, `Suffix: "-1"` ->
+   `branch-N/target-1/`). Useful fields:
+   - `RenderMethod`: `"cli"`, `"server-api"`, or `"repo-server-api"` (omit to use the default).
+   - `FileRegex` / `FilesChanged`: limit which changed files are considered.
+   - `WatchIfNoWatchPatternFound: "false"` + an `argocd-diff-preview/watch-pattern`
+     annotation on the app limits rendering to **just that app**, even though the
+     branch is a full repo snapshot. This is the easiest way to keep output focused.
+
+4. **Generate the golden output** by running the single test once with `-update`.
+   The first run for a new case must use `-update` (there is no golden file to
+   compare against yet). This needs Docker running:
+   ```bash
+   make go-build
+   cd integration-test
+   TEST_CASE="branch-N/target" go test -v -timeout 20m -run TestSingleCase -update ./...
+   ```
+   This writes `output.md` / `output.html`. Read them and sanity-check the diff.
+   Timing values are auto-normalized to `Xs`, so the golden file is stable.
+
+5. **Verify it actually passes** in compare mode (no `-update`):
+   ```bash
+   TEST_CASE="branch-N/target" go test -v -timeout 20m -run TestSingleCase ./...
+   ```
+
 ### Commit Messages and PR Titles
 
 Use the format: `<Prefix> | <message>`
@@ -69,17 +140,6 @@ Chore | Update ArgoCD dependency to v3.0
 
 **Do NOT add issue numbers in parentheses to commit messages or PR titles.** The `(#123)` format is what GitHub automatically adds when merging/squashing PRs (and it refers to the PR number, not the issue).
 
-## Project Structure
-
-```
-argocd-diff-preview/
-├── cmd/               # CLI entry point (main.go, options.go)
-├── pkg/               # Core go logic
-├── integration-test/  # Integration tests and expected outputs
-├── docs/              # MkDocs documentation
-└── examples/          # Test fixtures
-```
-
 ## Main Challenges when building a tool like this
 
 - A repository can contain multiple applications and applications with the same name. We ALWAYS need to make sure the names are unique.

integration-test/README.md

@@ -183,6 +183,11 @@ Each `branch-N/target[-suffix]/` directory contains expected output files (`outp
 ### Branch 15: Additional Coverage
 - `target`: Basic diff (no special options)
 
+### Branch 16-18: repo-server-api Render Method
+- `branch-16/target`: Config Management Plugin (kustomize-build-with-helm) via `repo-server-api`
+- `branch-17/target-1` / `target-2`: App-of-apps traversal via `repo-server-api`
+- `branch-18/target`: Single-source local Helm chart whose `helm.valueFiles` reference a file OUTSIDE the chart directory via a repo-root-absolute path (`/examples/out-of-chart-values/env/values.yaml`). Regression test for `repo-server-api`: the tool must stream the whole branch folder so the out-of-chart value file is reachable, otherwise the render fails with `no such file or directory`.
+
 ## Prerequisites
 
 - Docker running

integration-test/branch-18/target/output.html

@@ -0,0 +1,107 @@
+<html>
+<head>
+<meta charset="utf-8">
+<style>
+body {
+	font-family: arial;
+}
+.container {
+	margin: auto;
+	width: 910px;
+}
+.diffs {
+	margin: 20px 0 20px 0;
+}
+.diff_container {
+	width: 910px;
+	overflow-x: scroll;
+	border-radius: 8px;
+	background:rgb(239, 239, 239);
+	scrollbar-width: none;
+	margin: 10px 0 10px 0;
+}
+table {
+	font-family: monospace;
+	border-spacing: 0px;
+	width: 100%;
+}
+tr.normal_line {
+	background:rgb(239, 239, 239);
+}
+tr.added_line {
+	background:rgb(169, 216, 184);
+}
+tr.removed_line {
+	background:rgb(247, 149, 173);
+}
+tr.comment_line {
+	background:rgb(197, 194, 194);
+}
+.resource_header {
+	font-family: monospace;
+	font-size: 14px;
+	color: rgb(80, 80, 80);
+	margin: 15px 0 5px 0;
+	padding: 0;
+}
+.resource_header:first-of-type {
+	margin-top: 10px;
+}
+pre {
+	margin: 0;
+	padding-left: 15px;
+	padding-right: 15px;
+}
+</style>
+</head>
+<body>
+<div class="container">
+<h1>Argo CD Diff Preview</h1>
+
+<p>Summary:</p>
+<pre>Modified (1):
+± out-of-chart-values (+2|-2)</pre>
+
+<div class="diffs">
+<details>
+<summary>
+out-of-chart-values (examples/out-of-chart-values/application.yaml)
+</summary>
+
+<h4 class="resource_header">Deployment: default/out-of-chart-values</h4>
+<div class="diff_container">
+<table>
+
+	<tr class="normal_line"><td><pre> apiVersion: apps/v1</pre></td></tr>
+	<tr class="normal_line"><td><pre> kind: Deployment</pre></td></tr>
+	<tr class="normal_line"><td><pre> metadata:</pre></td></tr>
+	<tr class="normal_line"><td><pre>   name: out-of-chart-values</pre></td></tr>
+	<tr class="normal_line"><td><pre>   namespace: default</pre></td></tr>
+	<tr class="normal_line"><td><pre> spec:</pre></td></tr>
+	<tr class="removed_line"><td><pre>-  replicas: 2</pre></td></tr>
+	<tr class="added_line"><td><pre>+  replicas: 3</pre></td></tr>
+	<tr class="normal_line"><td><pre>   selector:</pre></td></tr>
+	<tr class="normal_line"><td><pre>     matchLabels:</pre></td></tr>
+	<tr class="normal_line"><td><pre>       app: out-of-chart-values</pre></td></tr>
+	<tr class="normal_line"><td><pre>   template:</pre></td></tr>
+	<tr class="normal_line"><td><pre>     metadata:</pre></td></tr>
+	<tr class="normal_line"><td><pre>       labels:</pre></td></tr>
+	<tr class="normal_line"><td><pre>         app: out-of-chart-values</pre></td></tr>
+	<tr class="normal_line"><td><pre>     spec:</pre></td></tr>
+	<tr class="normal_line"><td><pre>       containers:</pre></td></tr>
+	<tr class="removed_line"><td><pre>-      - image: nginx:1.25</pre></td></tr>
+	<tr class="added_line"><td><pre>+      - image: nginx:1.27</pre></td></tr>
+	<tr class="normal_line"><td><pre>         name: app</pre></td></tr>
+	<tr class="normal_line"><td><pre>         ports:</pre></td></tr>
+	<tr class="normal_line"><td><pre>         - containerPort: 80</pre></td></tr>
+</table>
+</div>
+
+</details>
+</div>
+
+<pre>_Stats_:
+[Applications: 2], [Full Run: Xs], [Rendering: Xs], [Cluster: Xs], [Argo CD: Xs]</pre>
+</div>
+</body>
+</html>

integration-test/branch-18/target/output.md

@@ -0,0 +1,41 @@
+## Argo CD Diff Preview
+
+Summary:
+```yaml
+Modified (1):
+± out-of-chart-values (+2|-2)
+```
+
+<details>
+<summary>out-of-chart-values (examples/out-of-chart-values/application.yaml)</summary>
+<br>
+
+#### Deployment: default/out-of-chart-values
+```diff
+ apiVersion: apps/v1
+ kind: Deployment
+ metadata:
+   name: out-of-chart-values
+   namespace: default
+ spec:
+-  replicas: 2
++  replicas: 3
+   selector:
+     matchLabels:
+       app: out-of-chart-values
+   template:
+     metadata:
+       labels:
+         app: out-of-chart-values
+     spec:
+       containers:
+-      - image: nginx:1.25
++      - image: nginx:1.27
+         name: app
+         ports:
+         - containerPort: 80
+```
+</details>
+
+_Stats_:
+[Applications: 2], [Full Run: Xs], [Rendering: Xs], [Cluster: Xs], [Argo CD: Xs]

integration-test/integration_test.go

@@ -326,6 +326,19 @@ var testCases = []TestCase{
 		FileRegex:         "examples/app-of-apps/.*",
 		TraverseAppOfApps: "true",
 	},
+	// Regression test for a single-source local Helm chart whose helm.valueFiles
+	// reference a file OUTSIDE the chart directory via a repo-root-absolute path
+	// (/examples/out-of-chart-values/env/values.yaml). With repo-server-api the
+	// tool must stream the whole branch folder so the out-of-chart value file is
+	// reachable; otherwise the render fails with "no such file or directory".
+	{
+		Name:                       "branch-18/target",
+		TargetBranch:               "integration-test/branch-18/target",
+		BaseBranch:                 "integration-test/branch-18/base",
+		RenderMethod:               "repo-server-api",
+		FileRegex:                  "examples/out-of-chart-values/.*",
+		WatchIfNoWatchPatternFound: "false",
+	},
 	// This test verifies that disabling cluster roles without using the API fails.
 	// When createClusterRoles: false is set but --render-method=cli is used,
 	// the tool should fail because it can't access cluster resources via CLI.

pkg/reposerverextract/extract.go

@@ -17,6 +17,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -572,7 +573,10 @@ func buildManifestRequestForSource(
 	// files to the repo server, which can otherwise fail Argo CD's symlink safety
 	// checks before rendering even starts. If a path also contains a kustomization
 	// file, keep the branch root because Argo CD discovers it as Kustomize even
-	// when Chart.yaml exists for kustomize helmCharts support.
+	// when Chart.yaml exists for kustomize helmCharts support. We also keep the
+	// branch root when the chart's helm.valueFiles reference files outside the
+	// chart directory, because those files would otherwise be missing from the
+	// narrowed stream (see buildStreamDirForLocalSource).
 	//
 	// Special case: if the primary source has a Chart field (external Helm
 	// registry chart) there are no local files to stream. We signal this by
@@ -726,13 +730,74 @@ func buildStreamDirForLocalSource(branchFolder string, source v1alpha1.Applicati
 	if _, err := os.Stat(sourceDir); err != nil {
 		return "", nil, fmt.Errorf("failed to inspect source dir %q: %w", sourceDir, err)
 	}
-	if isLocalHelmChart(sourceDir) && !isKustomizeSource(sourceDir) {
+	if isLocalHelmChart(sourceDir) && !isKustomizeSource(sourceDir) && !hasOutOfChartValueFile(branchFolder, sourceDir, source) {
 		return sourceDir, nil, nil
 	}
 
 	return branchFolder, nil, nil
 }
 
+// hasOutOfChartValueFile reports whether any of the source's Helm valueFiles
+// resolves to a path outside the chart directory (sourceDir). Argo CD resolves
+// a leading-slash value file relative to the repository root and a relative
+// value file relative to the chart (app) directory, so a value file can escape
+// the chart directory either via an absolute "/path" or a relative "../path".
+//
+// When a value file lives outside the chart directory, narrowing the repo
+// server stream to only the chart directory would drop that file from the
+// tarball and the render would fail with "no such file or directory". In that
+// case the caller must keep streaming the whole branch folder so the
+// out-of-chart value files remain reachable. URL value files (e.g. https://…)
+// are remote and never force the branch root.
+func hasOutOfChartValueFile(branchFolder, sourceDir string, source v1alpha1.ApplicationSource) bool {
+	if source.Helm == nil {
+		return false
+	}
+
+	absSourceDir, err := filepath.Abs(sourceDir)
+	if err != nil {
+		// If we cannot reason about the path, fall back to the safe behaviour
+		// of streaming the whole branch folder.
+		return true
+	}
+	chartRoot := absSourceDir + string(os.PathSeparator)
+
+	for _, vf := range source.Helm.ValueFiles {
+		if vf == "" {
+			continue
+		}
+		// Skip $ref value files - this function is only reached on the fast
+		// path where there are no ref sources, but be defensive.
+		if strings.HasPrefix(vf, "$") {
+			continue
+		}
+		// Skip remote URL value files (e.g. https://...). A URL has a scheme
+		// and is fetched by the repo server, not read from the tarball.
+		if u, parseErr := url.Parse(vf); parseErr == nil && u.Scheme != "" {
+			continue
+		}
+
+		var resolved string
+		if filepath.IsAbs(vf) {
+			// Leading-slash value files are resolved relative to the repo root.
+			abs, absErr := filepath.Abs(branchFolder)
+			if absErr != nil {
+				return true
+			}
+			resolved = filepath.Join(abs, vf)
+		} else {
+			// Relative value files are resolved relative to the chart directory.
+			resolved = filepath.Join(absSourceDir, vf)
+		}
+
+		if resolved != absSourceDir && !strings.HasPrefix(resolved+string(os.PathSeparator), chartRoot) {
+			return true
+		}
+	}
+
+	return false
+}
+
 func isLocalHelmChart(sourceDir string) bool {
 	info, err := os.Stat(filepath.Join(sourceDir, "Chart.yaml"))
 	return err == nil && !info.IsDir()