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.

> 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

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.

> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 86933d8d7e..bc22b3ec31 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -1004,6 +1004,41 @@ static int run_update_hook(struct command *cmd)
>  	return code;
>  }
>  
> +static int run_report_hook(struct strbuf *report)
> +{
> +	struct child_process proc = CHILD_PROCESS_INIT;
> +	struct async sideband_async;
> +	int sideband_async_started = 0;
> +	int saved_stderr = -1;
> +	struct strbuf out = STRBUF_INIT;
> +	const char *hook_path;
> +	int code;

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

> diff --git a/t/t5412-report-hook.sh b/t/t5412-report-hook.sh
> new file mode 100755
> index 0000000000..47f20e8d67
> --- /dev/null
> +++ b/t/t5412-report-hook.sh
> @@ -0,0 +1,176 @@
> +#!/bin/sh
> +
> +test_description='test report hook'
> +
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +
> +. ./test-lib.sh
> +
> +. "$TEST_DIRECTORY"/t5411/common-functions.sh
> +
> +URL_PREFIX="\.\."

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