fix(dynamic-routing): stop scrambling and truncating dynamic-agent launch prompts (#3741)

Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
2026-09-17 21:33:58 +08:00
parent 3faf244b1d
commit b99ac97a24
9 changed files with 422 additions and 11 deletions
@@ -661,11 +661,13 @@ func boundedConversation(userMessages []string, conversation string) string {
 	}
 }

-// ContinuationPrompt renders a bounded, provider-neutral handoff. The
-// original prompt remains first so providers that do not understand the
-// optional package still receive the user's request.
+// ContinuationPrompt renders a provider-neutral handoff. The original prompt
+// remains first, credential-redacted but otherwise verbatim and unbounded,
+// so providers that do not understand the optional package still receive
+// the user's request whole; the continuation package appended after it is
+// bounded.
 func ContinuationPrompt(prompt string, continuation Continuation) string {
-	prompt = strings.TrimSpace(routingerr.Sanitize(prompt))
+	prompt = strings.TrimSpace(routingerr.SanitizeCredentialsUnbounded(prompt))
 	continuation = sanitizeContinuation(continuation)
 	fields := make([]string, 0, 7)
 	if continuation.TaskDescription != "" {
@@ -126,6 +126,70 @@ func TestConductorSanitizesPromptBeforeEveryDownstreamLaunch(t *testing.T) {
 	}
 }

+// TestConductorPreservesLongUserAuthoredPromptOnEveryDownstreamLaunch proves
+// the primary launch prompt receives credential-only redaction, unbounded,
+// on both the first attempt and every fallback: a path, a commit SHA, a PR
+// URL and a task UUID all survive intact even past MaxRawExcerptBytes, while
+// a credential is still redacted.
+func TestConductorPreservesLongUserAuthoredPromptOnEveryDownstreamLaunch(t *testing.T) {
+	profile := Profile{
+		ID: "dynamic-profile",
+		Candidates: []Candidate{
+			{
+				ID:         "candidate-first",
+				Enabled:    true,
+				BindingKey: "first",
+				Rules:      map[string]Action{string(routingerr.CodeProviderUnavailable): ActionTryNext},
+			},
+			{ID: "candidate-second", Enabled: true, BindingKey: "second"},
+		},
+	}
+	const secret = "sk-abcdEFGH12345678ijklMNOPqrstUVWX"
+	const path = "/Users/alice/work/repo/main.go"
+	const sha = "94e7b02458b6c1a2d3e4f5061728394a5b6c7d8"
+	const url = "https://example.com/org/repo/pull/123?tab=files#discussion_r42"
+	const uuid = "3f9a1c2e-4b5d-4e6f-8a9b-0c1d2e3f4a5b"
+	filler := strings.Repeat("Then update the handler so it returns a typed error. ", 100)
+	prompt := filler + "Fix bug in " + path + " at commit " + sha + "; see " + url +
+		" and task " + uuid + ".\n API_KEY=" + secret
+
+	if len(prompt) <= routingerr.MaxRawExcerptBytes {
+		t.Fatalf("test prompt must exceed MaxRawExcerptBytes=%d, got %d", routingerr.MaxRawExcerptBytes, len(prompt))
+	}
+
+	downstream := &conductorTestDownstream{}
+	conductor := NewConductor(NewEngine(), conductorTestProfileLoader{profile: profile}, downstream)
+
+	_, err := conductor.Launch(context.Background(), ConductorLaunch{
+		SessionID:        "session-long-prompt",
+		LogicalProfileID: profile.ID,
+		Prompt:           prompt,
+		PriorACPSession:  "acp-first",
+	})
+	if err != nil {
+		t.Fatalf("Launch: %v", err)
+	}
+	if len(downstream.launches) != 2 {
+		t.Fatalf("launch count = %d, want 2", len(downstream.launches))
+	}
+	for i, launch := range downstream.launches {
+		if !strings.HasPrefix(launch.Prompt, "Then update the handler") {
+			t.Fatalf("launch %d prompt did not survive whole: %q", i, launch.Prompt)
+		}
+		for _, want := range []string{path, sha, url, uuid} {
+			if !strings.Contains(launch.Prompt, want) {
+				t.Fatalf("launch %d prompt lost %q: %q", i, want, launch.Prompt)
+			}
+		}
+		if strings.Contains(launch.Prompt, secret) {
+			t.Fatalf("launch %d prompt retained the raw secret: %q", i, launch.Prompt)
+		}
+		if !strings.Contains(launch.Prompt, "API_KEY: ***") {
+			t.Fatalf("launch %d prompt was not redacted, just dropped: %q", i, launch.Prompt)
+		}
+	}
+}
+
 type recordingConductorTestDownstream struct {
 	launches []DownstreamLaunch
 }
@@ -411,6 +411,15 @@ func SanitizeCredentials(s string) string {
 	return applyRedactions(s, credentialRedactions)
 }

+// SanitizeCredentialsUnbounded applies the same rule set as
+// SanitizeCredentials but without the MaxRawExcerptBytes output cap. Use
+// this for user-authored text that must reach its destination whole, such
+// as a task's primary launch prompt, where truncation would silently drop
+// instructions rather than only redact credentials.
+func SanitizeCredentialsUnbounded(s string) string {
+	return applyRedactionsUnbounded(s, credentialRedactions)
+}
+
 // SanitizeFullUnbounded applies the same rule set as Sanitize but without the
 // MaxRawExcerptBytes output cap. Use this only when the caller will apply its
 // own tail truncation afterward (see dynamic.sanitizedTail): sanitizing
@@ -631,6 +631,84 @@ func TestSanitizeCredentials_IdempotentFuzz(t *testing.T) {
 	}
 }

+// TestSanitizeCredentialsUnbounded_PreservesLengthAndContent proves the
+// unbounded credential tier used for the primary launch prompt neither
+// truncates nor mangles non-credential content, on an input well past
+// MaxRawExcerptBytes.
+func TestSanitizeCredentialsUnbounded_PreservesLengthAndContent(t *testing.T) {
+	body := strings.Repeat(
+		"Fix bug in /Users/alice/work/repo/main.go at commit "+
+			"94e7b02458b6c1a2d3e4f5061728394a5b6c7d8; see "+
+			"https://example.com/org/repo/pull/123?tab=files#discussion_r42 "+
+			"and task 3f9a1c2e-4b5d-4e6f-8a9b-0c1d2e3f4a5b. ",
+		100,
+	)
+	if len(body) <= MaxRawExcerptBytes {
+		t.Fatalf("test input must exceed MaxRawExcerptBytes=%d, got %d", MaxRawExcerptBytes, len(body))
+	}
+
+	got := SanitizeCredentialsUnbounded(body)
+
+	if len(got) != len(body) {
+		t.Fatalf("expected unbounded output length %d, got %d", len(body), len(got))
+	}
+	if got != body {
+		t.Fatalf("expected non-credential content to survive unchanged")
+	}
+	for _, want := range []string{
+		"/Users/alice/work/repo/main.go",
+		"94e7b02458b6c1a2d3e4f5061728394a5b6c7d8",
+		"https://example.com/org/repo/pull/123?tab=files#discussion_r42",
+		"3f9a1c2e-4b5d-4e6f-8a9b-0c1d2e3f4a5b",
+	} {
+		if !strings.Contains(got, want) {
+			t.Fatalf("expected output to retain %q, got %q", want, got)
+		}
+	}
+}
+
+// TestSanitizeCredentialsUnbounded_RedactsCredentials proves the unbounded
+// credential tier still redacts credential-shaped patterns, matching
+// SanitizeCredentials' behavior minus truncation.
+func TestSanitizeCredentialsUnbounded_RedactsCredentials(t *testing.T) {
+	cases := []struct {
+		name        string
+		in          string
+		mustNotHave string
+		mustHave    string
+	}{
+		{
+			name:        "anthropic-style key",
+			in:          "use sk-abcdef1234567890QQQQ to call",
+			mustNotHave: "sk-abcdef1234567890",
+			mustHave:    "sk-***",
+		},
+		{
+			name:        "github classic pat",
+			in:          "token ghp_AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA leaks",
+			mustNotHave: "ghp_AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
+			mustHave:    "ghp_***",
+		},
+		{
+			name:        "api key assignment",
+			in:          "API_KEY=sk-abcdEFGH12345678ijklMNOPqrstUVWX in the env",
+			mustNotHave: "sk-abcdEFGH12345678ijklMNOPqrstUVWX",
+			mustHave:    "API_KEY: ***",
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			got := SanitizeCredentialsUnbounded(c.in)
+			if strings.Contains(got, c.mustNotHave) {
+				t.Fatalf("expected %q to be redacted, got %q", c.mustNotHave, got)
+			}
+			if !strings.Contains(got, c.mustHave) {
+				t.Fatalf("expected %q in output, got %q", c.mustHave, got)
+			}
+		})
+	}
+}
+
 // TestSanitize_TruncatesOnRuneBoundary proves Sanitize never splits a
 // multi-byte rune when cutting to MaxRawExcerptBytes. Vietnamese (3-byte) and
 // CJK (3-byte) runes are repeated at every byte alignment relative to