From 3ee8c27afb4fb20bf98e616b1a2c72afbf1ef6c1 Mon Sep 17 00:00:00 2001 From: servicememeai Date: Fri, 23 Oct 2026 18:10:11 +0900 Subject: [PATCH 1/8] fix(branch): distinguish retryable HEAD conflicts from failures --- common/branch-mgr.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/common/branch-mgr.h b/common/branch-mgr.h index 7c09aeac..b7aa418d 100644 --- a/common/branch-mgr.h +++ b/common/branch-mgr.h @@ -52,6 +52,11 @@ seaf_branch_manager_update_branch (SeafBranchManager *mgr, SeafBranch *branch); #ifdef SEAFILE_SERVER +/* Only a changed HEAD is retryable. Policy or SQL failure must stop + * immediately; the caller cannot recover it by creating more commits. */ +#define SEAF_BRANCH_UPDATE_ERROR (-1) +#define SEAF_BRANCH_UPDATE_ACL_DENIED (-2) +#define SEAF_BRANCH_UPDATE_HEAD_CONFLICT (-3) /** * Atomically test whether the current head commit id on @branch * is the same as @old_commit_id and update branch in db. From e63e4fba13015c3c359dca78cc29fcf4c9675443 Mon Sep 17 00:00:00 2001 From: servicememeai Date: Fri, 23 Oct 2026 18:10:14 +0900 Subject: [PATCH 2/8] fix(branch): return a unique HEAD-conflict code --- common/branch-mgr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/branch-mgr.c b/common/branch-mgr.c index 499eb359..192d6a08 100644 --- a/common/branch-mgr.c +++ b/common/branch-mgr.c @@ -529,7 +529,7 @@ test_and_update_branch (SeafBranchManager *mgr, if (strcmp (old_commit_id, commit_id) != 0) { seaf_db_rollback (trans); seaf_db_trans_close (trans); - return -1; + return SEAF_BRANCH_UPDATE_HEAD_CONFLICT; } /* Branch is locked before consulting the current authority. ACL writers @@ -564,7 +564,7 @@ test_and_update_branch (SeafBranchManager *mgr, if (acl_result < 0) { seaf_db_rollback (trans); seaf_db_trans_close (trans); - return -2; + return SEAF_BRANCH_UPDATE_ACL_DENIED; } sql = "UPDATE Branch SET commit_id = ? " From 061d976dacc569bdc4c3052b5515ba42ba6e84e0 Mon Sep 17 00:00:00 2001 From: servicememeai Date: Fri, 23 Oct 2026 18:10:17 +0900 Subject: [PATCH 3/8] fix(repo): make metadata edits idempotent and retry only HEAD races --- server/repo-mgr.c | 54 +++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/server/repo-mgr.c b/server/repo-mgr.c index 13c4cd41..d851af6c 100644 --- a/server/repo-mgr.c +++ b/server/repo-mgr.c @@ -4294,6 +4294,7 @@ seaf_repo_manager_edit_repo (const char *repo_id, SeafRepo *repo = NULL; SeafCommit *commit = NULL, *parent = NULL; int ret = 0; + int retry_cnt = 0; if (!name && !description) { g_set_error (error, SEAFILE_DOMAIN, SEAF_ERR_BAD_ARGS, @@ -4312,10 +4313,9 @@ seaf_repo_manager_edit_repo (const char *repo_id, g_set_error (error, SEAFILE_DOMAIN, SEAF_ERR_BAD_ARGS, "No such library"); return -1; } - if (!name) - name = repo->name; - if (!description) - description = repo->desc; + /* Do not retain a previous head's metadata across a HEAD-conflict retry. */ + const char *effective_name = name ? name : repo->name; + const char *effective_description = description ? description : repo->desc; /* * We only change repo_name or repo_desc, so just copy the head commit @@ -4330,14 +4330,16 @@ seaf_repo_manager_edit_repo (const char *repo_id, ret = -1; goto out; } - if (!user) { - user = parent->creator_name; - } + /* Idempotent metadata writes must never create a commit or take ACL locks. */ + if (g_strcmp0 (parent->repo_name, effective_name) == 0 && + g_strcmp0 (parent->repo_desc, effective_description) == 0) + goto out; + const char *effective_user = user ? user : parent->creator_name; commit = seaf_commit_new (NULL, repo->id, parent->root_id, - user, + effective_user, EMPTY_SHA1, "Changed library name or description", 0); @@ -4345,9 +4347,9 @@ seaf_repo_manager_edit_repo (const char *repo_id, seaf_repo_to_commit (repo, commit); g_free (commit->repo_name); - commit->repo_name = g_strdup(name); + commit->repo_name = g_strdup(effective_name); g_free (commit->repo_desc); - commit->repo_desc = g_strdup(description); + commit->repo_desc = g_strdup(effective_description); if (seaf_commit_manager_add_commit (seaf->commit_mgr, commit) < 0) { ret = -1; @@ -4355,17 +4357,27 @@ seaf_repo_manager_edit_repo (const char *repo_id, } seaf_branch_set_commit (repo->head, commit->commit_id); - if (seaf_branch_manager_test_and_update_branch (seaf->branch_mgr, - repo->head, - parent->commit_id, - FALSE, NULL, NULL, NULL) < 0) { - seaf_repo_unref (repo); - seaf_commit_unref (commit); - seaf_commit_unref (parent); - repo = NULL; - commit = NULL; - parent = NULL; - goto retry; + int published = seaf_branch_manager_test_and_update_branch (seaf->branch_mgr, + repo->head, + parent->commit_id, + FALSE, NULL, NULL, NULL); + if (published < 0) { + if (published == SEAF_BRANCH_UPDATE_HEAD_CONFLICT && retry_cnt < 3) { + ++retry_cnt; + seaf_repo_unref (repo); + seaf_commit_unref (commit); + seaf_commit_unref (parent); + repo = NULL; + commit = parent = NULL; + g_usleep (g_random_int_range (10, 51) * 1000); + goto retry; + } + g_set_error (error, SEAFILE_DOMAIN, + published == SEAF_BRANCH_UPDATE_HEAD_CONFLICT ? + SEAF_ERR_CONCURRENT_UPLOAD : SEAF_ERR_GENERAL, + "Library metadata update rejected or unavailable"); + ret = -1; + goto out; } seaf_repo_manager_update_repo_info (seaf->repo_mgr, repo_id, repo->head->commit_id); From 93d746140bcee6af88016aa17e314010a6a45c3f Mon Sep 17 00:00:00 2001 From: servicememeai Date: Fri, 23 Oct 2026 18:10:55 +0900 Subject: [PATCH 4/8] fix(branch): bound revert retries and avoid retrying non-HEAD errors --- server/repo-op.c | 51 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/server/repo-op.c b/server/repo-op.c index 470ee241..6de8edb2 100644 --- a/server/repo-op.c +++ b/server/repo-op.c @@ -635,7 +635,7 @@ gen_new_commit_guarded (const char *repo_id, repo->store_id, &gc_conflict); if (published < 0) { - if (published == -2) { + if (published == SEAF_BRANCH_UPDATE_ACL_DENIED) { g_set_error (error, SEAFILE_DOMAIN, SEAF_ERR_GENERAL, "CloudFile authority unavailable or fenced"); ret = -1; @@ -648,6 +648,12 @@ gen_new_commit_guarded (const char *repo_id, ret = -1; goto out; } + if (published != SEAF_BRANCH_UPDATE_HEAD_CONFLICT) { + g_set_error (error, SEAFILE_DOMAIN, SEAF_ERR_GENERAL, + "Branch update failed (not a HEAD conflict)"); + ret = -1; + goto out; + } if (!handle_concurrent_update) { g_set_error (error, SEAFILE_DOMAIN, SEAF_ERR_CONCURRENT_UPLOAD, "Concurrent upload"); ret = -1; @@ -4930,8 +4936,9 @@ put_file_with_condition (SeafRepoManager *mgr, repo->store_id, &gc_conflict); if (published < 0) { g_set_error (error, SEAFILE_DOMAIN, - published == -2 ? SEAF_ERR_GENERAL : - (gc_conflict ? SEAF_ERR_GC_CONFLICT : SEAF_ERR_CONCURRENT_UPLOAD), + gc_conflict ? SEAF_ERR_GC_CONFLICT : + (published == SEAF_BRANCH_UPDATE_HEAD_CONFLICT ? + SEAF_ERR_CONCURRENT_UPLOAD : SEAF_ERR_GENERAL), "Conditional no-op could not be committed"); ret = -1; goto out; @@ -6849,10 +6856,11 @@ seaf_repo_manager_revert_on_server (SeafRepoManager *mgr, const char *user_name, GError **error) { - SeafRepo *repo; + SeafRepo *repo = NULL; SeafCommit *commit = NULL, *new_commit = NULL; char desc[512]; int ret = 0; + int retry_cnt = 0; gboolean cf_prepared = FALSE; /* Outside the retry label: rewinding the whole library is one operation no @@ -6869,7 +6877,8 @@ seaf_repo_manager_revert_on_server (SeafRepoManager *mgr, if (!repo) { g_set_error (error, SEAFILE_DOMAIN, SEAF_ERR_GENERAL, "No such repo"); - return -1; + ret = -1; + goto out; } commit = seaf_commit_manager_get_commit (seaf->commit_mgr, @@ -6903,17 +6912,27 @@ seaf_repo_manager_revert_on_server (SeafRepoManager *mgr, } seaf_branch_set_commit (repo->head, new_commit->commit_id); - if (seaf_branch_manager_test_and_update_branch (seaf->branch_mgr, - repo->head, - new_commit->parent_id, - FALSE, NULL, NULL, NULL) < 0) - { - seaf_repo_unref (repo); - seaf_commit_unref (commit); - seaf_commit_unref (new_commit); - repo = NULL; - commit = new_commit = NULL; - goto retry; + int published = seaf_branch_manager_test_and_update_branch (seaf->branch_mgr, + repo->head, + new_commit->parent_id, + FALSE, NULL, NULL, NULL); + if (published < 0) { + if (published == SEAF_BRANCH_UPDATE_HEAD_CONFLICT && retry_cnt < 3) { + ++retry_cnt; + seaf_repo_unref (repo); + seaf_commit_unref (commit); + seaf_commit_unref (new_commit); + repo = NULL; + commit = new_commit = NULL; + g_usleep (g_random_int_range (10, 51) * 1000); + goto retry; + } + g_set_error (error, SEAFILE_DOMAIN, + published == SEAF_BRANCH_UPDATE_HEAD_CONFLICT ? + SEAF_ERR_CONCURRENT_UPLOAD : SEAF_ERR_GENERAL, + "Revert branch update rejected or unavailable"); + ret = -1; + goto out; } CF_FILEOP_COMMITTED (CF_OP_REVERT_REPO, From 6046dcc78e517344196a65c40e64482d0d982e67 Mon Sep 17 00:00:00 2001 From: servicememeai Date: Fri, 23 Oct 2026 18:11:36 +0900 Subject: [PATCH 5/8] test(branch): guard no-op metadata and bounded HEAD-only retries --- tests/test_cf_branch_retry_contract.py | 58 ++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 tests/test_cf_branch_retry_contract.py diff --git a/tests/test_cf_branch_retry_contract.py b/tests/test_cf_branch_retry_contract.py new file mode 100644 index 00000000..f2375353 --- /dev/null +++ b/tests/test_cf_branch_retry_contract.py @@ -0,0 +1,58 @@ +"""Cheap regression guard for C Branch CAS return codes and retry boundaries. + +This is a source-level contract test. The native ACL integration harness still +requires an isolated MariaDB/Hub/C-server environment before merge. +""" +from pathlib import Path +import unittest + +ROOT = Path(__file__).resolve().parents[1] + + +def function(path, marker, until): + source = (ROOT / path).read_text(encoding="utf-8") + assert marker in source and until in source, path + return source.split(marker, 1)[1].split(until, 1)[0] + + +class BranchRetryContract(unittest.TestCase): + def test_only_head_mismatch_is_classified_retryable(self): + hdr = (ROOT / "common/branch-mgr.h").read_text() + branch = function("common/branch-mgr.c", + "test_and_update_branch (SeafBranchManager *mgr,", + "\nint\nseaf_branch_manager_test_and_update_branch (") + self.assertIn("#define SEAF_BRANCH_UPDATE_HEAD_CONFLICT (-3)", hdr) + self.assertIn("return SEAF_BRANCH_UPDATE_HEAD_CONFLICT;", branch) + self.assertIn("return SEAF_BRANCH_UPDATE_ACL_DENIED;", branch) + self.assertIn("return -1;", branch) # SQL/transaction failure stays distinct + + def test_metadata_noop_and_bounded_retry(self): + method = function("server/repo-mgr.c", "seaf_repo_manager_edit_repo (", + "\ngboolean\nget_total_file_number_cb") + self.assertIn("g_strcmp0 (parent->repo_name, effective_name)", method) + self.assertIn("g_strcmp0 (parent->repo_desc, effective_description)", method) + self.assertLess(method.index("g_strcmp0 (parent->repo_name"), + method.index("seaf_commit_new (")) + self.assertIn("published == SEAF_BRANCH_UPDATE_HEAD_CONFLICT && retry_cnt < 3", method) + self.assertIn("g_set_error (error", method) + self.assertIn("ret = -1;\n goto out;", method) + self.assertNotIn("if (!user) {\n user = parent->creator_name;", method) + + def test_file_commit_retries_only_head_conflicts(self): + method = function("server/repo-op.c", "gen_new_commit_guarded (", + "\nstatic int\ngen_new_commit (") + self.assertIn("if (published != SEAF_BRANCH_UPDATE_HEAD_CONFLICT)", method) + self.assertIn("if (published == SEAF_BRANCH_UPDATE_ACL_DENIED)", method) + self.assertIn("MAX_RETRY_COUNT 10", method) + + def test_revert_retries_are_bounded_and_always_abort_on_error(self): + method = function("server/repo-op.c", + "seaf_repo_manager_revert_on_server (", + "\nstatic void\nadd_deleted_entry") + self.assertIn("published == SEAF_BRANCH_UPDATE_HEAD_CONFLICT && retry_cnt < 3", method) + self.assertIn("CF_FILEOP_ABORTED (CF_OP_REVERT_REPO", method) + self.assertIn("ret = -1;\n goto out;", method) + + +if __name__ == "__main__": + unittest.main() From 30a53e2ac8f9be179a764348721f61db63182f13 Mon Sep 17 00:00:00 2001 From: servicememeai Date: Fri, 23 Oct 2026 18:17:05 +0900 Subject: [PATCH 6/8] ci(quick): run targeted Branch and ACL source contracts without builds --- .github/workflows/quick.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/quick.yml b/.github/workflows/quick.yml index f527cbb1..a86d8d9e 100644 --- a/.github/workflows/quick.yml +++ b/.github/workflows/quick.yml @@ -23,6 +23,9 @@ jobs: - name: Block retired managed-library code and contracts run: python3 tests/test_v2_no_managed_library.py + - name: Run cheap C-source ACL and Branch retry contracts + run: python3 -m unittest discover -s tests -p 'test_cf_*.py' + - name: Validate only changed source shell: bash run: | From a2611181c798101c21a1e4d531ad0a545064031a Mon Sep 17 00:00:00 2001 From: servicememeai Date: Fri, 23 Oct 2026 18:19:08 +0900 Subject: [PATCH 7/8] fix(branch): do not classify a missing HEAD row as a retryable conflict --- common/branch-mgr.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/common/branch-mgr.c b/common/branch-mgr.c index 192d6a08..2bfbd0d3 100644 --- a/common/branch-mgr.c +++ b/common/branch-mgr.c @@ -518,13 +518,15 @@ test_and_update_branch (SeafBranchManager *mgr, default: g_return_val_if_reached (-1); } - if (seaf_db_trans_foreach_selected_row (trans, sql, - get_commit_id, commit_id, - 2, "string", branch->name, - "string", branch->repo_id) < 0) { + int head_rows = seaf_db_trans_foreach_selected_row (trans, sql, + get_commit_id, commit_id, + 2, "string", branch->name, + "string", branch->repo_id); + /* A missing/corrupt Branch row is not a confirmed HEAD race. */ + if (head_rows != 1) { seaf_db_rollback (trans); seaf_db_trans_close (trans); - return -1; + return SEAF_BRANCH_UPDATE_ERROR; } if (strcmp (old_commit_id, commit_id) != 0) { seaf_db_rollback (trans); From d5e9fff60c7203f614e40cae7bf623d10033f29c Mon Sep 17 00:00:00 2001 From: servicememeai Date: Fri, 23 Oct 2026 18:19:21 +0900 Subject: [PATCH 8/8] test(branch): missing Branch rows are operational failures, not conflicts --- tests/test_cf_branch_retry_contract.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_cf_branch_retry_contract.py b/tests/test_cf_branch_retry_contract.py index f2375353..c767231b 100644 --- a/tests/test_cf_branch_retry_contract.py +++ b/tests/test_cf_branch_retry_contract.py @@ -22,6 +22,8 @@ def test_only_head_mismatch_is_classified_retryable(self): "test_and_update_branch (SeafBranchManager *mgr,", "\nint\nseaf_branch_manager_test_and_update_branch (") self.assertIn("#define SEAF_BRANCH_UPDATE_HEAD_CONFLICT (-3)", hdr) + self.assertIn("if (head_rows != 1)", branch) + self.assertIn("return SEAF_BRANCH_UPDATE_ERROR;", branch) self.assertIn("return SEAF_BRANCH_UPDATE_HEAD_CONFLICT;", branch) self.assertIn("return SEAF_BRANCH_UPDATE_ACL_DENIED;", branch) self.assertIn("return -1;", branch) # SQL/transaction failure stays distinct