Fix errors discovered in release 0.19.8 - #13366
Merged
Merged
Conversation
mtsgrd
requested review from
Byron,
Caleb-T-Owens,
estib-vega,
jonathantanmy2 and
krlvi
as code owners
April 17, 2026 20:41
github-actions
Bot
added
rust
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes multiple regressions/edge-cases uncovered in release 0.19.8, spanning legacy settings deserialization, stack-detail lookups returning stale/missing data, a Svelte modal lifecycle race, and workspace-commit graph constraints.
Changes:
- Make
stack_details_v3returnOption<StackDetails>for missing/removed stacks and propagate that through Rust + desktop RTK Query callers (treatingnullas an error state in the subscription). - Harden forge settings deserialization to ignore legacy bare-string account entries rather than crashing on load.
- Relax
parents_must_be_referencesfor workspace commits when a parent’s branch ref disappears, and simplify modal show/close logic to avoid teardown races.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/gitbutler-cli/src/command/vbranch.rs | Adjusts CLI stack listing to skip missing stack details (now Option). |
| crates/gitbutler-branch-actions/tests/branch-actions/support.rs | Updates test helper to unwrap Option<StackDetails> with an explicit expectation. |
| crates/gitbutler-branch-actions/src/upstream_integration.rs | Propagates optional stack details into upstream integration status logic. |
| crates/gitbutler-branch-actions/src/branch_upstream_integration.rs | Converts missing stack details into a user-facing error for integration-step generation. |
| crates/but/src/lib.rs | Updates legacy CLI paths to handle Option<StackDetails> (flattening Result<Option<_>>). |
| crates/but/src/legacy/commits.rs | Changes legacy stack_details wrapper to return Option. |
| crates/but/src/command/legacy/rub/undo.rs | Adjusts stack scanning logic to ignore missing stack details. |
| crates/but/src/command/legacy/push.rs | Skips stacks whose details are now missing (None) during push-related operations. |
| crates/but/src/command/legacy/pull/mod.rs | Handles Option stack details when checking for conflicted commits after integration. |
| crates/but/src/command/legacy/forge/review.rs | Treats missing stack details as “no default commit” and guards optional lookups. |
| crates/but/src/command/legacy/commit.rs | Flattens optional stack details and turns unexpected None into an error for new stack creation. |
| crates/but/src/command/legacy/clean.rs | Skips stacks with missing details while finding empty branches. |
| crates/but-workspace/tests/workspace/ref_info/with_workspace_commit/legacy.rs | Adds coverage ensuring nonexistent stack IDs return Ok(None). |
| crates/but-workspace/tests/workspace/ref_info/mod.rs | Updates test helper wrapper to expect Some in tests after API change. |
| crates/but-workspace/src/legacy/stacks.rs | Changes stack_details_v3 to Result<Option<StackDetails>> and returns None for missing metadata entries. |
| crates/but-testing/src/command/mod.rs | Makes but-testing stacks details print a friendly message when stack is missing. |
| crates/but-rebase/tests/rebase/graph_rebase/workspace_commit_behaviour.rs | Adds regression test for deleted branch ref relaxing parents_must_be_references. |
| crates/but-rebase/src/graph_rebase/creation.rs | Relaxes workspace commit constraint when parent paths don’t go through reference nodes. |
| crates/but-forge-storage/src/settings.rs | Adds custom account list deserializers that ignore legacy bare-string entries + tests. |
| crates/but-claude/src/hooks/mod.rs | Handles missing stack details as “not eligible” in rename eligibility logic. |
| crates/but-cherry-apply/tests/cherry_apply/conflicts_with_bar.rs | Updates tests for optional stack details return type. |
| crates/but-cherry-apply/tests/cherry_apply/clean_to_both.rs | Updates tests for optional stack details return type. |
| crates/but-api/src/legacy/workspace.rs | Changes API to return Option<StackDetails> and maps None through NAPI/JSON as null. |
| crates/but-api/src/legacy/forge.rs | Skips stacks whose details are missing when warming CI check caches. |
| crates/but-api/src/legacy/absorb.rs | Adapts stack-details lookups to Option across absorb logic. |
| apps/desktop/src/lib/stacks/stackEndpoints.ts | Treats null stack_details responses as query errors to handle stale subscriptions. |
| apps/desktop/src/components/views/GlobalModalRouter.svelte | Removes redundant modal.close() call to avoid teardown race and “null close” errors. |
krlvi
reviewed
Apr 18, 2026
| Ok(account) => Some(account), | ||
| Err(_) if v.is_string() => None, // known legacy bare-string format | ||
| Err(err) => { | ||
| eprintln!("warning: discarding unrecognised account entry: {err}"); |
There was a problem hiding this comment.
Using eprintln! for unexpected account entries will write to stderr unconditionally (including in GUI contexts/tests) and can’t be filtered/disabled. Prefer routing this through the project’s logging facilities (e.g. tracing/log) or returning a structured error so callers can decide how to surface it.
Suggested change
| eprintln!("warning: discarding unrecognised account entry: {err}"); | |
| tracing::warn!("discarding unrecognised account entry: {err}"); |
| async function onDeleteClicked() { | ||
| isDeleting = true; | ||
|
|
||
| let remainingProject; |
There was a problem hiding this comment.
let remainingProject; introduces an implicit any under strict: true TypeScript settings, which should fail type-checking. Give this a concrete type (e.g. Project | undefined) and type the projects result accordingly so the .id access is checked.
| popd | ||
|
|
||
| # Shallow-clone with only the latest commit. | ||
| git clone --depth 1 remote-project local-clone |
There was a problem hiding this comment.
git clone --depth 1 remote-project local-clone will typically ignore --depth for local path clones (it performs a local/hardlink clone). That means this script may not actually create a shallow repo, so the e2e scenario won’t exercise the intended behavior. Use a file:// URL (or pass --no-local) so the clone is truly shallow.
Suggested change
| git clone --depth 1 remote-project local-clone | |
| git clone --depth 1 "file://$(pwd)/remote-project" local-clone |
Comment on lines
+83
to
+92
| // In shallow repositories, the traversal may hit a commit whose parent | ||
| // objects are not present locally. Stop rather than propagating the error. | ||
| let info = match commit_info { | ||
| Ok(info) => info, | ||
| Err(_) => break, | ||
| }; | ||
| let commit = match info.id().object() { | ||
| Ok(obj) => obj.into_commit(), | ||
| Err(_) => break, | ||
| }; |
There was a problem hiding this comment.
log_target_first_parent now breaks on any per-commit traversal error (Err(_) => break) and on any id().object() error. That will silently truncate history not only for shallow clones, but also for unrelated corruption/IO errors, making real issues harder to detect. Consider only stopping early for the specific “object not found due to shallow” error kind and otherwise returning the error (or at least logging the unexpected error before breaking).
Caleb-T-Owens
approved these changes
Apr 21, 2026
Users who had accounts stored in an older format (plain username strings instead of typed objects) would crash on any operation that reads known_accounts (clear_all_gitlab_tokens, list_known_gitlab_accounts, list_known_github_accounts). Add a lenient deserializer that silently skips bare strings and warns on other unparseable entries.
When a branch ref is deleted or force-pushed after the workspace commit was created, the workspace commit retains the old branch tip as a parent but without a corresponding Reference node in the graph. This caused the rebase to fail with 'parents not referenced'. Affects <1% of v0.19.8 users. Detect this at graph construction time and relax the constraint so the rebase can proceed.
Regenerate the GitButler error analysis covering ~2026-04-19 23:00 UTC to 2026-04-20 11:15 UTC and add it as error-analysis-2026-04-20.txt. This summarizes PostHog and Sentry metrics, lists top errors for v0.19.8 and older versions, details user-visible toast errors, highlights Sentry noise from e2e runs, and records fixes pending release plus newly identified issues (notably BUG(opt-stack-id) and a diff/remove-resources bug). The report is intended to inform triage and track fixes pending release.
When gix's prepare_diff() encounters two resources that are both considered removed (e.g. from rename chains followed by deletion), it errors with 'Tried to diff resources that are both considered removed'. This surfaced as ~3.2% of v0.19.8 users in the last 24h. Catch this specific error variant (SourceAndDestinationRemoved) and return None (no diff) instead of propagating the error, matching the existing pattern for InvalidMode and ConvertToDiffable failures.
Child components in GlobalModalRouter were calling modal?.close()
directly, which races with the {#if modalProps} block unmount. When
modalProps becomes falsy the Modal binding goes null, and if a close
callback fires during teardown it hits 'null is not an object
(evaluating u.close)'. This affected ~1.8% of v0.19.8 users in the
last 24h.
Replace all modal?.close() / closeModal callbacks with handleModalClose,
which sets modalProps to undefined via the UI state store, letting
Svelte's {#if} block handle the unmount cleanly without touching the
modal binding directly.
Replace the cryptic 'Reference X cannot be created as segment at Y'
with a user-friendly message explaining that the target commit already
belongs to another branch. The old message leaked internal terminology
('segment') that meant nothing to users. Affects <1% of v0.19.8 users
who try to create branches targeting commits already claimed by existing
workspace branches.
CI failures were caused by modal teardown races where the portalled DOM wasn't being cleaned up with its closing animation. Introduce closeModal() that calls the Modal component's close() method when the modal ref exists, and fall back to the prior handleModalClose() behavior when it does not. Replace direct calls to handleModalClose with closeModal for various modal content components to ensure consistent, animated cleanup and avoid unmount races.
Navigate to the remaining project (or welcome page) before calling deleteProject so the [projectId] layout unmounts and its queries are cleaned up. Previously, deleteProject's cache invalidation caused AppLayout's getProject to refetch and fail with "project not found", showing an error page before goto() could complete the route transition. Handle navigation correctly when deleting projects Fix deletion flow to avoid app errors by deciding navigation based on whether another project exists and ensuring deletion and navigation happen in the correct order. If another project exists, navigate to it first so the [projectId] layout unmounts and its queries are cleaned up, then delete the project. If this is the last project, delete it first (to avoid root redirecting back) and then navigate to the root. Also centralize error handling and isDeleting state updates to prevent premature state changes and CI failures. Handle project deletion navigation and robust settings deserialization Ensure correct navigation when deleting projects by selecting another project before deletion when possible, or deleting the last project first and then navigating to root. This avoids stale layout queries refetching a deleted project and centralizes success toast handling and deletion state cleanup. Update settings deserialization to accept null and non-array inputs: treat null known-accounts as empty, warn (via tracing) and discard malformed entries instead of printing to stderr. Also add tracing to workspace dependencies and include a test for null known_accounts handling. Refetch projects after deletion Ensure the UI doesn't see stale project data after removing the last project by fetching the project list immediately after deletion. This prevents the root page from failing to redirect to /onboarding due to a cached empty or outdated list.
In shallow repositories, walking the first-parent chain may hit commits whose parent objects are not present locally. Previously this propagated the error and failed the entire operation. Now we stop the traversal gracefully and return the commits collected so far. Add an integration test that creates a shallow clone and calls log_target_first_parent, verifying it stops gracefully instead of erroring when parent objects are missing.
Relax strict parent-reference validation by removing the parents_must_be_references flag, its validation, and the all_parents_are_references utility and tests. The workspace-commit-specific workaround that relaxed the flag during graph construction was also removed, and tests that enforced the old restriction were updated to reflect that a workspace commit with deleted branch refs now rebases successfully. This is needed because the previous constraint caused spurious rebase failures when branch refs were deleted or force-pushed, and simplifying the model avoids those errors and clarifies test expectations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes for the top user-facing errors discovered in v0.19.8 via PostHog and Sentry analysis.
Fixes included
Handle legacy bare-string forge accounts in settings deserialization (PostHog)
invalid type: string "...", expected internally tagged enum GitLabAccountRelax
parents_must_be_referencesfor workspace commits with deleted refs — <1% of users (PostHog)Commit X has parents that are not referencedBUG: should have found upstream workspace segmentHandle diff of two removed resources gracefully — ~3.2% of users (PostHog)
Tried to diff resources that are both considered removedprepare_diff::Error::SourceAndDestinationRemovedvariant and returnsOk(None)instead of propagating the errorFix modal close TypeError — ~1.8% of users (PostHog)
null is not an object (evaluating 'u.close'){#if}block unmounts the modal component before.close()callback executescloseModal()which callsmodal.close()when available, falling back to direct state cleanup when the ref is nullImprove error message when branch creation fails due to commit ownership — <1% of users (PostHog)
Reference 'X' cannot be created as segment at YAvoid navigation race when deleting a project (CI)
projectOffboardingtests, not from user reportsdeleteProject()so the layout unmounts and its queries are cleaned up, preventing a brief "project not found" error pageHandle shallow repository traversal in
log_target_first_parent(CI)Also included
Test plan
cargo testpasses for modified crates🤖 Generated with Claude Code