From d5328c7ba0946af9874e5687261e41bdb54d1929 Mon Sep 17 00:00:00 2001
From: asto18089 <asto18089@126.com>
Date: Tue, 22 Sep 2026 02:51:59 +0800
Subject: [PATCH] fix: relativize context source labels to repo root

Chain-segment ("<!-- scoped instructions: ... -->") and
<project_rule source=...> labels carried absolute paths into the
pinned system prompt, so a checkout move or recase changed the prompt
prefix and could emit a spurious <context_update> history append.
Render both labels repo-relative (git-root-relative, forward
slashes), falling back to workspace-relative for rules outside any
checkout and to the absolute spelling only for paths outside the
label root (unreachable by construction).

Pins Pinvou/pinvou-agent#514.

Signed-off-by: asto18089 <asto18089@126.com>
---
 crates/tui/src/project_context.rs       | 251 ++++++++++++++++++++++--
 crates/tui/src/project_context/types.rs |  23 +++
 2 files changed, 258 insertions(+), 16 deletions(-)

diff --git a/crates/tui/src/project_context.rs b/crates/tui/src/project_context.rs
index 8dec398656..1040fc004c 100644
--- a/crates/tui/src/project_context.rs
+++ b/crates/tui/src/project_context.rs
@@ -29,6 +29,7 @@ pub use self::pack::generate_project_context_pack;
 pub use self::types::ProjectContext;
 use self::types::ProjectContextError;
 pub(crate) use self::types::project_instructions_source_label;
+use self::types::repo_relative_source_label;
 
 /// Names of project context files to look for, in priority order.
 ///
@@ -511,6 +512,19 @@ pub(crate) fn load_project_context_with_imports(
     // Each rule file is wrapped in a <project_rule> block and appended after
     // the main instructions content. Security model: same as AGENTS.md —
     // workspace-contained content only, no absolute-path escape.
+    //
+    // The source label sits inside the pinned system prompt, so it is
+    // rendered repo-relative (forward slashes) and a checkout move or
+    // recase leaves the block byte-identical (#514). `load_rules_from_dir`
+    // only returns paths under `workspace`, so workspace-relative is a
+    // safe fallback spelling when no git root exists; the absolute
+    // spelling remains only for a path outside the root, which is not
+    // reachable by construction.
+    // The label root does not depend on the loop below, so it is computed
+    // once up front: `repo_relative_source_label` must never render a rule
+    // before its root exists, and a `None` sentinel would silently degrade
+    // the label to the absolute spelling.
+    let rules_label_root = find_git_root(workspace).unwrap_or_else(|| workspace.to_path_buf());
     let mut rules_content = String::new();
     for rules_dir in rules_dirs_for(imports) {
         let rules = load_rules_from_dir(workspace, rules_dir);
@@ -520,7 +534,7 @@ pub(crate) fn load_project_context_with_imports(
             }
             rules_content.push_str(&format!(
                 "<project_rule source=\"{}\">\n{}\n</project_rule>",
-                path.display(),
+                repo_relative_source_label(&path, Some(rules_label_root.as_path())),
                 content.trim()
             ));
         }
@@ -632,13 +646,20 @@ fn load_project_context_with_parents_and_home_imports(
     imports: &ForeignInstructionImports,
 ) -> ProjectContext {
     let workspace_canonical = canonicalize_workspace_or_keep(workspace);
+    // Chain-segment labels sit inside the pinned system prompt, so they are
+    // rendered repo-relative: a checkout move or recase leaves the labels —
+    // and therefore the whole prompt prefix — byte-identical (#514). The
+    // chain never leaves the checkout (`context_chain_dirs`), so every
+    // labeled path strips cleanly; without a git root the chain is a single
+    // workspace segment that never emits a label at all.
+    let git_root = find_git_root(&workspace_canonical);
     let mut ctx = load_project_context_with_imports(&workspace_canonical, imports);
 
     // Assemble the repository-root → workspace instruction chain. The chain
     // directories come from Git traversal of the containing checkout, so a
     // linked worktree contributes its own root and files above the root —
     // other checkouts, unrelated parents — stay out of scope.
-    let chain_dirs = context_chain_dirs(&workspace_canonical, home_dir);
+    let chain_dirs = context_chain_dirs(&workspace_canonical, git_root.as_deref(), home_dir);
     // `chain_dirs` is ordered root → workspace; the workspace itself is the
     // last entry and was already loaded above.
     let ancestor_dirs = &chain_dirs[..chain_dirs.len().saturating_sub(1)];
@@ -663,7 +684,7 @@ fn load_project_context_with_parents_and_home_imports(
         let mut assembled = String::new();
 
         for (path, content) in &ancestor_docs {
-            append_chain_segment(&mut assembled, path, content);
+            append_chain_segment(&mut assembled, path, content, git_root.as_deref());
         }
 
         // The workspace's own file is the most specific link: it reads last,
@@ -674,7 +695,7 @@ fn load_project_context_with_parents_and_home_imports(
                 .source_path
                 .clone()
                 .unwrap_or_else(|| workspace_canonical.clone());
-            append_chain_segment(&mut assembled, &path, &content);
+            append_chain_segment(&mut assembled, &path, &content, git_root.as_deref());
         } else if let Some((path, _)) = ancestor_docs.last() {
             // No workspace-level file: the nearest ancestor is the most
             // specific source.
@@ -757,7 +778,8 @@ pub(crate) fn project_context_cache_candidate_paths(
     // invalidate the cache too. Changing the opt-in set clears the cache
     // outright (`set_foreign_instruction_imports`), so over-enumerating here
     // only ever costs an extra reload.
-    for dir in context_chain_dirs(&workspace, home_dir) {
+    let repo_root = find_git_root(&workspace);
+    for dir in context_chain_dirs(&workspace, repo_root.as_deref(), home_dir) {
         for filename in PROJECT_CONTEXT_FILES {
             paths.push(dir.join(filename));
         }
@@ -905,12 +927,20 @@ fn is_git_metadata_entry(path: &Path) -> bool {
 /// repository root down to the workspace (inclusive).
 ///
 /// Repository identity comes from the containing checkout itself
-/// ([`find_git_root`]); the chain never crosses the repository boundary, so
-/// sibling checkouts and unrelated parents stay out of scope. Outside any
-/// repository only the workspace itself is searched. When `home_dir` is an
-/// ancestor it remains an outer boundary the walk never leaves.
-fn context_chain_dirs(workspace: &Path, home_dir: Option<&Path>) -> Vec<PathBuf> {
-    let mut stop = find_git_root(workspace).unwrap_or_else(|| workspace.to_path_buf());
+/// ([`find_git_root`]), passed in as `repo_root` so the chain bounds and the
+/// chain-segment labels are derived from one and the same walk; the chain
+/// never crosses the repository boundary, so sibling checkouts and unrelated
+/// parents stay out of scope. Outside any repository (`repo_root` is `None`)
+/// only the workspace itself is searched. When `home_dir` is an ancestor it
+/// remains an outer boundary the walk never leaves.
+fn context_chain_dirs(
+    workspace: &Path,
+    repo_root: Option<&Path>,
+    home_dir: Option<&Path>,
+) -> Vec<PathBuf> {
+    let mut stop = repo_root
+        .map(Path::to_path_buf)
+        .unwrap_or_else(|| workspace.to_path_buf());
 
     if let Some(home) = home_dir {
         let home = canonicalize_workspace_or_keep(home);
@@ -942,12 +972,20 @@ fn context_chain_dirs(workspace: &Path, home_dir: Option<&Path>) -> Vec<PathBuf>
 /// The first segment is the file's raw content (a single-file chain stays
 /// byte-identical to a plain load); every later segment is prefixed with a
 /// provenance label so the model can tell the scopes apart, wider scopes
-/// first and the workspace last.
-fn append_chain_segment(assembled: &mut String, path: &Path, content: &str) {
+/// first and the workspace last. The label is repo-relative (forward
+/// slashes) so the pinned system prompt stays stable across checkout moves
+/// and recasings (#514); filenames alone would collide, since chain segments
+/// legitimately share the `AGENTS.md` basename.
+fn append_chain_segment(
+    assembled: &mut String,
+    path: &Path,
+    content: &str,
+    repo_root: Option<&Path>,
+) {
     if !assembled.is_empty() {
         assembled.push_str(&format!(
             "\n\n<!-- scoped instructions: {} (overrides wider scopes where they conflict) -->\n",
-            path.display()
+            repo_relative_source_label(path, repo_root)
         ));
     }
     assembled.push_str(content);
@@ -1417,8 +1455,8 @@ mod tests {
         // instructions block byte-identical, so a move emits no spurious
         // `<context_update>` history append and no absolute path enters a
         // provider-bound label. Scope note: ancestor-chain and project-rule
-        // labels keep their own (absolute) spellings; relativizing those is
-        // a separate decision.
+        // labels are repo-relative for the same reason (`#514`); this test
+        // pins the file-name spelling of the workspace-level block.
         let dir_a = tempdir().expect("tempdir a");
         let dir_b = tempdir().expect("tempdir b");
         fs::write(dir_a.path().join("AGENTS.md"), "Pinned content").expect("write a");
@@ -1441,6 +1479,175 @@ mod tests {
         );
     }
 
+    #[test]
+    fn repo_relative_source_label_falls_back_when_path_is_outside_root() {
+        // The absolute fallback is an escape hatch for a path the loader
+        // cannot place under the checkout — unreachable by construction —
+        // so it must degrade to a displayable spelling, never panic or
+        // invent a wrong relative label.
+        assert_eq!(
+            repo_relative_source_label(
+                Path::new("/repo/crates/tui/AGENTS.md"),
+                Some(Path::new("/repo"))
+            ),
+            "crates/tui/AGENTS.md"
+        );
+        // On unix the backslashes are literal filename bytes, on Windows they
+        // are separators — either way the label must come out forward-slashed,
+        // and this assertion is the only check that can see it off Windows
+        // (the fork's PR CI has no Windows leg).
+        assert_eq!(
+            repo_relative_source_label(
+                Path::new("/repo/crates\\tui\\AGENTS.md"),
+                Some(Path::new("/repo"))
+            ),
+            "crates/tui/AGENTS.md",
+            "backslash separators are normalized to forward slashes"
+        );
+        assert_eq!(
+            repo_relative_source_label(Path::new("/elsewhere/AGENTS.md"), Some(Path::new("/repo"))),
+            "/elsewhere/AGENTS.md",
+            "a path outside the root falls back to the absolute spelling"
+        );
+        assert_eq!(
+            repo_relative_source_label(Path::new("/repo/AGENTS.md"), None),
+            "/repo/AGENTS.md",
+            "a missing root falls back to the absolute spelling"
+        );
+    }
+
+    #[test]
+    fn chain_segment_labels_are_repo_relative_not_absolute() {
+        let tmp = tempdir().expect("tempdir");
+        let home = tempdir().expect("home tempdir");
+
+        // A two-level chain inside one checkout: the repo root carries the
+        // wide scope, `crates/tui` the workspace scope.
+        let repo = tmp.path().join("repo");
+        fs::create_dir_all(repo.join(".git")).expect("mkdir .git");
+        fs::write(repo.join(".git").join("HEAD"), "ref: refs/heads/main\n").expect("write HEAD");
+        fs::write(repo.join("AGENTS.md"), "ROOT-SCOPE instructions").expect("write root agents");
+
+        let workspace = repo.join("crates").join("tui");
+        fs::create_dir_all(&workspace).expect("mkdir workspace");
+        fs::write(workspace.join("AGENTS.md"), "WORKSPACE-SCOPE instructions")
+            .expect("write workspace agents");
+
+        let ctx = load_project_context_with_parents_and_home(&workspace, Some(home.path()));
+        let instructions = ctx.instructions.as_ref().expect("chain instructions");
+
+        assert!(instructions.contains("ROOT-SCOPE instructions"));
+        assert!(instructions.contains("WORKSPACE-SCOPE instructions"));
+        assert!(
+            instructions.contains("<!-- scoped instructions: crates/tui/AGENTS.md"),
+            "the chain segment label must be repo-relative, got: {instructions}"
+        );
+        assert!(
+            !instructions.contains(&tmp.path().display().to_string()),
+            "absolute paths must not enter chain segment labels: {instructions}"
+        );
+    }
+
+    #[test]
+    fn project_rule_labels_are_repo_relative_not_absolute() {
+        let tmp = tempdir().expect("tempdir");
+        let repo = tmp.path().join("repo");
+        fs::create_dir_all(repo.join(".git")).expect("mkdir .git");
+        fs::write(repo.join(".git").join("HEAD"), "ref: refs/heads/main\n").expect("write HEAD");
+        let rules_dir = repo.join(".codewhale/rules");
+        fs::create_dir_all(&rules_dir).expect("mkdir rules");
+        fs::write(
+            rules_dir.join("security.md"),
+            "# Security\nNo hardcoded secrets.",
+        )
+        .expect("write rule");
+
+        let ctx = load_project_context(&repo);
+        let rules = ctx.rules_block.as_ref().expect("rules block");
+
+        assert!(
+            rules.contains("<project_rule source=\".codewhale/rules/security.md\">"),
+            "the rule label must be repo-relative, got: {rules}"
+        );
+        assert!(
+            !rules.contains(&tmp.path().display().to_string()),
+            "absolute paths must not enter rule source labels: {rules}"
+        );
+    }
+
+    #[test]
+    fn prompt_block_is_pinned_across_checkout_moves() {
+        // The whole reason for repo-relative labels: the same repository
+        // tree checked out at two different absolute locations must render
+        // byte-identical system blocks (chain segment + rules included), so
+        // a move or recase neither appends a spurious `<context_update>`
+        // history entry nor invalidates the provider's prompt KV prefix.
+        let mut moved_block: Option<String> = None;
+        for location in ["checkout-a", "checkout-b"] {
+            let root = tempdir().expect(location);
+            let repo = root.path().join("repo");
+            fs::create_dir_all(repo.join(".git")).expect("mkdir .git");
+            fs::write(repo.join(".git").join("HEAD"), "ref: refs/heads/main\n")
+                .expect("write HEAD");
+            fs::write(repo.join("AGENTS.md"), "ROOT-SCOPE instructions").expect("write root");
+
+            let workspace = repo.join("nested");
+            fs::create_dir_all(&workspace).expect("mkdir nested");
+            fs::write(workspace.join("AGENTS.md"), "NESTED-SCOPE instructions")
+                .expect("write nested");
+            // Rules are discovered at workspace level, so the checkout tree
+            // must carry them under the session's workspace too.
+            let rules_dir = workspace.join(".codewhale/rules");
+            fs::create_dir_all(&rules_dir).expect("mkdir rules");
+            fs::write(rules_dir.join("security.md"), "No hardcoded secrets.").expect("write rule");
+
+            let ctx = load_project_context_with_parents_and_home(&workspace, None);
+            let block = ctx.as_system_block().expect("system block");
+
+            assert!(block.contains("<!-- scoped instructions: nested/AGENTS.md"));
+            assert!(
+                block.contains("<project_rule source=\"nested/.codewhale/rules/security.md\">"),
+                "rule label must be repo-relative, got: {block}"
+            );
+            assert!(
+                !block.contains(&root.path().display().to_string()),
+                "absolute paths must not enter the pinned prompt block: {block}"
+            );
+            if location == "checkout-a" {
+                moved_block = Some(block);
+            } else {
+                assert_eq!(
+                    moved_block.as_deref(),
+                    Some(block.as_str()),
+                    "the same tree at two locations must render byte-identical blocks"
+                );
+            }
+        }
+    }
+
+    #[test]
+    fn project_rule_labels_fall_back_to_workspace_relative_without_git_root() {
+        // Outside any repository there is no root to relativize against;
+        // the label falls back to workspace-relative spelling, which stays
+        // stable as long as the workspace tree itself is unchanged.
+        let tmp = tempdir().expect("tempdir");
+        let rules_dir = tmp.path().join(".codewhale/rules");
+        fs::create_dir_all(&rules_dir).expect("mkdir rules");
+        fs::write(rules_dir.join("security.md"), "No hardcoded secrets.").expect("write rule");
+
+        let ctx = load_project_context(tmp.path());
+        let rules = ctx.rules_block.as_ref().expect("rules block");
+
+        assert!(
+            rules.contains("<project_rule source=\".codewhale/rules/security.md\">"),
+            "the rule label must fall back to workspace-relative, got: {rules}"
+        );
+        assert!(
+            !rules.contains(&tmp.path().display().to_string()),
+            "absolute paths must not enter rule source labels: {rules}"
+        );
+    }
+
     #[test]
     fn test_empty_file_warning() {
         let tmp = tempdir().expect("tempdir");
@@ -1597,6 +1804,18 @@ mod tests {
             !instructions.contains("MAIN-CHECKOUT-ONLY instructions"),
             "the main checkout's file is outside the worktree's chain:\n{instructions}"
         );
+        // The label root is the worktree itself: resolving repository identity
+        // through the `gitdir:` pointer to the main checkout would fail the
+        // strip and fall back to an absolute path, and rooting the label at
+        // the workspace would collapse it to `AGENTS.md`.
+        assert!(
+            instructions.contains("<!-- scoped instructions: crates/tui/AGENTS.md"),
+            "the chain label must be relative to the worktree root:\n{instructions}"
+        );
+        assert!(
+            !instructions.contains(&tmp.path().display().to_string()),
+            "absolute paths must not enter worktree chain labels:\n{instructions}"
+        );
         let expected_source = fs::canonicalize(&nested)
             .expect("canonicalize nested")
             .join("AGENTS.md");
diff --git a/crates/tui/src/project_context/types.rs b/crates/tui/src/project_context/types.rs
index 3adc4a0455..7c500fcad2 100644
--- a/crates/tui/src/project_context/types.rs
+++ b/crates/tui/src/project_context/types.rs
@@ -138,6 +138,29 @@ pub(crate) fn project_instructions_source_label(source_path: Option<&Path>) -> S
         .unwrap_or_else(|| "project".to_string())
 }
 
+/// The `source` label for prompt blocks that must keep sibling scopes
+/// distinguishable (ancestor instruction-chain segments, `<project_rule>`
+/// blocks): the file's path relative to `root` — the containing checkout —
+/// rendered with forward slashes. When `root` is `None` or `path` is not
+/// inside it, falls back to the absolute spelling rather than guessing.
+///
+/// Like [`project_instructions_source_label`], the label sits inside the
+/// pinned system prompt, so keeping it stable across directory moves and
+/// recasings means an unchanged file does not emit a spurious
+/// `<context_update>` history append after a move, and absolute project
+/// paths stay out of provider-bound prompt labels. Root-relative spelling
+/// (rather than the bare file name) is deliberate: chain segments and rule
+/// files legitimately share basenames across scopes, and the label exists
+/// precisely to disambiguate them.
+pub(crate) fn repo_relative_source_label(path: &Path, root: Option<&Path>) -> String {
+    if let Some(root) = root
+        && let Ok(relative) = path.strip_prefix(root)
+    {
+        return relative.to_string_lossy().replace('\\', "/");
+    }
+    path.display().to_string()
+}
+
 /// Merge multiple project contexts (e.g., from nested directories)
 #[allow(dead_code)] // Public API for monorepo context merging
 pub fn merge_contexts(contexts: &[ProjectContext]) -> Option<String> {