Wire-up RefInfo change-ids in commits to UI (#GB-1165)#13151
Wire-up RefInfo change-ids in commits to UI (#GB-1165)#13151Byron merged 11 commits intogitbutlerapp:masterfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
@codex is attempting to deploy a commit to the GitButler Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Wires GitButler commit change-ids through the but-workspace UI-facing commit types so the desktop/web UIs can display and use them when available.
Changes:
- Add
change_id: Option<String>toui::Commitandui::UpstreamCommit. - Populate
change_idfrom commit headers across multiple code paths (gix commits,but_core::CommitOwned, ref_info conversions). - Propagate
change_idinto legacy stack commit output and branch-details commit traversal output.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/but-workspace/src/ui/mod.rs | Adds change_id to UI commit structs and populates it in conversion impls. |
| crates/but-workspace/src/legacy/stacks.rs | Includes change_id in legacy stack ui::Commit construction. |
| crates/but-workspace/src/branch_details.rs | Extracts change_id during local/upstream commit traversal for branch details output. |
279f1ad to
757406f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcee53ad05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This makes the change-ids usable. Note that they won't be stable unless the rebase engine uses the ones in the cache instead of persisting. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
This also introduces a separate schema version for the project cache, which certainly change more easily as it never results in caches not coming up. This works as failed migrations will lead to the cache db to be deleted, and restarted from scratch, so older applications can always restore something usable without falling back to in-memory caches. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Also make it easy to fill headers with a `change-id` if CommitId is initially known. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com> Co-authored-by: GPT 5.4 <codex@openai.com>
In future, we might want to parameterise it accordingly to create SHA256 style IDs. In practice, it probably doesn't matter though.
Co-authored-by: GPT 5.4 <codex@openai.com>
- clarify ChangeIDs - they actually have a hash-independent format.
This means that newer caches versions that are incompatible won't be overwritten or cleared by older clients. They will instead open in memory, which seems like it's desirable behaviour. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 57 out of 60 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
crates/but-core/src/change_id.rs:35
- The doc comment says the generated change-id is “(SHA-1)”, but
ChangeId::generate()is 16 random bytes encoded as 32 reverse-hex characters; it isn’t a SHA-1 hash (SHA-1 would be 20 bytes / 40 hex chars). Consider rephrasing to avoid implying a cryptographic hash algorithm is used (e.g., just “32-char reverse-hex ChangeId” or “16-byte random ChangeId encoded as reverse-hex”).
/// Creates a random length 32 reverse hex ChangeId (SHA-1).
pub fn generate() -> Self {
let mut rng = rand::rng();
let bytes: [u8; CHANGE_ID_REVERSE_BYTE_LEN] = rng.random();
ChangeId::from_bytes(&bytes)
This makes the change-ids usable in the UIs.
Note that they won't be stable unless the rebase engine also persists synthetic IDs instead of creating random ones, but that needs more work in a couple of well-known places. This is left as a follow-up.
UpstreamCommitis still usingOptionfor itschange_id, as I don't yet know how important it is to provide one (after all, it's not 'our' commits anyway and we probably shouldn't rebase them).It's notable that the backend still keeps
Optionto be sure places like integration-checks won't be negatively affected. There, I also prefer keeping the presence of a change-id explicit as it's easy to synthesize one when that is truly needed (and for the backend, there is no case for it yet).Tasks
cachefor ChangeIDs but instead produce them deterministically like JJ.Notes for the reviewer
These change-ids won't be fully functional as they are not currently persisted by the rebase-engine and when creating new commits. Thus they will not be stable. Also, they don't implement the
/Xsuffix for when there is known duplicates.