neiam
/gitgud
Git Gud
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Badges · Packages
⭐
1
Log in to mark this repository.
Document slice 74: the merge editor, the 500, the seeds
1ecec4a · Gabriel Morell · 2026-09-12 06:08
Message
{commit_body(@commit)}
Files changed
modified
PROGRESS.md
+27
−0
@@ -12,6 +12,33 @@ intermediate "I was here when I stopped" state) lives in the
| 12 | 12 | |
| 13 | 13 | --- |
| 14 | 14 | |
| 15 | +## Slice 74 — Reconcile divergence in the browser | |
| 16 | + | |
| 17 | +The 3-way merge editor the roadmap had been carrying, plus the two | |
| 18 | +things that fell out of building it. | |
| 19 | + | |
| 20 | +### One editor, two callers, its own route | |
| 21 | + | |
| 22 | +- **`/r/:owner/:name/merge?into=<ref>&from=<ref or sha>[&pr=<number>]`** — a route of its own rather than a tab on the pull request, because the second caller has no pull request. A PR whose merge conflicts and a branch that moved both here and upstream are the same problem with different back-links; inside the PR page the upstream case would have been a second-class citizen. Both entry points link into it: the PR page when `mergeable == false`, and the upstream settings panel beside "Take upstream's side". | |
| 23 | +- **`GitGud.Merges.analyze/3`** answers what an editor needs where `merge_tree/4` only answers what a *button* needs: the merge base, a written tree, and one entry per unresolved path carrying all three sides plus git's own marked-up merge to open. It reads `git merge-tree -z` as records rather than pattern-matching its prose, which is what keeps `add/add` and `modify/delete` from flattening into "content" — the shape of the nils *is* the kind of conflict. Binary conflicts are flagged rather than handed to a text editor; unrelated histories are refused outright. | |
| 24 | +- **`GitGud.Merges.resolve/4`** takes `%{path => :ours | :theirs | {:text, …}}`, writes the tree, commits with **both parents** — so the branch merges cleanly afterwards instead of re-conflicting — and moves the ref conditionally, losing cleanly to a push that lands mid-resolution. | |
| 25 | +- **Two refusals, both deliberate.** A partial resolution is rejected rather than guessed at, and so is content that still carries conflict markers: committing a file with `<<<<<<<` in it is the exact failure this whole flow exists to prevent, and it is far cheaper to catch at the button than in a `git blame` next month. | |
| 26 | +- **Branch protection applies.** Resolving in a browser is still a ref update, and a rule that would refuse it from a push or a PR merge refuses it here. | |
| 27 | +- Two new backend ops underneath, `write_blob/2` and `write_tree_with/3`, both temp-index plumbing in the Cli backend. `update-index --force-remove` refuses to run without a work tree even though it only touches the index, so it gets an empty directory to satisfy that. | |
| 28 | + | |
| 29 | +### A 500 the logs found, that nobody signed in could see | |
| 30 | + | |
| 31 | +- **`assign(:can_admin?, user && admin?(user, repo))` is `nil`, not `false`,** with no user — and the run page writes `@can_admin? and job.status in [...]`. A nil on the left of `and` is a `BadBooleanError`, so *every signed-out view of a workflow run page* returned 500. Signed in as an admin the assign is a real boolean and the page renders, which is exactly why it survived: the people who hit it were the people who could look but not touch. | |
| 32 | +- Fixed where the value is produced — `assign_chrome/2` and the run page's own mount assign the boolean their attr already declares — and in the two org pages carrying the same `user && …` shape, before something writes `and` next to them. The regression test loads a public repo's run page signed out, as a signed-in non-admin, and as an admin; the first two fail without the fix. | |
| 33 | + | |
| 34 | +### `mix gitgud.seed.conflicts` | |
| 35 | + | |
| 36 | +- Dev-only, and it builds the situations rather than describing them: an unmergeable pull request carrying one of *each* kind of conflict at once (content, add/add, modify/delete, binary) alongside two files that merge cleanly and should stay out of the editor, and a repository linked to a local stand-in remote in reconcile mode with a fast-forwardable branch, a diverged one and a local-only one. | |
| 37 | +- Boots the app with the HTTP listener, metrics port and SSH daemon off, so it runs beside `mix phx.server` instead of fighting it for ports, and re-running replaces what the last run made — a seed you can't re-run is a seed you stop trusting. | |
| 38 | +- The binary fixture carries a real NUL byte rather than only a PNG magic number. The sniff looks for a zero in the first 8 KiB, so the obvious fixture renders as *editable text* — the opposite of what it exists to demonstrate. | |
| 39 | + | |
| 40 | +--- | |
| 41 | + | |
| 15 | 42 | ## Slice 73 — Import adoption, reconcile-mode upstream sync, listing timestamps |
| 16 | 43 | |
| 17 | 44 | (Slice 72 — matrix builds — shipped in the commit history but was never |
modified
README.md
+18
−4
@@ -43,10 +43,11 @@ reinvent them.
| 43 | 43 | - **Review flow** on its own tab: assigned reviewers (requested by handle, or from chips suggesting who actually reviews in this repo), one standing verdict per reviewer rather than a pile of contradictory rows, and the full log behind a disclosure |
| 44 | 44 | - **Line comments** anchored to a file and line, pinned to the head they were written against |
| 45 | 45 | - **Suggested changes** — a ```` ```suggestion ```` block renders as the proposed replacement with an Apply button that commits it to the source branch. Refuses rather than guesses: `:stale` if the branch moved since, and the ref update is a compare-and-swap so a concurrent push loses the race instead of being overwritten |
| 46 | + - **In-browser conflict resolution** at `/r/:owner/:name/merge` — every unresolved path with all three sides and git's own marked-up merge to edit, or take one side wholesale. Its own route, not a PR tab, because the other caller is an upstream branch that diverged and has no PR. Commits with both parents, so the branch merges cleanly afterwards; refuses a partial resolution, refuses content that still carries conflict markers, and honours branch protection — resolving in a browser is still a ref update | |
| 46 | 47 | - **Per-repo merge rules**, all off by default except the last: block merging while changes are requested, require N approving reviews, and stop authors approving their own PR (on for new repos). Repo admins can override, and an override is recorded as such |
| 47 | 48 | - **Editable titles and descriptions** on issues and PRs, plus editable and soft-deletable comments — deletion is soft so the history can still render what was edited before it went |
| 48 | 49 | - Forks + fork-sync (fast-forward CAS) + compare-any-two-refs view |
| 49 | − - **Repository import** from a GitHub org/user, a GitLab group (subgroups included) or user, or any single git URL — `git clone --mirror` per repo on its own background job, with a live progress page and an on-demand "fetch from upstream" re-sync. Git data only; issues and PRs don't come across. Provider tokens are encrypted at rest, handed to `git` out-of-band (never in the clone URL), and wiped when the import finishes | |
| 50 | + - **Repository import** from a GitHub org/user, a GitLab group (subgroups included) or user, or any single git URL — `git clone --mirror` per repo on its own background job, with a live progress page. A name that's already taken isn't automatically a collision: if the local repo and the remote share a root commit they're the same project, so the local one is **adopted** — pointed at that upstream for syncing, with no ref touched — and only genuinely unrelated history still skips. Re-sync is on demand or hourly (opt-in, anonymous), in one of two modes: `mirror` force-updates and prunes, `reconcile` fast-forwards what it can and reports the rest for a human. Git data only; issues and PRs don't come across. Provider tokens are encrypted at rest, handed to `git` out-of-band (never in the clone URL), and wiped when the import finishes | |
| 50 | 51 | - Branch protection, deploy keys, webhooks, wikis |
| 51 | 52 | - **Labels** with HTML color picker + 16-swatch palette + live preview; **org-level labels** inherited by every repo unless the repo opts out (and re-opts in via a hidden-labels expander) |
| 52 | 53 | - **Profiles** |
@@ -197,6 +198,18 @@ git push -u origin master
| 197 | 198 | identity carrier — the only way the SSH channel handler can identify |
| 198 | 199 | you between `is_auth_key` and the exec callback). |
| 199 | 200 | |
| 201 | +### Seeding something to click on | |
| 202 | + | |
| 203 | +```sh | |
| 204 | +mix gitgud.seed.conflicts # --owner <handle>, --prefix <name> | |
| 205 | +``` | |
| 206 | + | |
| 207 | +Builds an unmergeable pull request carrying one of each kind of conflict | |
| 208 | +(content, add/add, modify/delete, binary) and a repository linked to a | |
| 209 | +local stand-in upstream that has diverged — the two things the merge | |
| 210 | +editor exists for. Dev only; runs beside `mix phx.server` without | |
| 211 | +fighting it for ports, and re-running replaces what the last run made. | |
| 212 | + | |
| 200 | 213 | ## Configuration knobs |
| 201 | 214 | |
| 202 | 215 | These all live in `config/*.exs`. Key ones: |
@@ -578,8 +591,9 @@ with the release functions rather than duplicating it.
| 578 | 591 | |
| 579 | 592 | See [ROADMAP.md](./ROADMAP.md). Highlights: |
| 580 | 593 | |
| 581 | −- gix NIF wiring for log / commit / tree / diff_stats / merge_base / merge_tree (Cli fallback works; slice 56 fills in the hot paths) | |
| 582 | −- In-browser PR conflict resolution | |
| 594 | +- gix NIF wiring for `root_commits`, `diff` and `merge_analysis` — everything else is native; these still fall through to the Cli backend | |
| 595 | +- Rename detection in the merge editor: a rename conflict renders as the paths git names, not as "this file moved" | |
| 596 | +- Stored credentials for scheduled upstream sync — the scheduler is anonymous, so a private upstream reports "needs credentials" and waits for a human | |
| 583 | 597 | - Avatar file uploads (the theme-aware identicon is the avatar today) |
| 584 | 598 | - Native runner (low priority while the external one works) |
| 585 | 599 | - Code search (Postgres FTS handles issues/PRs; code search is its own beast) |
@@ -596,7 +610,7 @@ See [ROADMAP.md](./ROADMAP.md). Highlights:
| 596 | 610 | every consumer that pinned the old one |
| 597 | 611 | |
| 598 | 612 | See [PROGRESS.md](./PROGRESS.md) for the slice-by-slice history of how |
| 599 | −the forge was built, and what each shipped — currently through slice 58. | |
| 613 | +the forge was built, and what each shipped — currently through slice 74. | |
| 600 | 614 | |
| 601 | 615 | ## Project conventions |
| 602 | 616 |
modified
ROADMAP.md
+6
−5
@@ -9,9 +9,10 @@ See [PROGRESS.md](./PROGRESS.md) for what's done.
| 9 | 9 | |
| 10 | 10 | ## Now — in flight |
| 11 | 11 | |
| 12 | −Nothing in flight. Slice 73 (import adoption, reconcile-mode upstream | |
| 13 | −sync, the remaining gix NIF ops) closed most recently; slice 70 (private | |
| 14 | −package registries) is the substantial one before it. | |
| 12 | +Nothing in flight. Slice 74 (the in-browser merge editor) closed most | |
| 13 | +recently, on top of slice 73 (import adoption, reconcile-mode upstream | |
| 14 | +sync, the remaining gix NIF ops); slice 70 (private package registries) | |
| 15 | +is the substantial one before them. | |
| 15 | 16 | |
| 16 | 17 | Federation feature set is closed through slice 42 (reviews over AP |
| 17 | 18 | shipped in slice 38, suggested-blocks in slice 38, actionable reports |
@@ -32,7 +33,6 @@ queue in slice 42). Remaining federation items are discretionary
| 32 | 33 | | Item | Why | Size | |
| 33 | 34 | |---|---|---| |
| 34 | 35 | | Import issues / PRs / wikis alongside git data | Slice 59 imports git data only. Issues need author mapping for users who don't exist locally; PRs need ref rewriting | large | |
| 35 | −| Reconcile divergence in-browser | One 3-way merge editor, two callers: a PR whose merge conflicts, and an upstream branch that moved on both sides. Slice 73 built the detection and the non-merge resolutions — `upstream_status/2` classifies every branch, `refs/upstream-sync/*` holds upstream's tip so it stays mergeable, and "take upstream's side" is a button — plus the merge primitives it will run on (`merge_base`, `merge_tree`, `commit_tree`, `update_ref` are all native now). What's missing is the editor itself and the same shape for PR conflicts | medium-large | | |
| 36 | 36 | | Package signing key rotation | Keys mint themselves on first use and are re-wrapped by the `GitGud.Secrets` master-key rotation, but there is no "roll the keypair" flow — because rolling it breaks every consumer that pinned the old one. Needs a publish-both-then-retire story before it is safe | medium | |
| 37 | 37 | |
| 38 | 38 | ## Eventually — bigger / discretionary |
@@ -58,7 +58,8 @@ These are documented in the auto-memory under `project-git-gud-moderation-deferr
| 58 | 58 | |
| 59 | 59 | | Item | Notes | |
| 60 | 60 | |---|---| |
| 61 | −| `root_commits/1` + `diff/4` in the NIF | The last two ops still falling through to the Cli backend. Everything else is native as of slice 73 | | |
| 61 | +| `root_commits/1`, `diff/4` + `merge_analysis/4` in the NIF | The ops still falling through to the Cli backend. Everything else is native as of slice 73; `merge_analysis/4` is Cli-only because `git merge-tree -z` already answers it exactly, where gix reports conflicts in a shape that needs mapping across the boundary | | |
| 62 | +| Rename detection in the merge editor | `merge_analysis/4` reports what git reports, and git's rename detection is on by default — but the editor renders a rename conflict as the paths it names rather than as "this file moved". Worth a pass once someone hits one | | |
| 62 | 63 | | Test fixture cleanup | `tmp/test/repos/` wiped on suite start; consider per-test isolation if parallelism grows | |
| 63 | 64 | | Documentation as code grows | This file + PROGRESS.md; append on each slice close | |
| 64 | 65 | | `mix precommit` enforces warnings-as-errors | **Currently red**, and has been for a while — 17 sites across 14 files, in three families: unreachable clauses after a type got narrowed (`duration/2`, `format_deny/1`, `actor_name/1`, …), one unused `require Logger`, and genuine type mismatches where `attach_label/2` / `detach_label/2` / `check_review_block/3` are handed a map where a struct is expected. None is a runtime bug today, but the gate is worthless while it is red, and each new warning hides behind the existing noise. Worth one pass with `mix compile --force --warnings-as-errors` | |
Parents: 42a014c