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

["Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com>]

On Tue, Aug 18, 2026, at 09:55, 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.
>
> Similar to the 'proc-receive' hook, this does not use the config-based
> hook infrastructure. That infrastructure is designed for parallelizable
> notification hooks. As this hook is a bidirectional filter, it would
> require significant modifications to that infrastructure and this hook
> cannot be parallelized anyway.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> To give some context, we at GitLab are building a custom MVCC around
> Git. Each git-push would initialize a new version which is then
> committed as the default post some operations. These operations take
> place after the reference transaction and based on the output status of
> those operations, we want to propagate the status to the user. There
> currently exists no good mechanism to do so.
>
> Having a report hook which allows us to modify the report being
> propagated to the user, allows us to modify the report based on the
> status of our MVCC commit phase.

Personally I think understanding concrete things is easier than
understanding general things. And discussing the concrete case in the
commit message would help with that as well as provide the context for
git-log(1) rather than just the people who have read these emails.

> ---
>  Documentation/githooks.adoc |  23 ++++++
>  builtin/receive-pack.c      |  41 +++++++++++
>  t/meson.build               |   1 +
>  t/t5412-report-hook.sh      | 176 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 241 insertions(+)

Should the git-receive-pack(1) doc be updated to mention that this hook
exists? I don’t understand the setup here. The existing
git-receive-pack(1) doc has sections for these hooks:

• `update`
• `pre-receive`
• `post-receive`
• `post-update`

But not these:

• `push-to-checkout`
• `proc-receive`

(referenced against githooks(5))

>
> diff --git a/Documentation/githooks.adoc b/Documentation/githooks.adoc
> index ed045940d1..7e6643ad89 100644
> --- a/Documentation/githooks.adoc
> +++ b/Documentation/githooks.adoc
> @@ -527,6 +527,29 @@ The exit status of the hook is ignored for any
> state except for the
>  status will cause the transaction to be aborted. The hook will not be
>  called with "aborted" state in that case.
>
> +report
> +~~~~~~
> +
> +This hook is invoked by linkgit:git-receive-pack[1] when it reacts to
> +`git push` and updates reference(s) in its repository. It executes on
> +the remote repository once after all refs have been updated, but before
> +the status report is sent back to the client.
> +
> +The hook receives the pkt-line encoded status report on standard input

Another naive question (I have never used any of this). Should this link
to some gitprotocol-X(5) after `pkt-line` in order to have a link that
explains what it is? I don’t see any mention of `pkt-line` on
git-receive-pack(1) or a mention of a gitprotocol-X(5).

> +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. 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.
> +
> +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.

To my naive eyes this description looks good and without any obvious
errors (typos ;) ).

> +
>  push-to-checkout
>  ~~~~~~~~~~~~~~~~
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
>[snip]
> @@ -2592,6 +2630,9 @@ static void report_v2(struct command *commands,
> const char *unpack_status)
>  	}
>  	packet_buf_flush(&buf);
>
> +	if (run_report_hook(&buf))
> +		die("report hook failed");

Okay, it seems typical for this command to use regular strings (not
translated) for errors. Which makes sense given the application. There
does seem to be translated error strings but one example is “refusing to
update current branch”, which seems to be more of a non-bare, end-user
error than a server error.

> +
>  	if (use_sideband)
>  		send_sideband(1, 1, buf.buf, buf.len, use_sideband);
>  	else
>[snip]
> diff --git a/t/t5412-report-hook.sh b/t/t5412-report-hook.sh
>[snip]
> +test_expect_success "no report hook, push succeeds" '
> +	test_when_finished "rm -rf upstream" &&
> +	test_when_finished "git -C workbench remote remove origin" &&

This teardown routine is common to all the tests. Is it better style
here to write it out compared to using a helper function (test code is
different from “normal” code)?

> +	git init --bare upstream &&
>[snip]