[codex] Structure VCS project config failures#3315
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Inspect failures abort config discovery
- Replaced Effect.mapError (which propagated the error and aborted the directory walk) with Effect.orElseSucceed(() => false) so that a failed fileSystem.exists is treated as 'not found' and the upward search continues to parent directories.
Or push these changes by commenting:
@cursor push 99690f3d5b
Preview (99690f3d5b)
diff --git a/apps/server/src/vcs/VcsProjectConfig.ts b/apps/server/src/vcs/VcsProjectConfig.ts
--- a/apps/server/src/vcs/VcsProjectConfig.ts
+++ b/apps/server/src/vcs/VcsProjectConfig.ts
@@ -62,17 +62,7 @@
let current = cwd;
while (true) {
const candidate = path.join(current, ".t3code", "vcs.json");
- const exists = yield* fileSystem.exists(candidate).pipe(
- Effect.mapError(
- (cause) =>
- new VcsProjectConfigError({
- operation: "inspect",
- cwd,
- configPath: candidate,
- cause,
- }),
- ),
- );
+ const exists = yield* fileSystem.exists(candidate).pipe(Effect.orElseSucceed(() => false));
if (exists) {
return Option.some(candidate);
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 69d4522. Configure here.
ApprovabilityVerdict: Approved This PR refactors error handling in VcsProjectConfig to use structured errors with better context (operation, cwd, configPath) for logging and debugging. Runtime behavior remains unchanged - errors still gracefully fall back to 'auto'. Tests thoroughly verify the new error structure. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
69d4522 to
0502c5e
Compare
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate 569a697


Summary:
Tests:
Note
Low Risk
Changes are localized to VCS config discovery with preserved
autofallback; main effect is new warning logs on previously silent failures.Overview
VCS project config resolution now surfaces inspect, read, and decode failures as
VcsProjectConfigError(operation,cwd,configPath, original cause) instead of silently treating them as missing or invalid config.resolveKindstill falls back toauto, but emits structured warnings vialogVcsProjectConfigError. JSON parsing usesdecodeUnknownEffectso decode failures are typed errors, notOption.none. During parent walk, a failed exists check on a candidate path is logged and skipped so a valid parent.t3code/vcs.jsoncan still be used.Tests cover error shape, inspect-then-parent behavior, and warning log fields for malformed JSON and unreadable paths.
Reviewed by Cursor Bugbot for commit 569a697. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Structure VCS project config failures with typed errors and structured logging
VcsProjectConfigError, a tagged error class carryingoperation('inspect' | 'read' | 'decode'),cwd,configPath, andcause, replacing silentOption.nonefallbacks with typed failures.logVcsProjectConfigErrorto emit structured warnings with operation, path, and stack context whenever config inspection, reading, or decoding fails.findConfigPathnow catches inspect errors, logs them, and continues traversal to parent directories instead of aborting.resolveKindcatches allVcsProjectConfigErrorvariants, logs a structured warning, and falls back to'auto'rather than propagating the failure.Option.none; all failure paths now produce log output.Macroscope summarized 569a697.