mirror of
https://github.com/codewhale-hq/Codewhale.git
synced 2026-11-09 17:10:44 -10:00
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:
commit
18477f36fc
2 files changed
+249
-17
No files matched your search
@@ -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");
|
||||
|
||||
@@ -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> {
|
||||
|
||||
Reference in new issue
Block a user