Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
4851ad9101 | ||
|
|
f0c272e0b1 | ||
|
|
624f6f19c8 | ||
|
|
49ab7e9c81 |
@@ -111,16 +111,33 @@ impl WorktreeManager {
|
|||||||
// Clear registrations whose directory is gone first, so a
|
// Clear registrations whose directory is gone first, so a
|
||||||
// previously-leaked entry doesn't push this branch to a suffix.
|
// previously-leaked entry doesn't push this branch to a suffix.
|
||||||
prune(base_repo)?;
|
prune(base_repo)?;
|
||||||
// Anything occupying the target — a tree we left behind, one the
|
let (branch, path) = if create_branch {
|
||||||
// operator made, an unrelated directory — is sidestepped with a
|
// Worktrees are commonly abandoned without deleting their branch,
|
||||||
// numeric suffix on the directory; the branch stays whatever was
|
// so a random handle can collide with one of its own past runs.
|
||||||
// requested (#77). The daemon never adopts what it finds.
|
// Unlike the `add_existing` arm below, the suffix goes on the
|
||||||
let mut path = self.target_path(base_repo, branch);
|
// branch name itself — the path is derived from it anyway.
|
||||||
let mut n = 1;
|
let mut candidate = branch.to_string();
|
||||||
while path.symlink_metadata().is_ok() {
|
let mut path = self.target_path(base_repo, &candidate);
|
||||||
n += 1;
|
let mut n = 1;
|
||||||
path = self.target_path_suffixed(base_repo, branch, n);
|
while path.symlink_metadata().is_ok() || branch_exists(base_repo, &candidate) {
|
||||||
}
|
n += 1;
|
||||||
|
candidate = format!("{branch}-{n}");
|
||||||
|
path = self.target_path(base_repo, &candidate);
|
||||||
|
}
|
||||||
|
(candidate, path)
|
||||||
|
} else {
|
||||||
|
// Existing branch: the name must stay exact, so only the
|
||||||
|
// directory — anything occupying it, ours or the operator's — is
|
||||||
|
// sidestepped with a numeric suffix (#77). The daemon never
|
||||||
|
// adopts what it finds.
|
||||||
|
let mut path = self.target_path(base_repo, branch);
|
||||||
|
let mut n = 1;
|
||||||
|
while path.symlink_metadata().is_ok() {
|
||||||
|
n += 1;
|
||||||
|
path = self.target_path_suffixed(base_repo, branch, n);
|
||||||
|
}
|
||||||
|
(branch.to_string(), path)
|
||||||
|
};
|
||||||
// The parent (`<repo>/worktrees` or the override root) may not exist
|
// The parent (`<repo>/worktrees` or the override root) may not exist
|
||||||
// yet; the target itself must not.
|
// yet; the target itself must not.
|
||||||
if let Some(parent) = path.parent() {
|
if let Some(parent) = path.parent() {
|
||||||
@@ -129,10 +146,10 @@ impl WorktreeManager {
|
|||||||
let mut cmd = Command::new("git");
|
let mut cmd = Command::new("git");
|
||||||
cmd.arg("-C").arg(base_repo).args(["worktree", "add"]);
|
cmd.arg("-C").arg(base_repo).args(["worktree", "add"]);
|
||||||
if create_branch {
|
if create_branch {
|
||||||
cmd.arg("-b").arg(branch).arg(&path);
|
cmd.arg("-b").arg(&branch).arg(&path);
|
||||||
} else {
|
} else {
|
||||||
// Existing branch: `worktree add <path> <branch>` with no `-b`.
|
// Existing branch: `worktree add <path> <branch>` with no `-b`.
|
||||||
cmd.arg(&path).arg(branch);
|
cmd.arg(&path).arg(&branch);
|
||||||
}
|
}
|
||||||
let out = cmd.output()?;
|
let out = cmd.output()?;
|
||||||
check(out)?;
|
check(out)?;
|
||||||
@@ -266,6 +283,17 @@ fn branch_relpath(branch: &str) -> Option<PathBuf> {
|
|||||||
Some(candidate.to_path_buf())
|
Some(candidate.to_path_buf())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Whether `refs/heads/<branch>` already exists in `base_repo`.
|
||||||
|
fn branch_exists(base_repo: &Path, branch: &str) -> bool {
|
||||||
|
Command::new("git")
|
||||||
|
.arg("-C")
|
||||||
|
.arg(base_repo)
|
||||||
|
.args(["show-ref", "--verify", "--quiet"])
|
||||||
|
.arg(format!("refs/heads/{branch}"))
|
||||||
|
.status()
|
||||||
|
.is_ok_and(|s| s.success())
|
||||||
|
}
|
||||||
|
|
||||||
/// `git -C <base_repo> worktree prune` — drops registrations whose directory
|
/// `git -C <base_repo> worktree prune` — drops registrations whose directory
|
||||||
/// no longer exists.
|
/// no longer exists.
|
||||||
fn prune(base_repo: &Path) -> Result<(), WorktreeError> {
|
fn prune(base_repo: &Path) -> Result<(), WorktreeError> {
|
||||||
|
|||||||
@@ -497,6 +497,57 @@ async fn occupied_path_gets_suffixed_directory_and_both_worktrees_work() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A random handle can reuse the name of a branch left behind by an earlier,
|
||||||
|
/// already-removed worktree — `git worktree add -b` refuses outright in that
|
||||||
|
/// case, even though the directory is free. The daemon suffixes the branch
|
||||||
|
/// itself (not just the directory) and both land on `-2`.
|
||||||
|
#[tokio::test(flavor = "multi_thread")]
|
||||||
|
async fn colliding_branch_name_gets_suffixed_too() {
|
||||||
|
let (_daemon, client) = bring_up().await;
|
||||||
|
|
||||||
|
let base = TempDir::new().unwrap();
|
||||||
|
init_repo(base.path());
|
||||||
|
|
||||||
|
// No worktree directory in the way, but the branch already exists.
|
||||||
|
run_git(&["branch", "handle"], base.path());
|
||||||
|
|
||||||
|
let s = client
|
||||||
|
.spawn_session(&worktree_req(base.path(), "handle", true))
|
||||||
|
.await
|
||||||
|
.expect("spawn onto a taken branch name");
|
||||||
|
|
||||||
|
let suffixed = base.path().join("worktrees").join("handle-2");
|
||||||
|
assert_eq!(
|
||||||
|
s.cwd, suffixed,
|
||||||
|
"the directory must follow the suffixed branch name"
|
||||||
|
);
|
||||||
|
assert!(suffixed.is_dir());
|
||||||
|
|
||||||
|
let registrations = git_worktree_list(base.path());
|
||||||
|
assert!(
|
||||||
|
registrations.iter().any(|l| l.contains("handle-2")),
|
||||||
|
"the -2 branch's worktree must be registered: {registrations:?}"
|
||||||
|
);
|
||||||
|
|
||||||
|
let branches = String::from_utf8(
|
||||||
|
std::process::Command::new("git")
|
||||||
|
.arg("-C")
|
||||||
|
.arg(base.path())
|
||||||
|
.args(["branch", "--list"])
|
||||||
|
.output()
|
||||||
|
.unwrap()
|
||||||
|
.stdout,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
assert!(
|
||||||
|
branches.contains("handle-2"),
|
||||||
|
"the branch itself must be named handle-2: {branches}"
|
||||||
|
);
|
||||||
|
|
||||||
|
let _origin = throwaway_origin(base.path());
|
||||||
|
client.kill_session(s.id).await.expect("kill");
|
||||||
|
}
|
||||||
|
|
||||||
/// #77/#88: the target path holds a plain directory the daemon never made and
|
/// #77/#88: the target path holds a plain directory the daemon never made and
|
||||||
/// git has never heard of. The daemon neither adopts it nor errors — it builds
|
/// git has never heard of. The daemon neither adopts it nor errors — it builds
|
||||||
/// beside it and leaves every byte of it alone.
|
/// beside it and leaves every byte of it alone.
|
||||||
|
|||||||
+12
-4
@@ -35,10 +35,18 @@ The branch name is used verbatim as a relative path, so `feature/login` becomes
|
|||||||
back to a short hash of the branch.
|
back to a short hash of the branch.
|
||||||
|
|
||||||
If that path exists, the daemon appends `-2`, then `-3`, and so on until it finds
|
If that path exists, the daemon appends `-2`, then `-3`, and so on until it finds
|
||||||
a free one. The suffix names the **directory only**. The branch is whatever was
|
a free one.
|
||||||
requested; a second worktree of the same branch is not what happens here, because
|
|
||||||
git refuses to check the same branch out twice and the branch picker already
|
For a *new* branch (`create_branch: true`), the suffix goes on the branch name
|
||||||
marks branches that are checked out elsewhere.
|
itself, and the directory follows since it is derived from the branch — a
|
||||||
|
random handle can otherwise reuse the name of a branch some earlier, already
|
||||||
|
removed worktree left behind, and `git worktree add -b` refuses outright even
|
||||||
|
though the directory is free.
|
||||||
|
|
||||||
|
For an *existing* branch (`create_branch: false`) the name must stay exact, so
|
||||||
|
only the directory is suffixed — a second worktree of the same branch is not
|
||||||
|
what happens here, because git refuses to check the same branch out twice and
|
||||||
|
the branch picker already marks branches that are checked out elsewhere.
|
||||||
|
|
||||||
### The `worktree_root` override
|
### The `worktree_root` override
|
||||||
|
|
||||||
|
|||||||
+23
-2
@@ -359,6 +359,9 @@ export function createAttachConnection(
|
|||||||
let ws: WebSocket;
|
let ws: WebSocket;
|
||||||
let reconnectMs = INITIAL_RECONNECT_MS;
|
let reconnectMs = INITIAL_RECONNECT_MS;
|
||||||
let reconnectTimer: number | null = null;
|
let reconnectTimer: number | null = null;
|
||||||
|
// Snapshot replay starts at row zero, which can leave Codex at its oldest line.
|
||||||
|
// Follow it once per socket; later scrolling remains under the operator's control.
|
||||||
|
let awaitingReplay = true;
|
||||||
|
|
||||||
const sendControl = (msg: ClientControl) => {
|
const sendControl = (msg: ClientControl) => {
|
||||||
if (ws.readyState !== WebSocket.OPEN) return;
|
if (ws.readyState !== WebSocket.OPEN) return;
|
||||||
@@ -378,6 +381,7 @@ export function createAttachConnection(
|
|||||||
};
|
};
|
||||||
|
|
||||||
function connect(): void {
|
function connect(): void {
|
||||||
|
awaitingReplay = true;
|
||||||
ws = new WebSocket(attachWsUrl(sessionId));
|
ws = new WebSocket(attachWsUrl(sessionId));
|
||||||
ws.binaryType = "arraybuffer";
|
ws.binaryType = "arraybuffer";
|
||||||
|
|
||||||
@@ -398,10 +402,20 @@ export function createAttachConnection(
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (ev.data instanceof ArrayBuffer) {
|
if (ev.data instanceof ArrayBuffer) {
|
||||||
term.write(new Uint8Array(ev.data));
|
const replay = awaitingReplay;
|
||||||
|
awaitingReplay = false;
|
||||||
|
term.write(new Uint8Array(ev.data), () => {
|
||||||
|
if (replay) term.scrollToBottom();
|
||||||
|
});
|
||||||
} else if (ev.data instanceof Blob) {
|
} else if (ev.data instanceof Blob) {
|
||||||
// Some browsers may default to Blob despite binaryType="arraybuffer".
|
// Some browsers may default to Blob despite binaryType="arraybuffer".
|
||||||
ev.data.arrayBuffer().then((b) => term.write(new Uint8Array(b)));
|
void ev.data.arrayBuffer().then((b) => {
|
||||||
|
const replay = awaitingReplay;
|
||||||
|
awaitingReplay = false;
|
||||||
|
term.write(new Uint8Array(b), () => {
|
||||||
|
if (replay) term.scrollToBottom();
|
||||||
|
});
|
||||||
|
});
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -450,6 +464,12 @@ export function createAttachConnection(
|
|||||||
const resizeDisp = term.onResize(({ cols, rows }) => {
|
const resizeDisp = term.onResize(({ cols, rows }) => {
|
||||||
sendControl({ type: "Resize", cols, rows });
|
sendControl({ type: "Resize", cols, rows });
|
||||||
});
|
});
|
||||||
|
// xterm has no built-in copy-on-select; mirror the selection to the
|
||||||
|
// clipboard ourselves (best-effort — insecure contexts lack the API).
|
||||||
|
const selectionDisp = term.onSelectionChange(() => {
|
||||||
|
const text = term.getSelection();
|
||||||
|
if (text) void navigator.clipboard?.writeText(text).catch(() => {});
|
||||||
|
});
|
||||||
|
|
||||||
window.addEventListener("resize", onResize);
|
window.addEventListener("resize", onResize);
|
||||||
const visual = window.visualViewport ?? null;
|
const visual = window.visualViewport ?? null;
|
||||||
@@ -505,6 +525,7 @@ export function createAttachConnection(
|
|||||||
visual?.removeEventListener("resize", onResize);
|
visual?.removeEventListener("resize", onResize);
|
||||||
dataDisp.dispose();
|
dataDisp.dispose();
|
||||||
resizeDisp.dispose();
|
resizeDisp.dispose();
|
||||||
|
selectionDisp.dispose();
|
||||||
releaseWebgl();
|
releaseWebgl();
|
||||||
try {
|
try {
|
||||||
ws.close();
|
ws.close();
|
||||||
|
|||||||
Reference in New Issue
Block a user