Land #6739 as itself: repo-relative source labels for rules and chain segments

Merges the head of #6739 (38e04c5b3, by asto18089) unmodified, so the
original commit keeps its author. The contributor's fork refuses
maintainer pushes (HTTP 403), so the merge with main that resolves the
import-block conflict in crates/tui/src/project_context.rs is carried here
instead of on their branch. No change to the contributor's code.

cargo test -p codewhale-tui --lib project_context: 88 passed; 0 failed
(run on the merge with main, before this merge with the wave; the wave's
two extra commits do not touch project_context). rustfmt clean on the
touched files; CI-policy clippy on codewhale-tui printed no warnings.

Refs #6739

Signed-off-by: CodeWhale Bot <[email protected]>
Co-Authored-By: Claude Fable 5.1 <[email protected]>
This commit is contained in:
CodeWhale BotandClaude Fable 5.1 committed 2026-10-01 17:50:38 -07:00
commit 18477f36fc
2 files changed
+249 -17

No files matched your search

+226 -17
View File
@@ -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.
///
@@ -510,6 +511,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. `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);
@@ -519,7 +533,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()
));
}
@@ -618,13 +632,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. 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)];
@@ -649,7 +670,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,
@@ -660,7 +681,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.
@@ -743,7 +764,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));
}
@@ -891,12 +913,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);
@@ -928,12 +958,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; 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);
@@ -1352,7 +1390,8 @@ pub fn project_instruction_sources(
// Repository-root → workspace instruction chain. Same dir order the
// loader assembles, so wider scopes list first.
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) {
let whale = dir.join(DEPRECATED_WHALE_FILENAME);
if context_candidate_exists(&whale) {
sources.push(InstructionSourceInfo {
@@ -1813,8 +1852,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; 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");
@@ -1837,6 +1876,176 @@ mod tests {
);
}
#[test]
fn repo_relative_source_label_falls_back_when_path_is_outside_root() {
use crate::project_context::repo_relative_source_label;
use std::path::Path;
// 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.
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");
+23
View File
@@ -134,6 +134,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> {