[PATCH v6 0/4] hook: introduce the receive-report hook

[Karthik Nayak <karthik.188@gmail.com>]

Introduce a new receive-report hook which kicks in after the reference
transaction is complete, but before the report is sent to the client.
The hook receives the pkt-line encoded report in its stdin and its
stdout replaces the report transferred to the user. If the hook exits
with a non-zero exit code, all references are marked as rejected.

The first patch, adds missing documentation to 'git-receive-pack.adoc'.
The second patch refactors code and the third patch contains the new
hook.

---
Changes in v6:
- Introduce a new commit which introduces `enum report_status_version`,
  use that and drop static variables in the codebase.
- Reword the commit message and documentation to:
  - State further why reference-transaction cannot be used.
  - State the responsibility of the hook owner to undo and reference
    changes if needed.
- Link to v5: https://patch.msgid.link/20260901-758-introduce-hook-v5-0-35cdc6be3cc1@gmail.com

Changes in v5:
- Rewrote some of the commit messages and documentation.
- Renamed the function `generate_response` to `generate_report` to avoid
  ambiguity.
- We now override the cmd's error_strings, this avoids the whole
  precedence issue with the earlier series.
- Also add information about how we can override the unpack status to
  fail the push and add a corresponding test.
- Thanks to Patrick for the review!
- Junio: This causes conflict with next ('jt/receive-pack-pluggable-writes')
  similar to before, please let me know if its better for me to add that
  dependency.
- Link to v4: https://patch.msgid.link/20260826-758-introduce-hook-v4-0-6b14975ad957@gmail.com

Changes in v4:
- Change the name of the hook to be 'receive-report' to avoid ambiguity.
- Link to v3: https://patch.msgid.link/20260824-758-introduce-hook-v3-0-499526f0a062@gmail.com

Changes in v3:
- Move out addition of proc-receive hook doc to 'git-receive-pack.adoc'
  into a new commit.
- Add a new commit to move out the response generation in receive-pack
  to a new function.
- Instead of die-ing on non-zero exit code, we modify each reference to
  indicate that the hook failed.
- Instead of correctly listing out the protocol, link to
  linkgit:gitprotocol-pack[5], as the protocol also differs between v1
  and v2.
- Link to v2: https://patch.msgid.link/20260821-758-introduce-hook-v2-1-e90e2f7ac2cf@gmail.com

Changes in v2:
- Modify the documentation and commit message to be more verbose.
- Add documentation to 'git-receive-pack.adoc'
- Use 'ret' as the variable name for the return code.
- Modify the test to also check for the 'remote:'.
- Link to v1: https://patch.msgid.link/20260818-758-introduce-hook-v1-1-8a8d89e65838@gmail.com

 To: git@vger.kernel.org
 CC: ps@pks.im
 CC: gitster@pobox.com
 CC: jltobler@gmail.com
 CC: kristofferhaugsbakk@fastmail.com
 CC: phillip.wood123@gmail.com

---
Karthik Nayak (4):
      doc: add proc-receive hook info in 'git-receive-pack.adoc'
      receive-pack: drop static variables to track report status version
      receive-pack: move message generation to separate function
      hook: introduce the receive-report hook

 Documentation/git-receive-pack.adoc |  17 +++
 Documentation/githooks.adoc         |  61 ++++++++++
 builtin/receive-pack.c              | 157 +++++++++++++++++--------
 t/meson.build                       |   1 +
 t/t5412-receive-report-hook.sh      | 224 ++++++++++++++++++++++++++++++++++++
 5 files changed, 415 insertions(+), 45 deletions(-)

Range-diff versus v5:

1:  5b55286c7b = 1:  ac4272c0ed doc: add proc-receive hook info in 'git-receive-pack.adoc'
-:  ---------- > 2:  e635158b10 receive-pack: drop static variables to track report status version
2:  c4e0e8185a ! 3:  0d588b7e24 receive-pack: move message generation to separate function
    @@ builtin/receive-pack.c: static void update_shallow_info(struct command *commands
     + * report per reference update.
     + */
     +static void generate_report(struct strbuf *buf, struct command *commands,
    -+			    const char *unpack_status, bool detailed_report)
    ++			    const char *unpack_status,
    ++			    enum report_status_version version)
      {
      	struct command *cmd;
     -	struct strbuf buf = STRBUF_INIT;
    @@ builtin/receive-pack.c: static void update_shallow_info(struct command *commands
     +		else
     +			packet_buf_write(buf, "ok %s\n", cmd->ref_name);
     +
    -+		if (!detailed_report || cmd->error_string)
    ++		if (version != REPORT_STATUS_V2 || cmd->error_string)
      			continue;
     -		}
     -		packet_buf_write(&buf, "ok %s\n",
    @@ builtin/receive-pack.c: static void update_shallow_info(struct command *commands
     +{
     +	struct strbuf buf = STRBUF_INIT;
     +
    -+	generate_report(&buf, commands, unpack_status, false);
    ++	generate_report(&buf, commands, unpack_status, REPORT_STATUS_V0);
     +
     +	if (use_sideband)
     +		send_sideband(1, 1, buf.buf, buf.len, use_sideband);
    @@ builtin/receive-pack.c: static void update_shallow_info(struct command *commands
     +{
     +	struct strbuf buf = STRBUF_INIT;
     +
    -+	generate_report(&buf, commands, unpack_status, true);
    ++	generate_report(&buf, commands, unpack_status, REPORT_STATUS_V2);
      
      	if (use_sideband)
      		send_sideband(1, 1, buf.buf, buf.len, use_sideband);
3:  750a58166e ! 4:  ee4346b991 hook: introduce the receive-report hook
    @@ Commit message
         operations post reference transaction succeed. So reporting the correct
         message based on the outcome of these operations is important.
     
    +    The outcome of these operations is only known after `execute_commands()`
    +    has returned and before the report is written. There is no point in
    +    receive-pack where the server can act on that.
    +
         We cannot use any of the existing hooks as:
     
           - The pre-receive hook runs too early, as we haven't updated
    @@ Commit message
           - 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 reference-transaction hook is not suited for this. It fires from
    +        within `ref_transaction_commit()`, which is before the outcome we
    +        need to report is known, so there is no phase at which it could give
    +        us the answer. It also does not contain any knowledge regarding the
    +        push and cannot communicate with the clients.
    +
    +      - The proc-receive hook replaces execute_commands() for references
    +        matching 'receive.procReceiveRefs'. We need to gate the report for
    +        the push as a whole.
     
           - 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
    @@ Commit message
         'remote:' lines on the client terminal. Writing to stderr alone does
         not affect the push outcome.
     
    -    Note that in either failure mode, ref updates already applied by
    -    execute_commands() are not rolled back. The hook can cause the client
    -    to perceive the push as failed, but cannot undo server-side changes.
    +    Reference updates applied by execute_commands() are not rolled back in
    +    either failure mode. The hook can cause the client to perceive the push
    +    as failed, but cannot undo server-side changes. This creates a
    +    divergence that the server cannot resolve: the client leaves its
    +    remote-tracking reference at the old value while the update is in fact
    +    applied, and a later fetch may reveal the update that the push reported
    +    as rejected.
    +
    +    The hook is therefore only appropriate for servers which can guarantee
    +    that a rejected update is not observable by any reader. In our case the
    +    transaction committed by execute_commands() produces a candidate version
    +    which is not visible to other readers and is only published once the
    +    subsequent operations succeed, so a report of 'ng' corresponds to a
    +    version that is discarded rather than published. On a repository where a
    +    committed reference update is immediately visible, rejecting a push from
    +    this hook would instead leave the pusher with a view that does not match
    +    the server.
     
         This hook does not use the config-based hook infrastructure, which
         supports running multiple scripts per hook event. This hook is a
    @@ Documentation/githooks.adoc: The exit status of the hook is ignored for any stat
     +to `ng` rolls back any ref changes that were already committed
     +server-side. The hook can cause the client to perceive the push as
     +failed, but cannot undo the server-side updates.
    ++
    ++This means that reporting a reference as `ng` makes the client believe
    ++the update did not happen while the server has in fact applied it. The
    ++client leaves its remote-tracking reference at its old value, and a
    ++later `git fetch` may reveal the very update that the push reported as
    ++rejected. Neither Git nor the server can reconcile this; only the user,
    ++by fetching again, will find out.
    ++
    ++This hook is therefore only appropriate for servers which can guarantee
    ++that a rejected update is not observable by any reader, for example
    ++because the committed transaction produces a candidate state that is
    ++discarded rather than published. On a repository where a committed
    ++reference update is immediately visible, using this hook to reject a
    ++push will leave the pusher with a view that does not match the server.
     +
      push-to-checkout
      ~~~~~~~~~~~~~~~~
    @@ builtin/receive-pack.c: static void update_shallow_info(struct command *commands
       * For v2 protocol, set `detailed_report` to true, which will also add detailed
     @@ builtin/receive-pack.c: static void report(struct command *commands, const char *unpack_status)
      
    - 	generate_report(&buf, commands, unpack_status, false);
    + 	generate_report(&buf, commands, unpack_status, REPORT_STATUS_V0);
      
     +	if (run_receive_report_hook(&buf)) {
     +		strbuf_reset(&buf);
    @@ builtin/receive-pack.c: static void report(struct command *commands, const char
      	else
     @@ builtin/receive-pack.c: static void report_v2(struct command *commands, const char *unpack_status)
      
    - 	generate_report(&buf, commands, unpack_status, true);
    + 	generate_report(&buf, commands, unpack_status, REPORT_STATUS_V2);
      
     +	if (run_receive_report_hook(&buf)) {
     +		strbuf_reset(&buf);

---
base-commit: 11c6700f10234578d10523faf35656ca491425c9
change-id: 20260812-758-introduce-hook-5b3af9f1a7e8


Thanks
- Karthik