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
8 changes: 4 additions & 4 deletions crates/gitlawb-node/src/api/issues.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ pub async fn create_issue(

let create_result = git_issues::create_issue(&disk_path, &issue_id, &json_str);

// Always release the advisory lock — even on error
guard.release().await;
// Always release the advisory lock — even on error; upload to Tigris only on success.
guard.release(create_result.is_ok()).await;

create_result.map_err(|e| AppError::Git(e.to_string()))?;

Expand Down Expand Up @@ -225,8 +225,8 @@ pub async fn close_issue(

let close_result = git_issues::close_issue(&disk_path, &issue_id);

// Always release the advisory lock — even on error
guard.release().await;
// Always release the advisory lock — even on error; upload to Tigris only on success.
guard.release(close_result.is_ok()).await;

let updated = close_result
.map_err(|e| AppError::Git(e.to_string()))?
Expand Down
4 changes: 2 additions & 2 deletions crates/gitlawb-node/src/api/pulls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ pub async fn merge_pr(
&pr.title,
);

// Always release the advisory lock — even on error
guard.release().await;
// Always release the advisory lock — even on error; upload to Tigris only on success.
guard.release(merge_result.is_ok()).await;

let merge_sha = merge_result.map_err(|e| AppError::Git(e.to_string()))?;

Expand Down
5 changes: 3 additions & 2 deletions crates/gitlawb-node/src/api/repos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,9 @@ pub async fn git_receive_pack(
let receive_result = smart_http::receive_pack(&disk_path, body).await;

// Always release the advisory lock — even on error — to prevent stale locks
// from blocking subsequent pushes to the same repo.
guard.release().await;
// from blocking subsequent pushes. Only upload to Tigris when the push
// succeeded; uploading a half-applied repo would propagate corruption.
guard.release(receive_result.is_ok()).await;

let result = receive_result.map_err(|e| {
tracing::error!(repo = %name, err = %e, "git receive-pack failed");
Expand Down
58 changes: 41 additions & 17 deletions crates/gitlawb-node/src/git/repo_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,19 @@ impl RepoStore {
if let Some(ref tigris) = self.tigris {
if tigris.exists(&owner_slug, repo_name).await.unwrap_or(false) {
debug!(repo = %repo_name, "acquire_fresh: downloading latest from tigris");
tigris
.download(&owner_slug, repo_name, &local_path)
.await
.context("downloading repo from tigris (fresh)")?;
if let Err(e) = tigris.download(&owner_slug, repo_name, &local_path).await {
// The Tigris archive is present (HEAD ok) but unreadable — a
// corrupt/partial upload, or a transient GET failure. If we have a
// valid local copy, proceed with it rather than blocking the write;
// the post-write upload re-syncs (self-heals) Tigris. Only hard-fail
// when there is no local copy to fall back to.
if local_path.exists() {
warn!(repo = %repo_name, err = %e,
"acquire_fresh: tigris download failed — falling back to local copy");
return Ok(local_path);
}
return Err(e).context("downloading repo from tigris (fresh)");
}
return Ok(local_path);
}
}
Expand Down Expand Up @@ -174,10 +183,17 @@ impl RepoStore {
if let Some(ref tigris) = self.tigris {
if tigris.exists(&owner_slug, repo_name).await.unwrap_or(false) {
debug!(repo = %repo_name, "write acquire: downloading latest from tigris");
tigris
.download(&owner_slug, repo_name, &local_path)
.await
.context("downloading repo from tigris for write")?;
if let Err(e) = tigris.download(&owner_slug, repo_name, &local_path).await {
// Same self-healing fallback as acquire_fresh: a corrupt/unreadable
// Tigris archive must not block a write when a valid local copy
// exists — release(success) will re-upload a good archive.
if local_path.exists() {
warn!(repo = %repo_name, err = %e,
"write acquire: tigris download failed — falling back to local copy");
} else {
return Err(e).context("downloading repo from tigris for write");
}
}
}
}

Expand Down Expand Up @@ -348,16 +364,24 @@ impl RepoWriteGuard {
&self.local_path
}

/// Upload to Tigris and release the advisory lock. Call this when the write is done.
pub async fn release(self) {
// Upload to Tigris
if let Some(ref tigris) = self.tigris {
if let Err(e) = tigris
.upload(&self.owner_slug, &self.repo_name, &self.local_path)
.await
{
warn!(repo = %self.repo_name, err = %e, "failed to upload repo to tigris after write");
/// Upload to Tigris (only when the write succeeded) and release the advisory
/// lock. Pass `success = false` when the write operation failed — uploading a
/// half-applied or otherwise inconsistent repo would propagate corruption to
/// Tigris (and to every node that later downloads it). The lock is always
/// released regardless, to avoid stale locks blocking future writes.
pub async fn release(self, success: bool) {
// Upload to Tigris only on success.
if success {
if let Some(ref tigris) = self.tigris {
if let Err(e) = tigris
.upload(&self.owner_slug, &self.repo_name, &self.local_path)
.await
{
warn!(repo = %self.repo_name, err = %e, "failed to upload repo to tigris after write");
}
}
} else {
warn!(repo = %self.repo_name, "write failed — skipping tigris upload to avoid propagating an inconsistent repo");
}

// Release advisory lock
Expand Down
44 changes: 37 additions & 7 deletions crates/gitlawb-node/src/git/tigris.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,16 +157,46 @@ fn compress_repo(repo_path: &Path) -> Result<Vec<u8>> {
}

/// Decompress a tar.zst byte vector into a local directory.
///
/// Extraction is atomic with respect to `local_path`: the archive is unpacked
/// into a sibling temp directory first, and only swapped into place once it
/// fully succeeds. A corrupt or truncated archive therefore can never clobber a
/// good existing copy at `local_path` — on failure we discard the temp dir and
/// leave `local_path` exactly as it was.
fn decompress_repo(data: &[u8], local_path: &Path) -> Result<()> {
// Ensure parent directory exists
if let Some(parent) = local_path.parent() {
std::fs::create_dir_all(parent).context("creating parent dir")?;
let parent = local_path.parent().context("repo path has no parent")?;
std::fs::create_dir_all(parent).context("creating parent dir")?;

let file_name = local_path
.file_name()
.context("repo path has no file name")?
.to_string_lossy();
let tmp_dir = parent.join(format!(".{file_name}.tmp-extract"));

// Clear any leftover temp dir from a previously-interrupted extraction.
let _ = std::fs::remove_dir_all(&tmp_dir);
std::fs::create_dir_all(&tmp_dir).context("creating temp extract dir")?;

// Unpack into the temp dir; on any failure, clean up and bail without
// touching local_path.
let unpack = (|| -> Result<()> {
let decoder = zstd::stream::Decoder::new(data)?;
let mut archive = tar::Archive::new(decoder);
archive.unpack(&tmp_dir).context("unpacking tar.zst")?;
Ok(())
})();
if let Err(e) = unpack {
let _ = std::fs::remove_dir_all(&tmp_dir);
return Err(e);
}
std::fs::create_dir_all(local_path).context("creating repo dir")?;

let decoder = zstd::stream::Decoder::new(data)?;
let mut archive = tar::Archive::new(decoder);
archive.unpack(local_path).context("unpacking tar.zst")?;
// Swap the freshly-extracted repo into place. rename within the same parent
// is effectively atomic, but most platforms refuse to rename onto a
// non-empty dir, so remove the old copy first.
if local_path.exists() {
std::fs::remove_dir_all(local_path).context("removing stale repo dir")?;
}
std::fs::rename(&tmp_dir, local_path).context("swapping extracted repo into place")?;

Ok(())
}
Loading