Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 84 additions & 14 deletions crosslink/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ mod trust;
mod tests;

use std::path::Path;
use std::process::Command;
use std::sync::Once;

/// Directory name under .crosslink for the hub cache worktree.
Expand Down Expand Up @@ -172,13 +173,26 @@ impl PushFailure {
}
}

/// Read the configured tracker remote name from `.crosslink/hook-config.json`.
/// Read the configured tracker remote name for hub sync operations.
///
/// Returns the value of `tracker_remote` if set, otherwise `"origin"`.
/// This is a pure config read — no subprocess calls. Use
/// `SyncManager::remote_exists()` to validate the remote.
/// Resolution order:
///
/// 1. If `hook-config.json` sets `tracker_remote` to a non-placeholder
/// string, use it verbatim. The GH#739 `"(text)"` corruption sentinel
/// is still detected here and a single WARN is emitted before
/// falling through to inference.
/// 2. Otherwise infer from the project's git remotes (see
/// [`infer_tracker_remote`]). This replaces the noisy per-invocation
/// `"no tracker_remote configured"` warning that used to fire on
/// every command in fresh projects (GH#611). The common case — a
/// single `origin` remote — is now resolved silently.
/// 3. If the repo has no remotes at all, fall back to `"origin"` and
/// emit a single WARN, since `crosslink sync` will fail without a
/// remote and the user needs to know.
///
/// Use [`SyncManager::remote_exists`] to validate the result against
/// the actual git config before any push/fetch.
pub fn read_tracker_remote(crosslink_dir: &Path) -> String {
static WARNED: Once = Once::new();
static CORRUPT_WARNED: Once = Once::new();

let config_path = crosslink_dir.join("hook-config.json");
Expand All @@ -194,7 +208,7 @@ pub fn read_tracker_remote(crosslink_dir: &Path) -> String {
// GH#739 — pre-fix builds of the init walkthrough wrote the
// TUI placeholder "(text)" into hook-config.json for every
// `ConfigType::String` key. Detect that here and warn the
// user once, falling back to "origin" so sync doesn't bail
// user once, falling back to inference so sync doesn't bail
// with a (correct but unhelpful) RemoteMisconfigured error.
// The permanent fix is `crosslink config set tracker_remote
// <name>` or `crosslink init --force` (which now auto-repairs
Expand All @@ -203,25 +217,81 @@ pub fn read_tracker_remote(crosslink_dir: &Path) -> String {
CORRUPT_WARNED.call_once(|| {
tracing::warn!(
"tracker_remote in {} is the corrupt placeholder \"(text)\" \
(GH#739). Falling back to \"origin\". Repair with: \
(GH#739). Falling back to inferred remote. Repair with: \
`crosslink config set tracker_remote <name>` or \
`crosslink init --force`.",
config_path.display()
);
});
return "origin".to_string();
// fall through to inference rather than blindly returning "origin"
} else {
return remote;
}
return remote;
}

// Warn once when falling back to "origin".
WARNED.call_once(|| {
// GH#611: silently infer from git remotes instead of WARN-and-default.
let repo_path = crosslink_dir.parent().unwrap_or(crosslink_dir);
infer_tracker_remote(repo_path)
}

/// List the names of git remotes configured for `repo_path`, alphabetically.
///
/// Returns an empty vec when the directory isn't a git repo or `git remote`
/// fails for any other reason — the caller treats "no remotes" as a soft
/// signal, not a hard error. Sort is deterministic so callers picking
/// "first alphabetical" get repeatable behaviour across machines.
fn list_git_remotes(repo_path: &Path) -> Vec<String> {
let output = Command::new("git")
.current_dir(repo_path)
.args(["remote"])
.output();
let Ok(output) = output else {
return Vec::new();
};
if !output.status.success() {
return Vec::new();
}
let mut remotes: Vec<String> = String::from_utf8_lossy(&output.stdout)
.lines()
.map(str::trim)
.filter(|l| !l.is_empty())
.map(str::to_string)
.collect();
remotes.sort();
remotes
}

/// Infer the tracker remote name from the project's git remotes when
/// `hook-config.json` doesn't set one (GH#611).
///
/// Selection rules — designed so >99% of projects (single `origin`
/// remote) resolve silently:
///
/// - Any remote named `origin` exists → use `"origin"`. Covers
/// single-remote-named-origin and multi-remote-with-origin cases.
/// - Otherwise, if at least one remote exists → use the first
/// alphabetically. Deterministic across machines.
/// - Zero remotes → default to `"origin"` and emit a single WARN.
/// The user will need to `git remote add` before sync works, and
/// this is the one case where a real diagnostic is warranted.
fn infer_tracker_remote(repo_path: &Path) -> String {
static NO_REMOTE_WARNED: Once = Once::new();

let remotes = list_git_remotes(repo_path);
if remotes.iter().any(|r| r == "origin") {
return "origin".to_string();
}
if let Some(first) = remotes.first() {
return first.clone();
}

NO_REMOTE_WARNED.call_once(|| {
tracing::warn!(
"no tracker_remote configured in {}, defaulting to \"origin\"",
config_path.display()
"no git remote configured in {}; defaulting tracker_remote to \"origin\". \
Add a remote with `git remote add origin <url>` before `crosslink sync`.",
repo_path.display()
);
});

"origin".to_string()
}

Expand Down
122 changes: 122 additions & 0 deletions crosslink/src/sync/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -840,6 +840,128 @@ fn test_read_tracker_remote_custom_value() {
assert_eq!(remote, "upstream");
}

// ── GH#611: silent inference from git remotes ────────────────────

/// `git remote add` a name pointing at a placeholder URL on `repo`.
/// The URL value doesn't matter for `git remote` (the listing command),
/// so we just need a syntactically valid string.
fn add_git_remote(repo: &Path, name: &str) {
let url = format!("https://example.invalid/{name}.git");
let status = Command::new("git")
.current_dir(repo)
.args(["remote", "add", name, &url])
.status()
.expect("git remote add failed to spawn");
assert!(status.success(), "git remote add {name} failed");
}

#[test]
fn test_read_tracker_remote_single_origin_no_warn() {
// The single most common project shape: one git remote called "origin",
// hook-config.json has no `tracker_remote` field. Before GH#611 this
// path emitted a WARN once per process; after, it must be silent.
let (work_dir, _remote_dir) = setup_sync_env();
let crosslink_dir = work_dir.path().join(".crosslink");
// hook-config.json from setup_sync_env only sets {"remote":"origin"},
// intentionally NOT a tracker_remote field — so we drop into inference.
let remote = read_tracker_remote(&crosslink_dir);
assert_eq!(remote, "origin");
}

#[test]
fn test_read_tracker_remote_single_non_origin_remote() {
// One remote, not called "origin" — inferred verbatim.
let dir = tempdir().unwrap();
Command::new("git")
.current_dir(dir.path())
.args(["init", "-b", "main"])
.output()
.unwrap();
add_git_remote(dir.path(), "upstream");

let crosslink_dir = dir.path().join(".crosslink");
std::fs::create_dir_all(&crosslink_dir).unwrap();
// hook-config.json exists but has no tracker_remote field
std::fs::write(crosslink_dir.join("hook-config.json"), r#"{}"#).unwrap();

let remote = read_tracker_remote(&crosslink_dir);
assert_eq!(
remote, "upstream",
"single non-origin remote should be inferred as the tracker_remote"
);
}

#[test]
fn test_read_tracker_remote_multi_remotes_prefers_origin() {
// Multiple remotes including "origin" — origin wins regardless of order.
let dir = tempdir().unwrap();
Command::new("git")
.current_dir(dir.path())
.args(["init", "-b", "main"])
.output()
.unwrap();
// Add in non-alphabetical order to confirm origin wins on name, not order.
add_git_remote(dir.path(), "zzz");
add_git_remote(dir.path(), "origin");
add_git_remote(dir.path(), "fork");

let crosslink_dir = dir.path().join(".crosslink");
std::fs::create_dir_all(&crosslink_dir).unwrap();

let remote = read_tracker_remote(&crosslink_dir);
assert_eq!(remote, "origin", "origin must win in multi-remote setups");
}

#[test]
fn test_read_tracker_remote_multi_remotes_no_origin_picks_first_alphabetical() {
// Multiple remotes, none called origin → first alphabetically (deterministic).
let dir = tempdir().unwrap();
Command::new("git")
.current_dir(dir.path())
.args(["init", "-b", "main"])
.output()
.unwrap();
add_git_remote(dir.path(), "upstream");
add_git_remote(dir.path(), "alice");
add_git_remote(dir.path(), "bob");

let crosslink_dir = dir.path().join(".crosslink");
std::fs::create_dir_all(&crosslink_dir).unwrap();

let remote = read_tracker_remote(&crosslink_dir);
assert_eq!(
remote, "alice",
"with no origin and multiple remotes, picks the alphabetically first one"
);
}

#[test]
fn test_read_tracker_remote_explicit_config_wins_over_inference() {
// hook-config.json's explicit value beats whatever git remotes say.
let dir = tempdir().unwrap();
Command::new("git")
.current_dir(dir.path())
.args(["init", "-b", "main"])
.output()
.unwrap();
add_git_remote(dir.path(), "origin");
add_git_remote(dir.path(), "upstream");

let crosslink_dir = dir.path().join(".crosslink");
std::fs::create_dir_all(&crosslink_dir).unwrap();
std::fs::write(
crosslink_dir.join("hook-config.json"),
r#"{"tracker_remote":"upstream"}"#,
)
.unwrap();

let remote = read_tracker_remote(&crosslink_dir);
assert_eq!(
remote, "upstream",
"explicit hook-config.json value must take precedence over inference"
);
}

#[test]
fn test_read_tracker_remote_falls_back_when_corrupt_placeholder() {
// GH#739: older builds of the init walkthrough wrote the literal
Expand Down
Loading