Hardening the git server - deadlines, early conflicts, and push work that survives a restart

Five changes to denji and makima. Every git call now has a deadline and an output cap, a pull request learns it conflicts when a branch moves, a review survives a comment-only push, a push's follow-up work lives in Postgres until it is done, and clients get protocol v2 without Rezee's private refs.

Oct 17, 2026 · 20 min read · Kash Gohil · LinkedIn · X

Rezee is one workspace for issues, code, pull requests, CI/CD, docs, chat, design and agents. Under the Code & Ship layer sits denji, our git server in Go. The first post about it ended with a short list of what was missing. Git ran with no per-operation timeout. The FAQ said protocol v2 "works transparently". The SSH post corrected that: denji spoke v0 only. And the post-receive post said the durability of a push's follow-up work was "deliberately none".

This week we closed those items, and a few that real use found. There are five changes. Each one starts with a failure that we saw or that we could cause on purpose.

A hung git, a huge diff, a silent makima

Denji shells out to the git binary for everything. That is still the right decision. But until this week every call was exec.Command("git", ...) with no deadline, and its stdout went into a bytes.Buffer with no limit. Three things could hold a request open with no bound:

A git process that never exits. A stale lock, a slow disk, or a helper process that waits for input will do it.
A diff that is too large. A generated file or a vendored directory can produce hundreds of megabytes of patch. All of it went into memory before the handler wrote one byte.
A makima that does not answer. Denji asks makima to authorize every clone and push, and it used http.DefaultClient. That client has no timeout.

The fix is one wrapper that every read in the data layer now goes through:

func command(ctx context.Context, args ...string) *exec.Cmd {
	cmd := exec.CommandContext(ctx, "git", args...)
	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
	cmd.Cancel = func() error { return syscall.Kill(-cmd.Process.Pid, syscall.SIGTERM) }
	cmd.WaitDelay = 5 * time.Second
	return cmd
}

Two details in it matter. First, git often starts other processes, so killing only git leaves its children running. Denji puts git in its own process group and sends the signal to the whole group. The test for this uses a git alias that runs sleep 30. Without the group signal, the sleep keeps the pipe open and the call does not return. Second, the signal is SIGTERM, not SIGKILL. SIGTERM lets git remove its lock files. If git is still alive five seconds later, WaitDelay sends SIGKILL.

The output cap is a writer that stops the command when it fills:

// It must not embed bytes.Buffer: io.Copy would use its ReadFrom and skip the cap.
type cappedBuffer struct {
	buf  bytes.Buffer
	max  int64
	over bool
	stop context.CancelFunc
}

func (b *cappedBuffer) Write(p []byte) (int, error) {
	if int64(b.buf.Len()+len(p)) > b.max {
		b.over = true
		b.stop()
		return 0, ErrTooLarge
	}
	return b.buf.Write(p)
}

The comment on that type records the bug that we nearly shipped. os/exec copies a pipe into your writer with io.Copy. If the writer has a ReadFrom method, io.Copy calls it and never calls Write. An embedded bytes.Buffer gives the type a ReadFrom, so the cap would compile, pass a code review, and do nothing.

The defaults are 60 seconds and 64 MiB per command. DENJI_GIT_TIMEOUT and DENJI_GIT_MAX_OUTPUT change them. A value that does not parse stops denji at boot. We do not want a typo to become "no limit" without a message.

A limit is not a missing file

The limits found an older bug. Before this change, many reads turned every git failure into ErrNotFound. A diff that was too large to show came back as a 404, which tells the reader that the diff does not exist. Now a missing() helper lets the two limit errors through, and the HTTP layer gives each one its own status:

case errors.Is(err, gitdata.ErrTooLarge):
	http.Error(w, "too large to show", http.StatusRequestEntityTooLarge)
case errors.Is(err, gitdata.ErrTimeout):
	http.Error(w, "git took too long to answer", http.StatusGatewayTimeout)

A test holds this. It makes a 5,000-line change, sets the cap to 4 KiB, and requires ErrTooLarge from both compare calls, not ErrNotFound.

A second bug was in conflict resolution. Denji checks a proposed resolution for leftover conflict markers with git grep. Exit status 1 from git grep means "no match", which is the good answer. The old code treated every failure as "no match". A grep that crashed or timed out passed as a clean resolution. Now only exit status 1 means clean, and every other failure is an error.

The calls with no deadline

Some calls do not use the deadline, on purpose. Ref transactions (update-ref --stdin) take milliseconds, and a kill in the middle of one can leave some refs moved and some not. Clones, fetches, pushes, raw file downloads and the export bundle stream to the client. They do not collect output in memory, and a large clone on a slow link can take minutes for a good reason. The raw file path has its own size check before any bytes move.

The makima side got the same treatment. The authorize client now has a 10-second timeout. The post-receive hook's curl got -m 10, the same as the pre-receive hook. That last one needs a correction. In May we measured a 75-second push against a black-holed makima, and wrote that "the fix is one flag". The flag was not in the hook until this change. It is now.

Would these two commits merge?

Before this week, a pull request learned that it conflicted with its base when somebody pressed Merge. That is the latest possible moment, and often the person who presses Merge is not the person who can fix the conflict.

Now denji answers the question without a merge. The new endpoint is GET .../merge-check?base=&head=, and it runs the same command that the real merge runs:

cmd := command(ctx, "-C", dir, "merge-tree", "--write-tree", "--name-only",
	"--no-messages", "-z", "--end-of-options", baseSHA, headSHA)

Exit 0 is clean. Exit 1 is conflicted, and the output is a tree id and then one path for each conflicted file. Any other exit, unrelated histories included, is "no answer". Because the check and the merge use one command, they cannot disagree about a pair. The cost is that --write-tree writes a tree object that nothing references. Git's garbage collection removes it later, and below you will see that denji now runs gc on a schedule.

When the check runs

Makima runs the check every time a branch moves. That includes a push, a merge, a commit made in the browser, an applied review suggestion and a new branch. For each open pull request with the moved branch as its base or its head, makima asks for the pair's answer.

Take the demo workspace. kobeni merges web#128 into main. main moved, so makima checks every open pull request on main, and web#131 (kobeni/mask-env) is one of them. Suppose web#131 now conflicts in two files. The merge gate on web#131 says "Conflicts with main in 2 files" and lists the two paths. Nobody had to press anything.

The answer for two commits never changes, so the cache key is the pair and no row is ever invalidated:

CREATE TABLE IF NOT EXISTS pull_merge_checks (
  repo_id uuid NOT NULL REFERENCES repositories(id) ON DELETE CASCADE,
  base_sha text NOT NULL,
  head_sha text NOT NULL,
  merge_base_sha text,
  clean boolean NOT NULL,
  paths text[] NOT NULL DEFAULT '{}',
  truncated boolean NOT NULL DEFAULT false,
  checked_at timestamptz NOT NULL DEFAULT now(),
  PRIMARY KEY (repo_id, base_sha, head_sha)
);

A prune deletes rows older than a week, every six hours. A pruned pair that is still in use gets a new check on its next read. That costs one merge-tree and never a wrong answer. We cap the list at 500 paths and set truncated past that.

One rule about failure: no answer is not a conflict. If denji cannot answer, the gate shows no conflict blocker. The merge itself still refuses a conflict, so a failed check costs an early warning and nothing more.

A conflict becomes a case

A pull request that stops merging cleanly also gets a conflict case. Rezee already had cases for a rebase or a restack that stopped on conflicts. The inputs are pinned under refs/rezee/conflicts/, and any person or agent can claim the case and propose a fix. Pull requests are now a third source. A partial unique index allows one live case per pull request:

CREATE UNIQUE INDEX IF NOT EXISTS code_conflicts_pull_live_uq
  ON code_conflicts (pull_id)
  WHERE source = 'pull' AND state IN ('open', 'claimed', 'proposed');

The case id is a hash of the pull request and the commit pair. A retried check opens the same case, not a second one. When either branch moves again, the old case closes itself as abandoned, with the reason clean or moved and no actor in the audit row. Nobody decided to close it, and the record says so.

The resolution for a pull request's case is different from a restack's. A restack's resolution must have the new base as its first parent. A pull request's resolution must only contain both sides, which is what git merge origin/main on the pull's branch makes. When somebody accepts the resolution, makima moves the pull's branch to it. Before that, it checks two things. The resolution must keep every commit the branch had, so nobody rewrites a branch under review. And the move must pass the branch's own protection rules. The case page shows the five git commands that make such a commit. Our CLI, aki, has conflict claim, propose and accept, and agents get the same actions as MCP tools.

The field that disappeared

We shipped this, and the pull request page never listed the conflicted files. The gate's answer had a conflicts field, and makima computed it correctly. But makima's routes declare a response model, and the response model strips fields that it does not declare. Nobody added conflicts to the model. Every layer was correct alone, and the browser got nothing. The fix was four lines in models.ts, and a route test now requires the field in the response. We expect to make this mistake again, so the test asks the HTTP route, not the service function.

Did the code change, or only the comments?

A review in Rezee belongs to a version of the pull request. When the author pushes, older verdicts become stale and the page marks them "Code changed". That is correct for a logic change. For a fixed typo in a comment, or a reformatted file, it costs a second review of code that did not change.

Now a verdict carries to the new version when two things are true:

The left side of the diff is the same commit. The reviewer read a diff from a merge base, and if the merge base moved, the reviewer read a different diff.
Everything the head gained since then touched only comments or formatting.

Anything that the judges cannot be sure about counts as a change.

Judging Go in denji

Denji is written in Go, and Go's parser and scanner are in its standard library. So denji judges Go files. A new endpoint lists the files two commits differ in, from git diff --raw -z --no-renames. Only a plain modification gets judged. An added, deleted or renamed file, or a mode change, is a change. For a modified .go file, denji compares the token stream with comments removed:

for {
	_, tok, lit := s.Scan()
	if tok == token.EOF {
		break
	}
	if tok == token.COMMENT {
		if isGoDirective(lit) {
			directives.WriteString(lit)
			directives.WriteByte('\n')
		}
		continue
	}
	// The grammar lets a semicolon go before ) or }, which is exactly
	// where joining or splitting lines adds or removes one.
	if (tok == token.RPAREN || tok == token.RBRACE) && len(kept) > 0 && kept[len(kept)-1] == ";" {
		kept = kept[:len(kept)-1]
	}
	if tok == token.SEMICOLON {
		kept = append(kept, ";")
		continue
	}
	kept = append(kept, tok.String()+" "+lit)
}

The semicolon handling is the subtle part. Go inserts a semicolon at the end of a line when the line ends with an identifier, a literal, a closing bracket or a few other tokens. So line breaks are sometimes code. The test cases show the difference:

return a + and then b on the next line is the same as return a + b. The line ends with +, so Go inserts no semicolon.
return a and then +b on the next line is different. Go inserts a semicolon after a, and +b becomes a new statement.

Removing a semicolon before ) or } makes func Add(a, b int) int { return a+b } on one line equal the same function on three lines.

Some comments are code too. A comment that starts with //go:, // +build, //line, //export, //extern, //nolint or //lint: goes into the comparison, so a new //go:build linux line is a change. A file that imports "C" is never judged, because the cgo preamble is a comment that the compiler reads as C. A file that does not parse is a change. Files over 1 MiB, and pairs that touch more than 200 files, are changes too.

Judging TypeScript, JavaScript and JSON in makima

Makima runs on Bun and parses scripts with oxc-parser. For .ts, .tsx, .js, .jsx and their .mts/.cts/.mjs/.cjs forms, it compares the syntax tree with positions removed. It also removes the raw text of literals, because that text holds the quote style, and 'a' and "a" run the same. Comments are removed, except the ones a tool reads: @ts- directives, eslint, biome-ignore, prettier-ignore, __PURE__ annotations and a few more. JSON is parsed and written back, so whitespace goes and the order of keys stays. Every other file type is a change.

The answer for a commit pair goes into a cache table with the same shape as the merge check. Makima judges the step from the previous version when the push arrives. Then the merge gate does not judge it while somebody waits for the page to load. If denji is down, the answer is "changed" and makima does not cache it, because an absent denji says nothing about the code.

On the page, a carried review gets a "Still counts" chip. Its tooltip says "Only comments or formatting changed since this review". Some teams want a new review after every push, whatever it changed. Branch protection now has the option "Review again after comment or formatting changes". If any rule that matches the branch sets it, no verdict carries.

We made this deliberately strict. A renamed local variable is a change. A changed string literal is a change. A reformat with a tool that changes the syntax tree is a change. Each false "changed" costs one more review, and a false "same" lets unreviewed code merge. Those two costs are not equal.

A push's follow-up work, kept until it is done

When a ref moves, makima does a list of work. The list is an audit row, the pull request's diff version, issue links, the conflict check above, activity, the push webhook, doc watchers, pipeline definitions, the path index, API contract and lockfile indexes, and the CI run. Until this week, that list ran on a promise chain in memory:

export function refMovedLater(move: RefMoved): void {
	const lane = `${move.repo.ws}/${move.repo.name}:${move.ref}`;
	const running = (refLanes.get(lane) ?? Promise.resolve())
		.then(() => refMoved(move))
		.catch((err) => console.error("ref move failed", move.via, lane, err))
		...

The lane kept one ref's moves in order, and that part was good. But if makima restarted in the middle of the list, the rest of the list was lost. The push was on disk. The CI run, the pull request version and the webhooks were not, and nobody got a message about it.

Now each move is a row in ref_moves, and makima writes the row before it answers the hook. The internal endpoint changed from "start the work and return" to await refMovedLater(...), which returns when the row exists. That is the moment the work survives a restart. The work itself still runs after the hook's answer, so a push does not wait for CI to be queued.

A worker claims the oldest open row in a lane:

update ref_moves
set state = 'running', lease_until = now() + interval '5 minutes', attempts = attempts + 1
where id = (
	select id from ref_moves
	where lane = $1 and state in ('pending', 'running')
	order by created_at, id
	limit 1
	for update skip locked
)
and (
	(state = 'pending' and next_attempt_at <= now())
	or (state = 'running' and lease_until < now())
)
returning id, move, operation_id, steps, attempts

The inner select always takes the lane's oldest open row. The outer condition then says whether that row may run now. If the oldest row waits for a retry, the claim returns nothing, and the newer moves on that ref wait behind it. One ref's moves still run in order. A failure on main does not hold back kobeni/mask-env, because that is a different lane.

Each item in the list is now a named step. When a step finishes, makima appends its name to the row's steps array and extends the lease. A resumed move skips the steps that it already finished:

async step(name, fn) {
	if (finished.has(name)) return;
	await fn();
	finished.add(name);
	await db
		.update(refMoves)
		.set({
			steps: sql`array_append(${refMoves.steps}, ${name})`,
			leaseUntil: LEASE,
		})
		.where(where);
},

A failed move goes back to pending with a backoff of 20, 40, 80 and then 160 seconds. After five attempts it is failed, with its last error on the row. Done rows are deleted after a week, failed rows after 30 days. A sweep every 30 seconds starts any lane with a row that is due.

At least once, so two steps protect themselves

A checkpoint comes after its step, so a crash between the two runs the step again. Makima accepts a repeat for most steps. Two steps must not repeat. The operation that groups a push's effects is kept on the row the first time, and a second attempt reuses its id. The CI step looks for a run that its operation already queued on that ref before it queues one. A test makes exactly this crash. It queues CI, removes ci from the finished steps and puts the row back to pending. Then it runs the sweep and requires one pipeline run, not two.

Makima runs as one instance today, and the worker depends on that. At boot, it sets every running row back to pending at once, because with one instance those rows belong to a process that is gone. The scaling runbook now says that a second instance must remove that boot step and wait for the leases instead.

The same commit fixed a smaller copy of the problem. The symbol index for pull request versions also runs from a queue in memory. At boot, makima now queues every version from the last 14 days that has no index row, up to 500.

What this does not cover

The durability starts at makima. The hook still ends its curl with || true, because a push that already landed must not report failure. If makima is down when the hook runs, the hook gives up after 10 seconds and the move is never written. A push during a makima outage still loses its follow-up work. What changed is the much more common case: makima got the move and then restarted. If the insert itself fails, makima logs it and runs the work without a row, which is the old behavior.

We did not add a broker for this, for the same reason as before. The row is the queue, and at our size Postgres is a good queue.

What clients see, and what the disk keeps

Rezee keeps private refs in each repository. refs/rezee/pulls/ pins the commits of every pull request diff and version, so git keeps the commits an old diff needs, even after a force push. refs/rezee/merges/ holds merge receipts, which let makima recover a merge that was interrupted. refs/rezee/stacks/ holds a restack in progress. Git advertises every ref to every client unless the server hides it. A default fetch did not download these refs, but git ls-remote listed them all, and every advertisement carried them.

Now every git process that denji serves, over HTTP and over SSH, runs with this config:

var hiddenRefs = []string{"refs/rezee/pulls", "refs/rezee/merges", "refs/rezee/stacks"}

func GitConfig() []string {
	args := []string{"-c", "uploadpack.allowReachableSHA1InWant=true"}
	for _, ref := range hiddenRefs {
		args = append(args, "-c", "transfer.hideRefs="+ref)
	}
	return args
}

-c on the command line, not in each repository's config file, covers every repository that already exists. refs/rezee/conflicts/ stays visible on purpose. aki fetches a conflict case's inputs, and pushes its resolution, by those names. A test runs ls-remote in v0 and in v2 and requires main and the conflict pins, with no pull pins and no merge receipts.

Protocol v2, for real this time

Our March post said denji served v0 and v2 "transparently", because git did the negotiation. That was wrong. A client asks for v2 with a Git-Protocol HTTP header, or with the GIT_PROTOCOL variable over SSH. The server's git only sees the request if the server passes it on, and denji did not. Every client got v0.

Now denji passes the value to git as GIT_PROTOCOL. The value goes into a process environment, so denji first checks it: at most 128 characters, from lowercase letters, digits and =:.- only. A value like version=2\nEVIL=1 is refused. Over SSH, denji accepts an env request only for GIT_PROTOCOL, checks its value the same way, and refuses every other variable. For v2, the HTTP discovery response also drops the # service= banner, because a v2 client reads the capability list first. receive-pack has no v2, so a push keeps the banner and answers in v0.

The SSH post measured why this matters. Over stateless HTTP in v0, every round of a fetch sends the full ref advertisement again, about 73 KB for a repository with 1,000 branches. In v2 the client asks for the refs that it wants with ls-refs. A client that does not ask for v2 still gets v0, exactly as before.

Daily maintenance

Denji now runs housekeeping on every repository: the first sweep 10 minutes after boot, then every 24 hours (DENJI_MAINTENANCE_INTERVAL). For each repository it does two things:

It deletes merge receipts older than 30 days, in one ref transaction. A receipt matters only while makima recovers an interrupted merge, which takes minutes. A month is a wide margin.
It runs git gc --auto --quiet with a 30-minute deadline. Git repacks and packs refs only when its own thresholds say so, and does nothing otherwise. This is also what removes the trees that the merge check writes.

A background job that fails without a sound is worse than no job, because people believe it runs. So the sweep reports three metrics: repositories visited by result, receipts pruned, and denji_maintenance_last_sweep_timestamp_seconds. The help text for that gauge is "Silence here is the alert."

Pull request pins get a slower rule, and makima owns it because makima knows the pull request's state. Thirty days after a pull request merges or closes, makima tells denji to drop its pins, except the two commits of its final diff. It works in batches of 50 and marks each pull request with pins_pruned_at. One failure case got special care. A denji that does not have the prune route yet answers 404, and so does a repository that no longer exists. Only the second one is "done". Makima checks for the message "repository not found". Any other 404 is retried the next day, so the order of a deploy cannot mark pull requests as pruned when nothing was pruned.

What we chose not to do

The deadline is per command, not per request. A request that makes three git calls can take three deadlines. A deadline per request needs a context passed through every function in the data layer. That is a larger change, and the per-command limit already removes "forever".
The merge check writes objects. A dry run that writes nothing would need a second merge implementation. The point of this design is that the check and the merge run one command.
Strict judges, few languages. Go, TypeScript, JavaScript and JSON are judged. Every other file is a change. We will add a language when the reviews it costs are worth a parser.
One makima. The ref move worker's boot step depends on one instance. The runbook records the change that a second instance needs.

None of this is new computer science. It is the work that changes "it works" into "it fails in bounded time, and says why". The git binary still does the hard part. Denji now has a deadline for each call it makes.

FAQ

Why is the merge check cached by commit pair and not by branch?

Because a branch is a pointer that moves, and a pair of commits is not. A cache by branch needs invalidation on every push, and invalidation is where caches go wrong. A cache by pair never goes stale. The only question is how long to keep a row, and losing a row costs a recompute.

Can an agent resolve a pull request's conflict case?

Yes. A case is an ordinary record that a person or an agent can claim. The MCP tools conflicts_list, conflict_claim, conflict_propose and conflict_accept drive the same service as the case page and aki. Accept still checks that the resolution keeps the branch's commits and passes the branch's protection, whoever proposed it.

Does a comment-only push re-run CI?

Yes. The carry rule is about review verdicts only. A push is a new commit, and the ref move runs its CI step as usual. Required checks run against the new head.

Do old git clients still work?

Yes. Denji uses v2 only when the client asks for it. A client that sends no Git-Protocol header or GIT_PROTOCOL variable gets v0, and a push is always v0, because git has no v2 for receive-pack.