mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-04 10:48:20 +00:00
fix(windows): report why an atomic publish failed instead of blaming the repo
#1620: every index_repository run fails, every pass succeeds, the worker exits 0, no error-level line appears anywhere, and the user is told: Pipeline failed. Check repo_path exists and contains source files. The repository is fine. The publish is failing on a filesystem permission problem, and two separate defects conspired to hide it. cbm_rename_replace discarded GetLastError() on Windows and returned a bare CBM_NOT_FOUND. Callers then log errno - see finalize.rename_failed - so the one field that should explain the failure held whatever an unrelated CRT call left there. The Win32 error is now translated to errno, with ERROR_ACCESS_DENIED the interesting case: MoveFileEx needs DELETE on the destination, which a cache file created under an empty or foreign DACL does not grant. cbm_pipeline_finalize_staged_generation returned PERSIST_FAILED on a failed sidecar removal with no log at all. That is the silent path: a publish that fails must say so. Neither change fixes the underlying ACL problem. Together they turn a report that misdirects the user at their own repository into one naming a permission failure on a specific file - which is the difference between an unactionable issue and a five-minute one. platform + pipeline suites: 268 passed, 0 failed. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
1 parent
c0bd4bbf8d
commit
10b935cf47
2 files changed
+57
-3
No files matched your search
@@ -1039,9 +1039,54 @@ int cbm_rename_replace(const char *src, const char *dst) {
|
||||
wchar_t *wdst = cbm_path_to_wide(dst);
|
||||
int ret = CBM_NOT_FOUND;
|
||||
if (wsrc && wdst) {
|
||||
ret = MoveFileExW(wsrc, wdst, MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH)
|
||||
? 0
|
||||
: CBM_NOT_FOUND;
|
||||
if (MoveFileExW(wsrc, wdst, MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH)) {
|
||||
ret = 0;
|
||||
} else {
|
||||
/* Translate the Win32 error into errno so callers can report WHY.
|
||||
*
|
||||
* Callers log `errno` after a failed rename (see
|
||||
* finalize.rename_failed in the pipeline). Without this the value
|
||||
* is whatever happened to be left there by an unrelated CRT call,
|
||||
* so on Windows the one field that should explain an atomic-publish
|
||||
* failure was noise. #1620 is exactly that: an ACL problem surfaced
|
||||
* to the user as "Pipeline failed. Check repo_path exists and
|
||||
* contains source files" — blaming their repository — because
|
||||
* ERROR_ACCESS_DENIED never reached the log.
|
||||
*
|
||||
* ERROR_ACCESS_DENIED is the interesting one here: MoveFileEx needs
|
||||
* DELETE on the destination, which a cache file created under an
|
||||
* empty or foreign DACL does not grant. */
|
||||
DWORD error = GetLastError();
|
||||
switch (error) {
|
||||
case ERROR_ACCESS_DENIED:
|
||||
case ERROR_WRITE_PROTECT:
|
||||
errno = EACCES;
|
||||
break;
|
||||
case ERROR_FILE_NOT_FOUND:
|
||||
case ERROR_PATH_NOT_FOUND:
|
||||
errno = ENOENT;
|
||||
break;
|
||||
case ERROR_SHARING_VIOLATION:
|
||||
case ERROR_LOCK_VIOLATION:
|
||||
case ERROR_USER_MAPPED_FILE:
|
||||
errno = EBUSY;
|
||||
break;
|
||||
case ERROR_NOT_SAME_DEVICE:
|
||||
errno = EXDEV;
|
||||
break;
|
||||
case ERROR_DISK_FULL:
|
||||
errno = ENOSPC;
|
||||
break;
|
||||
case ERROR_INVALID_NAME:
|
||||
case ERROR_FILENAME_EXCED_RANGE:
|
||||
errno = ENAMETOOLONG;
|
||||
break;
|
||||
default:
|
||||
errno = EIO;
|
||||
break;
|
||||
}
|
||||
ret = CBM_NOT_FOUND;
|
||||
}
|
||||
}
|
||||
free(wsrc);
|
||||
free(wdst);
|
||||
|
||||
@@ -1806,6 +1806,15 @@ int cbm_pipeline_finalize_staged_generation(char *stage_path, const char *final_
|
||||
struct timespec t_fin;
|
||||
cbm_clock_gettime(CLOCK_MONOTONIC, &t_fin);
|
||||
if (cbm_remove_db_sidecars(stage_path) != 0) {
|
||||
/* This returned PERSIST_FAILED with no log at all, which is how #1620
|
||||
* presented: every pass succeeded, the worker exited 0, no error-level
|
||||
* line was emitted anywhere, and the user was told "Pipeline failed.
|
||||
* Check repo_path exists and contains source files" — pointed at their
|
||||
* repository for a filesystem permission problem. A publish that fails
|
||||
* must say so. */
|
||||
char errno_text[16];
|
||||
(void)snprintf(errno_text, sizeof(errno_text), "%d", errno);
|
||||
cbm_log_error("finalize.sidecar_removal_failed", "errno", errno_text, "stage", stage_path);
|
||||
discard_generation_stage(stage_path);
|
||||
return CBM_PIPELINE_PERSIST_FAILED;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user