Re: [PATCH] hook: introduce the report hook for git-receive-pack(1)

On Tue, Aug 18, 2026 at 09:55:55AM +0200, Karthik Nayak wrote:

When running 'git-receive-pack(1)', there is currently no way for the
server to intercept and modify the status report before it is sent back
to the client. This is useful for servers with custom logic that need
to transform or gate the report based on the outcome of external logic
post reference updates.

Introduce a new 'report' hook which receives the pkt-line encoded
status report on stdin and whose stdout replaces the report sent to the
client. A non-zero exit status causes `receive-pack` to die and the
client to treat the push as failed.

I think it would have been useful to add context why none of the
preexisting hooks work for us:

  - The pre-receive hook runs too early, as we haven't updated
    references at that point yet and we need to have the full view of
    all resulting updates (both objects and references).

  - The update hook is too inefficient as it runs once per reference,
    and we cannot trivially determine the last update.

  - The reference-transaction hook cannot be used by us because we care
    about the phase where it was committed already. And while the hook
    fires in that phase, it does not allow the caller to modify the
    result in any capacity.

  - The post-receive and post-update hooks cannot be used as they run
    too late, at the point where we have already reported success to the
    client.

I'd drop "remote" here -- from the point of view of git-receive-pack(1)
it really is the local repository.

+the status report is sent back to the client.
+
+The hook receives the pkt-line encoded status report on standard input
+and its standard output replaces the report sent to the client. Any
+output written to standard error is forwarded to the client over the
+sideband channel and will appear as `remote:` lines on the client's
+terminal.

This assumes a bit too much about the implementation of the client, as
it may not even be git-push(1) in the first place. We could still
mention this, but we should say that this depends on the client.

To reject individual ref updates, rewrite the corresponding
+`ok` lines to `ng` lines in the output report (with an explanatory
+error string) and exit zero; standard error can accompany this to
+provide a human-readable explanation. A non-zero exit status causes
+`receive-pack` to die.

We should probably document that we expect the hook to never return
non-zero, even if it rejects reference updates, and that doing so
indicates a bug. This is mostly because git-receive-pack(1) shouldn't
ever just die on the client without giving it a proper status.

+Note that by the time this hook runs, all ref updates have already been
+applied to the repository. A non-zero exit causes the client to see the
+push as failed, but does *not* roll back any ref changes that were
+already committed server-side.

Good thing to call out.

Nit: I think it's more commont to call this `ret` rather than `code`.

I was about to say that this looks unused, but it's used by
"common-functions.sh".

[snip]

+test_expect_success "hook stderr is relayed to client via sideband" '
+	test_when_finished "rm -rf upstream" &&
+	test_when_finished "git -C workbench remote remove origin" &&
+
+	git init --bare upstream &&
+	git -C workbench remote add origin ../upstream &&
+	git -C workbench push origin $A:refs/heads/main &&
+
+	test_hook -C upstream --setup report <<-\EOF &&
+	echo "hook-stderr-message" >&2
+	exit 1

Should we maybe not exit abnormally here to see that the push succeeds?

+	EOF
+
+	test_must_fail git -C workbench push origin $B:refs/heads/main >out 2>&1 &&
+	test_grep "hook-stderr-message" out
+'

This should have the "remote: " prefix, right? If so, should we verify
that?

Patrick