mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Add thread ownership metadata for managed worktrees (#40716)
## What changed - Add `WorktreeManager` APIs to bind a managed linked worktree to a thread and read its owner. - Store the versioned `codex-thread.json` record in Git metadata with atomic, no-clobber writes, making repeat bindings idempotent while rejecting conflicting owners. - Validate the managed worktree layout and reject primary, nested, and unmanaged checkouts before accessing ownership metadata. - Run Git metadata queries with repository-selecting environment variables removed and hooks, filesystem monitoring, attributes, and LFS smudging disabled. ## Testing - Add integration coverage for rejecting primary and unmanaged worktrees and for writing and reading the expected ownership schema. GitOrigin-RevId: afec9a0ab711fe4017074bcec5807d860e9c88d0
This commit is contained in:
committed by
copyberry
parent
346c4db7c2
commit
23cedf4802
Generated
+3
@@ -4855,8 +4855,11 @@ name = "codex-worktree"
|
||||
version = "0.0.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"codex-git-utils",
|
||||
"codex-protocol",
|
||||
"dunce",
|
||||
"pretty_assertions",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"tempfile",
|
||||
]
|
||||
|
||||
@@ -13,9 +13,12 @@ test = false
|
||||
|
||||
[dependencies]
|
||||
anyhow = { workspace = true }
|
||||
codex-git-utils = { workspace = true }
|
||||
codex-protocol = { workspace = true }
|
||||
dunce = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
pretty_assertions = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
//! Executes Git metadata queries and decodes native filesystem paths.
|
||||
//!
|
||||
//! Commands remove inherited repository selectors and per-command Git config,
|
||||
//! select the repository from the supplied working directory, and disable hooks
|
||||
//! and filesystem monitoring.
|
||||
|
||||
use std::ffi::OsStr;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::bail;
|
||||
use codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG;
|
||||
use codex_protocol::shell_environment::scrub_non_inheritable_env_vars;
|
||||
|
||||
const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" };
|
||||
|
||||
pub(crate) fn git_path<I, S>(cwd: &Path, args: I) -> Result<PathBuf>
|
||||
where
|
||||
I: IntoIterator<Item = S>,
|
||||
S: AsRef<OsStr>,
|
||||
{
|
||||
let output = base_git_command(cwd)
|
||||
.args(args)
|
||||
.output()
|
||||
.context("failed to start git")?;
|
||||
if !output.status.success() {
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
bail!("git command failed: {}", stderr.trim());
|
||||
}
|
||||
let bytes = output.stdout.strip_suffix(b"\n").unwrap_or(&output.stdout);
|
||||
#[cfg(windows)]
|
||||
let bytes = bytes.strip_suffix(b"\r").unwrap_or(bytes);
|
||||
git_path_from_bytes(bytes)
|
||||
}
|
||||
|
||||
pub(crate) fn git_path_from_bytes(bytes: &[u8]) -> Result<PathBuf> {
|
||||
if bytes.is_empty() {
|
||||
bail!("git returned an empty path");
|
||||
}
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::ffi::OsStringExt;
|
||||
Ok(PathBuf::from(std::ffi::OsString::from_vec(bytes.to_vec())))
|
||||
}
|
||||
#[cfg(not(unix))]
|
||||
{
|
||||
Ok(PathBuf::from(
|
||||
std::str::from_utf8(bytes).context("git path is not valid UTF-8")?,
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
fn base_git_command(cwd: &Path) -> Command {
|
||||
let mut command = Command::new("git");
|
||||
// Git wrappers export repository-local state to their children. Select this
|
||||
// repository from cwd, and install only our own per-command config below.
|
||||
for name in [
|
||||
"GIT_ALTERNATE_OBJECT_DIRECTORIES",
|
||||
"GIT_CONFIG",
|
||||
"GIT_CONFIG_PARAMETERS",
|
||||
"GIT_CONFIG_COUNT",
|
||||
"GIT_OBJECT_DIRECTORY",
|
||||
"GIT_DIR",
|
||||
"GIT_WORK_TREE",
|
||||
"GIT_IMPLICIT_WORK_TREE",
|
||||
"GIT_GRAFT_FILE",
|
||||
"GIT_INDEX_FILE",
|
||||
"GIT_NO_REPLACE_OBJECTS",
|
||||
"GIT_REPLACE_REF_BASE",
|
||||
"GIT_PREFIX",
|
||||
"GIT_SHALLOW_FILE",
|
||||
"GIT_COMMON_DIR",
|
||||
] {
|
||||
command.env_remove(name);
|
||||
}
|
||||
command
|
||||
.current_dir(cwd)
|
||||
.arg("-c")
|
||||
.arg(SAFE_BARE_REPOSITORY_CONFIG)
|
||||
.arg("-c")
|
||||
.arg(format!("core.hooksPath={DISABLED_HOOKS_PATH}"))
|
||||
.arg("-c")
|
||||
.arg("core.fsmonitor=")
|
||||
.arg("-c")
|
||||
.arg("attr.tree=")
|
||||
.arg("-c")
|
||||
.arg("core.attributesFile=")
|
||||
.env("GIT_LFS_SKIP_SMUDGE", "1")
|
||||
.env("GIT_TERMINAL_PROMPT", "0");
|
||||
scrub_non_inheritable_env_vars(&mut command);
|
||||
command
|
||||
}
|
||||
@@ -1,4 +1,99 @@
|
||||
mod git;
|
||||
mod metadata;
|
||||
mod settings;
|
||||
|
||||
use crate::git::git_path;
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::bail;
|
||||
use std::path::Component;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
pub use settings::DEFAULT_WORKTREE_KEEP_COUNT;
|
||||
pub use settings::WorktreeSettings;
|
||||
|
||||
/// Creates and identifies worktrees using the existing Codex Desktop contract.
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct WorktreeManager {
|
||||
settings: WorktreeSettings,
|
||||
}
|
||||
|
||||
impl WorktreeManager {
|
||||
pub fn new(mut settings: WorktreeSettings) -> Self {
|
||||
settings.root = dunce::simplified(&settings.root).to_path_buf();
|
||||
Self { settings }
|
||||
}
|
||||
|
||||
pub fn settings(&self) -> &WorktreeSettings {
|
||||
&self.settings
|
||||
}
|
||||
|
||||
pub fn bind_thread(&self, checkout: &Path, thread_id: &str) -> Result<()> {
|
||||
let checkout = self.managed_checkout(checkout)?;
|
||||
metadata::bind_thread(&checkout, thread_id)
|
||||
}
|
||||
|
||||
pub fn owner(&self, checkout: &Path) -> Result<Option<String>> {
|
||||
let checkout = self.managed_checkout(checkout)?;
|
||||
metadata::owner(&checkout)
|
||||
}
|
||||
|
||||
fn managed_checkout(&self, checkout: &Path) -> Result<PathBuf> {
|
||||
let managed_root = dunce::canonicalize(&self.settings.root).with_context(|| {
|
||||
format!(
|
||||
"cannot resolve managed worktree root {}",
|
||||
self.settings.root.display()
|
||||
)
|
||||
})?;
|
||||
let checkout = dunce::canonicalize(checkout)
|
||||
.with_context(|| format!("cannot resolve worktree {}", checkout.display()))?;
|
||||
if !has_managed_layout(&managed_root, &checkout) {
|
||||
bail!("{} is not a managed worktree", checkout.display());
|
||||
}
|
||||
|
||||
linked_worktree_common_dir(&checkout)?;
|
||||
Ok(checkout)
|
||||
}
|
||||
}
|
||||
|
||||
fn linked_worktree_common_dir(checkout: &Path) -> Result<PathBuf> {
|
||||
if checkout != repository_root(checkout)? {
|
||||
bail!("{} is not a worktree root", checkout.display());
|
||||
}
|
||||
let git_dir = resolve_git_path(checkout, "--git-dir")?;
|
||||
let common_dir = resolve_git_path(checkout, "--git-common-dir")?;
|
||||
if git_dir == common_dir {
|
||||
bail!("{} is not a linked worktree", checkout.display());
|
||||
}
|
||||
Ok(common_dir)
|
||||
}
|
||||
|
||||
fn has_managed_layout(root: &Path, checkout: &Path) -> bool {
|
||||
let Ok(relative) = checkout.strip_prefix(root) else {
|
||||
return false;
|
||||
};
|
||||
let mut components = relative.components();
|
||||
let Some(Component::Normal(bucket)) = components.next() else {
|
||||
return false;
|
||||
};
|
||||
let Some(bucket) = bucket.to_str() else {
|
||||
return false;
|
||||
};
|
||||
bucket.len() == 4
|
||||
&& bucket.bytes().all(|byte| byte.is_ascii_hexdigit())
|
||||
&& matches!(components.next(), Some(Component::Normal(_)))
|
||||
&& components.next().is_none()
|
||||
}
|
||||
|
||||
fn resolve_git_path(checkout: &Path, argument: &str) -> Result<PathBuf> {
|
||||
let path = checkout.join(git_path(checkout, ["rev-parse", argument])?);
|
||||
dunce::canonicalize(&path)
|
||||
.with_context(|| format!("cannot resolve Git path {}", path.display()))
|
||||
}
|
||||
|
||||
fn repository_root(cwd: &Path) -> Result<PathBuf> {
|
||||
let path = git_path(cwd, ["rev-parse", "--show-toplevel"])?;
|
||||
dunce::canonicalize(&path)
|
||||
.with_context(|| format!("cannot resolve repository root {}", path.display()))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
//! Reads and binds Desktop-compatible thread ownership for managed worktrees.
|
||||
//!
|
||||
//! Stores a versioned `codex-thread.json` record in each worktree's Git metadata.
|
||||
//! Binding publishes the record atomically without replacing another owner;
|
||||
//! binding the same thread again leaves the existing record unchanged.
|
||||
|
||||
use crate::git::git_path;
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::bail;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::fs;
|
||||
use std::io::Write;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use tempfile::NamedTempFile;
|
||||
|
||||
const OWNER_FILENAME: &str = "codex-thread.json";
|
||||
const OWNER_VERSION: u8 = 1;
|
||||
|
||||
#[derive(Debug, Deserialize, PartialEq, Eq, Serialize)]
|
||||
#[serde(rename_all = "camelCase", deny_unknown_fields)]
|
||||
struct OwnerRecord {
|
||||
version: u8,
|
||||
owner_thread_id: String,
|
||||
}
|
||||
|
||||
pub(crate) fn owner(checkout: &Path) -> Result<Option<String>> {
|
||||
let path = metadata_path(checkout)?;
|
||||
let contents = match fs::read(&path) {
|
||||
Ok(contents) => contents,
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None),
|
||||
Err(error) => {
|
||||
return Err(error)
|
||||
.with_context(|| format!("cannot read worktree owner at {}", path.display()));
|
||||
}
|
||||
};
|
||||
|
||||
let record: OwnerRecord = serde_json::from_slice(&contents)
|
||||
.with_context(|| format!("invalid worktree owner at {}", path.display()))?;
|
||||
if record.version != OWNER_VERSION || record.owner_thread_id.is_empty() {
|
||||
bail!("invalid worktree owner at {}", path.display());
|
||||
}
|
||||
|
||||
Ok(Some(record.owner_thread_id))
|
||||
}
|
||||
|
||||
pub(crate) fn bind_thread(checkout: &Path, thread_id: &str) -> Result<()> {
|
||||
if thread_id.is_empty() {
|
||||
bail!("worktree owner thread ID cannot be empty");
|
||||
}
|
||||
|
||||
if let Some(existing) = owner(checkout)? {
|
||||
if existing == thread_id {
|
||||
return Ok(());
|
||||
}
|
||||
bail!("worktree already belongs to thread {existing}");
|
||||
}
|
||||
|
||||
let path = metadata_path(checkout)?;
|
||||
let directory = path.parent().context("worktree metadata has no parent")?;
|
||||
let mut temporary = NamedTempFile::new_in(directory)
|
||||
.with_context(|| format!("cannot create worktree metadata in {}", directory.display()))?;
|
||||
serde_json::to_writer(
|
||||
&mut temporary,
|
||||
&OwnerRecord {
|
||||
version: OWNER_VERSION,
|
||||
owner_thread_id: thread_id.to_owned(),
|
||||
},
|
||||
)?;
|
||||
temporary.flush()?;
|
||||
|
||||
match temporary.persist_noclobber(&path) {
|
||||
Ok(_) => Ok(()),
|
||||
Err(error) if error.error.kind() == std::io::ErrorKind::AlreadyExists => {
|
||||
if owner(checkout)?.as_deref() == Some(thread_id) {
|
||||
Ok(())
|
||||
} else {
|
||||
bail!("worktree was concurrently assigned to another thread")
|
||||
}
|
||||
}
|
||||
Err(error) => Err(error.error)
|
||||
.with_context(|| format!("cannot write worktree owner at {}", path.display())),
|
||||
}
|
||||
}
|
||||
|
||||
fn metadata_path(checkout: &Path) -> Result<PathBuf> {
|
||||
let path = git_path(checkout, ["rev-parse", "--git-path", OWNER_FILENAME])?;
|
||||
Ok(checkout.join(path))
|
||||
}
|
||||
@@ -0,0 +1,177 @@
|
||||
#![allow(clippy::expect_used)]
|
||||
|
||||
use codex_worktree::WorktreeManager;
|
||||
use codex_worktree::WorktreeSettings;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
use tempfile::TempDir;
|
||||
|
||||
struct RepositoryFixture {
|
||||
_temp_dir: TempDir,
|
||||
codex_home: PathBuf,
|
||||
repository: PathBuf,
|
||||
}
|
||||
|
||||
impl RepositoryFixture {
|
||||
fn new() -> Self {
|
||||
let temp_dir = tempfile::tempdir().expect("create temporary test directory");
|
||||
let temp_root = dunce::canonicalize(temp_dir.path()).expect("canonicalize temporary root");
|
||||
let codex_home = temp_root.join("codex-home");
|
||||
let repository = temp_root.join("project");
|
||||
fs::create_dir_all(&codex_home).expect("create Codex home");
|
||||
initialize_repository(&repository);
|
||||
|
||||
Self {
|
||||
_temp_dir: temp_dir,
|
||||
codex_home,
|
||||
repository,
|
||||
}
|
||||
}
|
||||
|
||||
fn manager(&self) -> WorktreeManager {
|
||||
let settings =
|
||||
WorktreeSettings::from_desktop_config(&self.codex_home, /*desktop*/ None)
|
||||
.expect("load default worktree settings");
|
||||
WorktreeManager::new(settings)
|
||||
}
|
||||
}
|
||||
|
||||
fn initialize_repository(repository: &Path) {
|
||||
fs::create_dir_all(repository.join("nested/component"))
|
||||
.expect("create temporary repository directories");
|
||||
run_git(repository, &["init", "--quiet"]);
|
||||
fs::write(
|
||||
repository.join("README.md"),
|
||||
"initial repository contents\n",
|
||||
)
|
||||
.expect("write tracked repository file");
|
||||
fs::write(
|
||||
repository.join("nested/component/tracked.txt"),
|
||||
"nested tracked file\n",
|
||||
)
|
||||
.expect("write nested tracked repository file");
|
||||
run_git(repository, &["add", "."]);
|
||||
commit(repository, "initial commit");
|
||||
}
|
||||
|
||||
fn commit(repository: &Path, message: &str) {
|
||||
run_git(
|
||||
repository,
|
||||
&["commit", "--quiet", "--no-gpg-sign", "-m", message],
|
||||
);
|
||||
}
|
||||
|
||||
fn git_output(repository: &Path, args: &[&str]) -> std::process::Output {
|
||||
Command::new("git")
|
||||
.current_dir(repository)
|
||||
.args([
|
||||
"-c",
|
||||
"user.name=Codex Worktree Test",
|
||||
"-c",
|
||||
"user.email=codex-worktree-test@example.invalid",
|
||||
"-c",
|
||||
"commit.gpgSign=false",
|
||||
"-c",
|
||||
"tag.gpgSign=false",
|
||||
])
|
||||
.args(args)
|
||||
.output()
|
||||
.unwrap_or_else(|error| panic!("run git {args:?} in {repository:?}: {error}"))
|
||||
}
|
||||
|
||||
fn run_git(repository: &Path, args: &[&str]) -> String {
|
||||
let output = git_output(repository, args);
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"git {args:?} failed in {repository:?}: {}",
|
||||
String::from_utf8_lossy(&output.stderr),
|
||||
);
|
||||
String::from_utf8(output.stdout)
|
||||
.expect("git output is UTF-8")
|
||||
.trim()
|
||||
.to_owned()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thread_binding_rejects_primary_and_unmanaged_worktrees() {
|
||||
let fixture = RepositoryFixture::new();
|
||||
let manager = fixture.manager();
|
||||
fs::create_dir_all(&manager.settings().root).expect("create managed root");
|
||||
assert!(
|
||||
manager
|
||||
.bind_thread(&fixture.repository, "not-a-managed-thread")
|
||||
.is_err(),
|
||||
);
|
||||
assert!(
|
||||
!fixture.repository.join(".git/codex-thread.json").exists(),
|
||||
"primary checkout ownership metadata must not be modified",
|
||||
);
|
||||
|
||||
let unmanaged = manager.settings().root.join("scratch").join("project");
|
||||
fs::create_dir_all(unmanaged.parent().expect("unmanaged parent"))
|
||||
.expect("create unmanaged parent");
|
||||
run_git(
|
||||
&fixture.repository,
|
||||
&[
|
||||
"worktree",
|
||||
"add",
|
||||
"--detach",
|
||||
unmanaged.to_str().expect("UTF-8 unmanaged worktree"),
|
||||
"HEAD",
|
||||
],
|
||||
);
|
||||
assert!(manager.bind_thread(&unmanaged, "unmanaged").is_err());
|
||||
assert!(manager.owner(&unmanaged).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thread_binding_writes_the_exact_desktop_owner_schema_into_git_metadata() {
|
||||
let fixture = RepositoryFixture::new();
|
||||
let manager = fixture.manager();
|
||||
let checkout = manager.settings().root.join("a1b2/project");
|
||||
fs::create_dir_all(checkout.parent().expect("checkout parent")).expect("create bucket");
|
||||
run_git(
|
||||
&fixture.repository,
|
||||
&[
|
||||
"worktree",
|
||||
"add",
|
||||
"--detach",
|
||||
checkout.to_str().expect("UTF-8 checkout"),
|
||||
"HEAD",
|
||||
],
|
||||
);
|
||||
let thread_id = "019f1234-5678-7000-8000-000000000001";
|
||||
|
||||
assert_eq!(manager.owner(&checkout).expect("read initial owner"), None);
|
||||
|
||||
manager
|
||||
.bind_thread(&checkout, thread_id)
|
||||
.expect("bind managed worktree to thread");
|
||||
|
||||
assert_eq!(
|
||||
manager.owner(&checkout).expect("read bound owner"),
|
||||
Some(thread_id.to_owned()),
|
||||
);
|
||||
|
||||
let metadata_path = checkout.join(run_git(
|
||||
&checkout,
|
||||
&["rev-parse", "--git-path", "codex-thread.json"],
|
||||
));
|
||||
let metadata: Value = serde_json::from_slice(
|
||||
&fs::read(&metadata_path).expect("read Desktop-compatible worktree owner metadata"),
|
||||
)
|
||||
.expect("parse Desktop-compatible worktree owner metadata");
|
||||
|
||||
assert_eq!(
|
||||
metadata,
|
||||
json!({
|
||||
"version": 1,
|
||||
"ownerThreadId": thread_id,
|
||||
}),
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user