fix(secrets): address fifth-round Rust PR review feedback on #624

Three concrete issues from the latest review pass:

1. `normalize_source_path` double-escaped `"` in the JSON marker.
   The helper escaped `"` -> `\"` before the path reached
   `serde_json::json!`, which then escaped the backslash again. The
   marker stored `"source":"agents/foo\\\"bar.md"` instead of the
   canonical `"source":"agents/foo\"bar.md"` — every round-trip
   carried a spurious backslash. Move the `"` -> `\"` escape into
   `generate_header_comment` (the only YAML-comment surface that
   needs it) and leave `normalize_source_path` returning the
   canonical form for the JSON marker and the `--source` filter.

2. `is_direct_match` false-positive for same-stem in different
   directories. The previous `yaml_normalized.ends_with("/{stem}")`
   branch would label an unrelated pipeline `Direct` when its
   `yamlFilename` happened to share the same trailing
   `<stem>.lock.yml` (e.g. marker `agents/foo.md` matched
   `other/agents/foo.lock.yml`). Both sides are already normalised
   to repo-root-relative paths without a leading slash, so strict
   equality is the correct check. Updated test name and added a
   regression test for the same-stem-different-directory case.

3. Documented case-sensitivity of `--source` in CLI help and the
   discovery module's doc comment so Windows users don't get silent
   zero-result runs from a case mismatch.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: jamesadevine

src/ado/discovery.rs

@@ -491,6 +491,16 @@ fn is_direct_match(def: &DefinitionSummary, markers: &[MarkerMetadata]) -> bool
     // the definition's root YAML. Convention: `<stem>.md` compiles to
     // `<stem>.lock.yml`.
     //
+    // Equality is required — an earlier version also accepted
+    // `yaml_normalized.ends_with("/{stem}")` for a defensive
+    // tail-match, but that produced false-positives when an unrelated
+    // pipeline happened to live under a same-named lock file in a
+    // different directory (e.g. marker `agents/foo.md` + yamlFilename
+    // `other/agents/foo.lock.yml` would mislabel a Consumer as Direct).
+    // Both `marker.source` and the post-`normalize_ado_yaml_path`
+    // form of `yaml_filename` are repo-root-relative without a leading
+    // slash, so strict equality is the correct check.
+    //
     // Non-`.md` sources are treated conservatively as `Consumer`: this
     // branch is unreachable today (the compiler always emits `.md`
     // source paths) but stays defensive against future extensions that
@@ -506,7 +516,7 @@ fn is_direct_match(def: &DefinitionSummary, markers: &[MarkerMetadata]) -> bool
     else {
         return false;
     };
-    yaml_normalized == stem || yaml_normalized.ends_with(&format!("/{stem}"))
+    yaml_normalized == stem
 }
 
 async fn parse_local_lock(path: &Path) -> Option<MarkerMetadata> {
@@ -597,6 +607,12 @@ pub fn discovered_to_matched(d: &DiscoveredPipeline) -> Option<MatchedDefinition
 /// `source_filter` filters discovery results so only definitions whose
 /// markers reference that source path are kept. Match is by exact
 /// equality on the normalized source string in the marker JSON.
+/// Normalisation is applied to the user-supplied value too
+/// ([`crate::compile::normalize_source_path`] — forward-slash separators,
+/// CR/LF stripping, leading `./` collapsed) so the common variants
+/// (`./agents/foo.md`, `agents\foo.md` on Windows) match. Matching is
+/// **case-sensitive** even on Windows; pass the path in the same case
+/// it was compiled with.
 ///
 /// Skip-summary warnings are emitted differently depending on whether
 /// `source_filter` is active:
@@ -833,10 +849,10 @@ mod tests {
     }
 
     #[test]
-    fn direct_when_yaml_filename_has_extra_path_prefix() {
-        // ADO sometimes stores yamlFilename with a project-relative
-        // leading slash + extra path components. The marker source is
-        // just the markdown path the user passed at compile time.
+    fn direct_when_yaml_filename_has_leading_slash() {
+        // ADO sometimes returns yamlFilename with a leading slash. The
+        // `normalize_ado_yaml_path` helper strips it, so equality with
+        // the derived `<stem>.lock.yml` still holds.
         let def = def_with(1, "a", Some("/agents/foo.lock.yml"), None);
         let markers = vec![MarkerMetadata {
             schema: 1,
@@ -847,6 +863,24 @@ mod tests {
         assert!(is_direct_match(&def, &markers));
     }
 
+    #[test]
+    fn consumer_when_same_stem_in_different_directory() {
+        // Regression: previously `yaml_normalized.ends_with("/{stem}")`
+        // would mislabel a Consumer pipeline as Direct whenever a
+        // same-named lock file lived under any unrelated prefix
+        // (e.g. marker `agents/foo.md` + yamlFilename
+        // `other/agents/foo.lock.yml`). The fix requires strict
+        // equality after normalisation.
+        let def = def_with(1, "a", Some("other/agents/foo.lock.yml"), None);
+        let markers = vec![MarkerMetadata {
+            schema: 1,
+            source: "agents/foo.md".to_string(),
+            version: "0.30.0".to_string(),
+            target: "standalone".to_string(),
+        }];
+        assert!(!is_direct_match(&def, &markers));
+    }
+
     #[test]
     fn consumer_when_yaml_filename_does_not_match_marker() {
         let def = def_with(1, "a", Some("/release-readiness.yml"), None);

src/compile/common.rs

@@ -1219,19 +1219,26 @@ pub const HEADER_MARKER: &str = "# @ado-aw";
 /// The source path is the input path as provided to the compiler (e.g., `agents/my-agent.md`,
 /// `.azdo/pipelines/review.md`, or any other location the user chose). Path separators
 /// are normalized to forward slashes for cross-platform consistency.
-/// Normalise a source markdown path for embedding in generated YAML.
+/// Normalise a source markdown path into the canonical form embedded
+/// in the `# ado-aw-metadata:` JSON marker.
 ///
-/// Applies the same transformations the `# @ado-aw` comment header uses
-/// (forward-slash separators, newline/CR stripping, `"` escaping, leading
-/// `./` collapsed). Shared between [`generate_header_comment`] and the
-/// always-on `ado-aw-marker` compiler extension so both surfaces agree on
-/// the canonical form of a source path.
+/// Applies forward-slash separator normalisation, strips CR/LF, and
+/// collapses any leading `./`. Does **not** escape `"` — JSON encoding
+/// is the caller's job (`serde_json::json!` handles it for the marker;
+/// [`generate_header_comment`] escapes for its YAML comment surface).
+///
+/// Shared between the always-on `ado-aw-marker` compiler extension and
+/// the `--source` filter in Preview-driven discovery so both sides
+/// agree on the canonical form of a source path. Previously this also
+/// escaped `"`, but that produced a literal `\"` inside the marker
+/// JSON (serde_json then escaped the leading backslash on top, storing
+/// a spurious extra backslash on every round-trip). The header
+/// comment now applies its own escape inline.
 pub fn normalize_source_path(input_path: &std::path::Path) -> String {
     let mut source_path = input_path
         .to_string_lossy()
         .replace('\\', "/")
-        .replace(['\n', '\r'], "")
-        .replace('"', "\\\"");
+        .replace(['\n', '\r'], "");
 
     // Strip redundant leading "./" prefixes to prevent accumulation when
     // compile_all_pipelines re-joins paths through Path::new(".").join(source).
@@ -1244,7 +1251,11 @@ pub fn normalize_source_path(input_path: &std::path::Path) -> String {
 
 pub fn generate_header_comment(input_path: &std::path::Path) -> String {
     let version = env!("CARGO_PKG_VERSION");
-    let source_path = normalize_source_path(input_path);
+    // The header comment embeds the source inside double quotes
+    // (`source="…"`); escape `"` so legacy `parse_header_line` consumers
+    // can still recover the original. The JSON marker does not need
+    // this — serde_json escapes JSON-meaningful chars on its own.
+    let source_path = normalize_source_path(input_path).replace('"', "\\\"");
 
     format!(
         "# This file is auto-generated by ado-aw. Do not edit manually.\n\

src/compile/extensions/ado_aw_marker.rs

@@ -270,4 +270,39 @@ mod tests {
             "expected `##vso[` neutralised via canonical backtick-wrap in echo line: {echo_line}"
         );
     }
-}
+
+    #[test]
+    fn json_marker_quote_in_source_round_trips_correctly() {
+        // Regression: `normalize_source_path` previously escaped `"` to
+        // `\"` before embedding the path. `serde_json::json!` then
+        // double-encoded the backslash, so the marker JSON looked like
+        // `"source":"agents/foo\\\"bar.md"` — and the path returned by
+        // `parse_marker_step` carried a spurious `\` that did not exist
+        // in the original filename. The fix is to feed the canonical
+        // (unescaped) path into the JSON value and let serde_json do
+        // the JSON-level escaping.
+        let fm = parse_fm("name: t\ndescription: x\n");
+        let input_path = Path::new(r#"agents/foo"bar.md"#);
+        let ctx = CompileContext {
+            agent_name: &fm.name,
+            front_matter: &fm,
+            ado_context: None,
+            engine: crate::engine::Engine::Copilot,
+            compile_dir: None,
+            input_path: Some(input_path),
+        };
+        let steps = AdoAwMarkerExtension.setup_steps(&ctx).unwrap();
+        assert_eq!(steps.len(), 1);
+
+        // Parse the marker step back via the canonical discovery parser
+        // and confirm the source field reconstructs to the original
+        // path (forward-slash-normalised, no spurious backslashes).
+        let parsed = crate::detect::parse_marker_step(&steps[0]);
+        assert_eq!(parsed.len(), 1, "expected exactly one marker in step");
+        assert_eq!(
+            parsed[0].source,
+            r#"agents/foo"bar.md"#,
+            "marker source should round-trip without spurious backslash"
+        );
+    }
+}

src/main.rs

@@ -72,7 +72,9 @@ enum SecretsCmd {
         /// ado-aw template (e.g. `agents/security-scan.md`). Activates
         /// the discovery code path. **Without `--all-repos`, only
         /// definitions in the current repository are searched** — pair
-        /// with `--all-repos` to search the full project.
+        /// with `--all-repos` to search the full project. Path matching
+        /// is case-sensitive and forward-slash-normalised; on Windows,
+        /// pass the path in the same case it was compiled with.
         #[arg(long, conflicts_with = "definition_ids")]
         source: Option<String>,
     },
@@ -95,7 +97,9 @@ enum SecretsCmd {
         all_repos: bool,
         /// Filter discovered definitions to consumers of one specific
         /// ado-aw template. **Without `--all-repos`, only definitions
-        /// in the current repository are searched.**
+        /// in the current repository are searched.** Path matching is
+        /// case-sensitive and forward-slash-normalised; on Windows,
+        /// pass the path in the same case it was compiled with.
         #[arg(long, conflicts_with = "definition_ids")]
         source: Option<String>,
     },
@@ -119,7 +123,9 @@ enum SecretsCmd {
         all_repos: bool,
         /// Filter discovered definitions to consumers of one specific
         /// ado-aw template. **Without `--all-repos`, only definitions
-        /// in the current repository are searched.**
+        /// in the current repository are searched.** Path matching is
+        /// case-sensitive and forward-slash-normalised; on Windows,
+        /// pass the path in the same case it was compiled with.
         #[arg(long, conflicts_with = "definition_ids")]
         source: Option<String>,
     },