From bce89956adfff67c8dbe2f968265142e2dd2b040 Mon Sep 17 00:00:00 2001
From: Sam Bostock <sam.bostock@shopify.com>
Date: Tue, 21 Oct 2025 01:45:54 -0400
Subject: [PATCH] Fix config parameter precedence for same-scope values

From `git config`'s documentation:

> The files are read in the order given above, with last value found taking precedence over values read earlier.

Currently, we discard values if we already have one with the same
priority. However, this gives us "first one wins" semantics, which is
incorrect.

By instead only discarding values if we already have one with a higher
priority, we can achieve "last one wins" semantics, matching `git`.

This is of particular importance because we use config values to resolve
the nickname and ruleset fields, so the value we resolve must match what
a user would expect when writing their config. For example, consider the
following git config:

    [otel.trace2]
      ruleset = dl:drop # Drop traces by default)

    [includeIf "hasconfig:remote.*.url:https://example.com/my-repo.git"]
      path = my-repo-tracing.gitconfig

and in my-repo-tracing.gitconfig

    [otel.trace2]
      ruleset = dl:verbose # Collect detailed traces for my-repo

Because we have "first one wins" semantics instead of git's "last one
wins" semantics, the override doesn't do anything and traces are
dropped unconditionally.
---
 evt_apply.go      |  4 +++-
 evt_apply_test.go | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/evt_apply.go b/evt_apply.go
index dea456c..0d661b9 100644
--- a/evt_apply.go
+++ b/evt_apply.go
@@ -582,9 +582,11 @@ func apply__def_param(tr2 *trace2Dataset, evt *TrEvent) (err error) {
 	_, havePrevVal := tr2.process.paramSetValues[key]
 	priCur, havePrevPri := tr2.process.paramSetPriorities[key]
 
-	if havePrevVal && havePrevPri && priNew <= priCur {
+	if havePrevVal && havePrevPri && priNew < priCur {
 		// We already have a value for this key with a higher
 		// priority, so ignore this value.
+		// Note: When priorities are equal, we accept the new value
+		// to match Git's "last one wins" behavior.
 		return nil
 	}
 
diff --git a/evt_apply_test.go b/evt_apply_test.go
index e698342..bdc3794 100644
--- a/evt_apply_test.go
+++ b/evt_apply_test.go
@@ -500,6 +500,41 @@ func Test_Dataset_DefParam_Scoped(t *testing.T) {
 	assert.Equal(t, tr2.process.paramSetValues["bar"], x_param_local_bar)
 }
 
+// Verify that when multiple def_param events are sent for the same parameter
+// with the SAME scope, the last one wins (matching Git's behavior).
+// This happens when using includeIf to include config files.
+func Test_Dataset_DefParam_SameScope_LastWins(t *testing.T) {
+
+	var events []string = []string{
+		x_make_version(),
+		x_make_start(),
+
+		// Multiple values for "ruleset" with the same scope (local).
+		// The last one should win, matching git's "last one wins" behavior.
+		x_make_def_param("local", "ruleset", "dl:drop"),
+		x_make_def_param("local", "ruleset", "dl:verbose"),
+
+		// Also test with global scope
+		x_make_def_param("global", "nickname", "first"),
+		x_make_def_param("global", "nickname", "second"),
+		x_make_def_param("global", "nickname", "last"),
+
+		x_make_atexit(), // Should be last
+	}
+
+	tr2, sufficient, _ := load_test_dataset(t, events)
+	assert.True(t, sufficient, "have sufficient data")
+
+	assert.Equal(t, len(tr2.process.paramSetValues), 2)
+
+	// For same-priority configs, the LAST one should win (not first)
+	assert.NotNil(t, tr2.process.paramSetValues["ruleset"])
+	assert.Equal(t, tr2.process.paramSetValues["ruleset"], "dl:verbose")
+
+	assert.NotNil(t, tr2.process.paramSetValues["nickname"])
+	assert.Equal(t, tr2.process.paramSetValues["nickname"], "last")
+}
+
 func Test_Dataset_ChildProcesses(t *testing.T) {
 
 	var events []string = []string{