feat(hooks): report the pull-request author in lifecycle payloads

README.md

@@ -607,13 +607,18 @@ hooks.
 
 Every payload contains `event`, `pr_url`, `run_id`, `profile`, `pass_number`
 (the ledger attempt), `artifact_dir`, and `dry_run`; terminal events also contain
-`outcome`. `reviewer.completed` adds `reviewer_id` and `reviewer_status`,
+`outcome`. `author` carries the pull request author's git-host login (GitHub
+login, GitLab username) from the moment the run reads the pull request, which is
+every event after `run.started`, and is omitted when a run fails before that
+read. It is observed from the snapshot the run already fetches, so a hook that
+addresses the author costs no extra host call. `reviewer.completed` adds
+`reviewer_id` and `reviewer_status`,
 `posting.action` adds `action_kind` and the canonical `action_marker` when the
 provider call carries one, and `selection.completed` adds sorted `agents` plus
 an agent-to-model `models` object. Values not yet allocated at an early event,
 such as the run ID at `run.started`, are empty. The common fields are also
-available as `CR_EVENT`, `CR_PR_URL`, `CR_RUN_ID`, `CR_OUTCOME`, `CR_PROFILE`,
-`CR_PASS_NUMBER`, `CR_ARTIFACT_DIR`, and `CR_DRY_RUN`.
+available as `CR_EVENT`, `CR_PR_URL`, `CR_RUN_ID`, `CR_AUTHOR`, `CR_OUTCOME`,
+`CR_PROFILE`, `CR_PASS_NUMBER`, `CR_ARTIFACT_DIR`, and `CR_DRY_RUN`.
 
 Missing commands, non-zero exits, and timeouts write warnings to stderr with
 combined command output capped at 8 KiB. They never change the pipeline result

internal/app/hook_dispatch.go

@@ -27,6 +27,7 @@ type hookDispatcher struct {
 
 	mu            sync.Mutex
 	dryRun        bool
+	author        string
 	run           ledger.Run
 	once          map[string]bool
 	selectionSeen bool
@@ -83,6 +84,7 @@ func (d *hookDispatcher) emit(event string, extra hooks.Payload, run ledger.Run,
 		Event:       event,
 		PRURL:       d.prURL,
 		RunID:       run.RunID,
+		Author:      d.author,
 		Profile:     d.profile,
 		PassNumber:  run.Attempt,
 		ArtifactDir: run.ArtifactPath,
@@ -134,6 +136,22 @@ func (d *hookDispatcher) observeRunID(runID string) ledger.Run {
 	return run
 }
 
+// observeAuthor records the pull request author for every later event. The
+// first non-empty login wins: a run reads the pull request repeatedly, and a
+// later read that fails or returns an unauthored snapshot must not erase an
+// identity the hooks already reported.
+func (d *hookDispatcher) observeAuthor(login string) {
+	login = strings.TrimSpace(login)
+	if d == nil || login == "" {
+		return
+	}
+	d.mu.Lock()
+	defer d.mu.Unlock()
+	if d.author == "" {
+		d.author = login
+	}
+}
+
 func (d *hookDispatcher) observeRun(run ledger.Run) {
 	if d == nil || run.RunID == "" {
 		return

internal/app/hook_dispatch_test.go

@@ -58,6 +58,14 @@ func TestReviewHooksFanOutFromExistingProgressSeams(t *testing.T) {
 		PRURL: "https://github.com/acme/repo/pull/7",
 	}, store)
 	dispatcher.begin(false)
+	ref := gitprovider.PRRef{Host: "github.com", Owner: "acme", Repo: "repo", Number: 7}
+	fake := &gitprovider.Fake{}
+	if err := fake.SetPR(ref, gitprovider.PR{Ref: ref, State: gitprovider.PRStateOpen, Author: gitprovider.Identity{Login: "piekstra", ID: "1795"}}); err != nil {
+		t.Fatalf("SetPR: %v", err)
+	}
+	if _, err := withProgressProvider(nil, dispatcher, "review", fake).GetPR(context.Background(), ref); err != nil {
+		t.Fatalf("GetPR: %v", err)
+	}
 	progress := newPipelineTaskProgress(nil, "review", dispatcher)
 	logPath := run.ArtifactPath + "/agent-logs/task.jsonl"
 
@@ -84,7 +92,6 @@ func TestReviewHooksFanOutFromExistingProgressSeams(t *testing.T) {
 		t.Fatalf("RenderAction: %v", err)
 	}
 	provider := withHookProvider(dispatcher, &gitprovider.Fake{})
-	ref := gitprovider.PRRef{Host: "github.com", Owner: "acme", Repo: "repo", Number: 7}
 	if _, err := provider.PostIssueComment(context.Background(), ref, rendered+"\n\nreview"); err != nil {
 		t.Fatalf("PostIssueComment: %v", err)
 	}
@@ -113,6 +120,52 @@ func TestReviewHooksFanOutFromExistingProgressSeams(t *testing.T) {
 	if got := byEvent["posting.action"][0]; got.ActionKind != marker.ActionKindRollupComment || got.ActionMarker != rendered {
 		t.Fatalf("posting payload = %#v", got)
 	}
+	if got := byEvent["run.started"][0]; got.Author != "" {
+		t.Fatalf("run.started carried an author before the pull request was read: %#v", got)
+	}
+	for _, event := range []string{"workspace.prepared", "dossier.ready", "selection.completed", "reviewer.completed", "plan.ready", "posting.action", "run.completed"} {
+		if got := byEvent[event][0]; got.Author != "piekstra" {
+			t.Fatalf("event %s author = %q, want piekstra", event, got.Author)
+		}
+	}
+}
+
+func TestHookAuthorKeepsTheFirstIdentityReadFromTheProvider(t *testing.T) {
+	capture := t.TempDir() + "/events.jsonl"
+	t.Setenv("GO_WANT_APP_HOOK_HELPER", "1")
+	t.Setenv("APP_HOOK_CAPTURE", capture)
+	dispatcher := newHookDispatcher(OpenRequest{
+		Profile: config.Profile{Hooks: hookEntries([]string{"run.completed"})}, ProfileName: "work", Command: "review",
+	}, hookStore{})
+	ref := gitprovider.PRRef{Host: "github.com", Owner: "acme", Repo: "repo", Number: 7}
+	fake := &gitprovider.Fake{}
+	provider := withProgressProvider(nil, dispatcher, "review", fake)
+	for _, pr := range []gitprovider.PR{
+		{Ref: ref, State: gitprovider.PRStateOpen, Author: gitprovider.Identity{Login: "piekstra"}},
+		{Ref: ref, State: gitprovider.PRStateOpen},
+	} {
+		if err := fake.SetPR(ref, pr); err != nil {
+			t.Fatalf("SetPR: %v", err)
+		}
+		if _, err := provider.GetPR(context.Background(), ref); err != nil {
+			t.Fatalf("GetPR: %v", err)
+		}
+	}
+	dispatcher.emit("run.completed", hooks.Payload{Outcome: "approved"}, ledger.Run{RunID: "run-1"}, true)
+	dispatcher.drain()
+
+	payloads := readAppHookPayloads(t, capture)
+	if len(payloads) != 1 || payloads[0].Author != "piekstra" {
+		t.Fatalf("payloads = %#v, want one carrying author piekstra", payloads)
+	}
+}
+
+func TestHookProviderWrapperIsSkippedWhenNoHooksAreConfigured(t *testing.T) {
+	dispatcher := newHookDispatcher(OpenRequest{ProfileName: "work", Command: "review"}, hookStore{})
+	fake := &gitprovider.Fake{}
+	if got := withProgressProvider(nil, dispatcher, "review", fake); got != gitprovider.GitProvider(fake) {
+		t.Fatalf("provider = %#v, want the unwrapped provider", got)
+	}
 }
 
 func TestRespondHookNamespaceDoesNotFireReviewEvents(t *testing.T) {

internal/app/provider_progress.go

@@ -13,25 +13,29 @@ import (
 type progressProvider struct {
 	provider gitprovider.GitProvider
 	logger   *progress.Logger
+	hooks    *hookDispatcher
 	command  string
 }
 
 type progressRangeProvider struct {
 	progressProvider
 }
 
-func withProgressProvider(logger *progress.Logger, command string, provider gitprovider.GitProvider) gitprovider.GitProvider {
+func withProgressProvider(logger *progress.Logger, dispatcher *hookDispatcher, command string, provider gitprovider.GitProvider) gitprovider.GitProvider {
 	if provider == nil {
 		return nil
 	}
-	if logger == nil {
+	if dispatcher != nil && !dispatcher.enabled {
+		dispatcher = nil
+	}
+	if logger == nil && dispatcher == nil {
 		return provider
 	}
 	command = strings.TrimSpace(command)
 	if command == "" {
 		command = "review"
 	}
-	wrapped := progressProvider{provider: provider, logger: logger, command: command}
+	wrapped := progressProvider{provider: provider, logger: logger, hooks: dispatcher, command: command}
 	if _, ok := provider.(interface {
 		GetDiffBetweenRefs(context.Context, gitprovider.PRRef, string, string) (gitprovider.UnifiedDiff, error)
 	}); ok {
@@ -40,26 +44,38 @@ func withProgressProvider(logger *progress.Logger, command string, provider gitp
 	return wrapped
 }
 
+// start opens a progress span, or none when the wrapper exists only to observe
+// provider calls for hooks.
+func (p progressProvider) start(op, target string) *progress.Span {
+	if p.logger == nil {
+		return nil
+	}
+	return p.logger.Start(p.command, op, target)
+}
+
 func (p progressProvider) WhoAmI(ctx context.Context, creds gitprovider.Credential) (gitprovider.Identity, error) {
-	span := p.logger.Start(p.command, "resolve_identity", "runtime")
+	span := p.start("resolve_identity", "runtime")
 	identity, err := p.provider.WhoAmI(ctx, creds)
 	return identity, endProgressSpan(span, err)
 }
 
 func (p progressProvider) ReviewAuthority(ctx context.Context, ref gitprovider.PRRef, identity gitprovider.Identity) (gitprovider.ReviewAuthority, error) {
-	span := p.logger.Start(p.command, "check_review_authority", "runtime")
+	span := p.start("check_review_authority", "runtime")
 	authority, err := p.provider.ReviewAuthority(ctx, ref, identity)
 	return authority, endProgressSpan(span, err)
 }
 
 func (p progressProvider) GetPR(ctx context.Context, ref gitprovider.PRRef) (gitprovider.PR, error) {
-	span := p.logger.Start(p.command, "fetch_pr", "pr")
+	span := p.start("fetch_pr", "pr")
 	pr, err := p.provider.GetPR(ctx, ref)
+	if err == nil && p.hooks != nil {
+		p.hooks.observeAuthor(pr.Author.Login)
+	}
 	return pr, endProgressSpan(span, err)
 }
 
 func (p progressProvider) GetDiff(ctx context.Context, ref gitprovider.PRRef) (gitprovider.UnifiedDiff, error) {
-	span := p.logger.Start(p.command, "fetch_diff", "pr")
+	span := p.start("fetch_diff", "pr")
 	diff, err := p.provider.GetDiff(ctx, ref)
 	return diff, endProgressSpan(span, err)
 }
@@ -68,66 +84,66 @@ func (p progressRangeProvider) GetDiffBetweenRefs(ctx context.Context, ref gitpr
 	rangeProvider := p.provider.(interface {
 		GetDiffBetweenRefs(context.Context, gitprovider.PRRef, string, string) (gitprovider.UnifiedDiff, error)
 	})
-	span := p.logger.Start(p.command, "fetch_diff_between_refs", "pr")
+	span := p.start("fetch_diff_between_refs", "pr")
 	diff, err := rangeProvider.GetDiffBetweenRefs(ctx, ref, baseSHA, headSHA)
 	return diff, endProgressSpan(span, err)
 }
 
 func (p progressProvider) GetFileAtRef(ctx context.Context, ref gitprovider.PRRef, gitRef, path string) ([]byte, error) {
-	span := p.logger.Start(p.command, "read_file", fileTarget(path))
+	span := p.start("read_file", fileTarget(path))
 	data, err := p.provider.GetFileAtRef(ctx, ref, gitRef, path)
 	return data, endProgressSpan(span, err)
 }
 
 func (p progressProvider) ListTreeAtRef(ctx context.Context, ref gitprovider.PRRef, gitRef, path string) ([]gitprovider.TreeEntry, error) {
-	span := p.logger.Start(p.command, "list_tree", fileTarget(path))
+	span := p.start("list_tree", fileTarget(path))
 	entries, err := p.provider.ListTreeAtRef(ctx, ref, gitRef, path)
 	return entries, endProgressSpan(span, err)
 }
 
 func (p progressProvider) ListInlineThreads(ctx context.Context, ref gitprovider.PRRef) ([]gitprovider.InlineThread, error) {
-	span := p.logger.Start(p.command, "list_threads", "threads")
+	span := p.start("list_threads", "threads")
 	threads, err := p.provider.ListInlineThreads(ctx, ref)
 	return threads, endProgressSpan(span, err)
 }
 
 func (p progressProvider) ListReviews(ctx context.Context, ref gitprovider.PRRef) ([]gitprovider.Review, error) {
-	span := p.logger.Start(p.command, "list_reviews", "reviews")
+	span := p.start("list_reviews", "reviews")
 	reviews, err := p.provider.ListReviews(ctx, ref)
 	return reviews, endProgressSpan(span, err)
 }
 
 func (p progressProvider) ListIssueComments(ctx context.Context, ref gitprovider.PRRef) ([]gitprovider.IssueComment, error) {
-	span := p.logger.Start(p.command, "list_issue_comments", "posts")
+	span := p.start("list_issue_comments", "posts")
 	comments, err := p.provider.ListIssueComments(ctx, ref)
 	return comments, endProgressSpan(span, err)
 }
 
 func (p progressProvider) PostInlineComment(ctx context.Context, ref gitprovider.PRRef, c gitprovider.InlineComment) (gitprovider.CommentID, error) {
-	span := p.logger.Start(p.command, "post_inline_comment", "posts")
+	span := p.start("post_inline_comment", "posts")
 	id, err := p.provider.PostInlineComment(ctx, ref, c)
 	return id, endProgressSpan(span, err)
 }
 
 func (p progressProvider) ReplyToThread(ctx context.Context, ref gitprovider.PRRef, threadID gitprovider.ThreadID, body string) (gitprovider.CommentID, error) {
-	span := p.logger.Start(p.command, "reply_thread", "posts")
+	span := p.start("reply_thread", "posts")
 	id, err := p.provider.ReplyToThread(ctx, ref, threadID, body)
 	return id, endProgressSpan(span, err)
 }
 
 func (p progressProvider) ResolveThread(ctx context.Context, ref gitprovider.PRRef, threadID gitprovider.ThreadID) error {
-	span := p.logger.Start(p.command, "resolve_thread", "posts")
+	span := p.start("resolve_thread", "posts")
 	return endProgressSpan(span, p.provider.ResolveThread(ctx, ref, threadID))
 }
 
 func (p progressProvider) PostIssueComment(ctx context.Context, ref gitprovider.PRRef, body string) (gitprovider.CommentID, error) {
-	span := p.logger.Start(p.command, "post_issue_comment", "posts")
+	span := p.start("post_issue_comment", "posts")
 	id, err := p.provider.PostIssueComment(ctx, ref, body)
 	return id, endProgressSpan(span, err)
 }
 
 func (p progressProvider) SubmitReview(ctx context.Context, ref gitprovider.PRRef, r gitprovider.ReviewRequest) (gitprovider.ReviewID, error) {
-	span := p.logger.Start(p.command, "submit_review", "posts")
+	span := p.start("submit_review", "posts")
 	id, err := p.provider.SubmitReview(ctx, ref, r)
 	return id, endProgressSpan(span, err)
 }

internal/app/runtime.go

@@ -173,7 +173,7 @@ func Open(ctx context.Context, req OpenRequest) (Runtime, error) {
 		cleanup()
 		return Runtime{}, err
 	}
-	repoProvider = withProgressProvider(req.Progress, command, repoProvider)
+	repoProvider = withProgressProvider(req.Progress, dispatcher, command, repoProvider)
 	postingGit := gitConfigForReviewerAuth(profile)
 	_, postingProviderStore, err := stores.Open(postingGit.Credential)
 	if err != nil {
@@ -186,7 +186,7 @@ func Open(ctx context.Context, req OpenRequest) (Runtime, error) {
 		return Runtime{}, err
 	}
 	rawPostingProvider := postingProvider
-	postingProvider = withProgressProvider(req.Progress, command, postingProvider)
+	postingProvider = withProgressProvider(req.Progress, dispatcher, command, postingProvider)
 	postingProvider = withHookProvider(dispatcher, postingProvider)
 	postingIdentity, err := deps.ResolvePostingIdentity(ctx, postingProvider, credential, postingProviderStore, profile)
 	if err != nil {

internal/hooks/hooks.go

@@ -23,9 +23,12 @@ const maxOutput = 8 << 10
 // set only for terminal events. The remaining optional fields are specific to
 // selection, reviewer, and posting events.
 type Payload struct {
-	Event          string            `json:"event"`
-	PRURL          string            `json:"pr_url"`
-	RunID          string            `json:"run_id"`
+	Event string `json:"event"`
+	PRURL string `json:"pr_url"`
+	RunID string `json:"run_id"`
+	// Author is the pull request author's git-host login, known once the run
+	// has read the pull request.
+	Author         string            `json:"author,omitempty"`
 	Outcome        string            `json:"outcome,omitempty"`
 	Profile        string            `json:"profile"`
 	PassNumber     int               `json:"pass_number"`
@@ -130,6 +133,7 @@ func payloadEnv(payload Payload) []string {
 		"CR_EVENT=" + payload.Event,
 		"CR_PR_URL=" + payload.PRURL,
 		"CR_RUN_ID=" + payload.RunID,
+		"CR_AUTHOR=" + payload.Author,
 		"CR_OUTCOME=" + payload.Outcome,
 		"CR_PROFILE=" + payload.Profile,
 		"CR_PASS_NUMBER=" + strconv.Itoa(payload.PassNumber),

internal/hooks/hooks_test.go

@@ -41,8 +41,8 @@ func TestHookHelperProcess(_ *testing.T) {
 		// #nosec G304,G703 -- the parent test supplies a t.TempDir capture path.
 		_ = os.WriteFile(path, []byte(strings.Join([]string{
 			os.Getenv("CR_EVENT"), os.Getenv("CR_PR_URL"), os.Getenv("CR_RUN_ID"),
-			os.Getenv("CR_OUTCOME"), os.Getenv("CR_PROFILE"), os.Getenv("CR_PASS_NUMBER"),
-			os.Getenv("CR_ARTIFACT_DIR"), os.Getenv("CR_DRY_RUN"),
+			os.Getenv("CR_AUTHOR"), os.Getenv("CR_OUTCOME"), os.Getenv("CR_PROFILE"),
+			os.Getenv("CR_PASS_NUMBER"), os.Getenv("CR_ARTIFACT_DIR"), os.Getenv("CR_DRY_RUN"),
 		}, "\n")), 0o600)
 	}
 	if code, _ := strconv.Atoi(os.Getenv("HOOK_HELPER_EXIT")); code != 0 {
@@ -60,7 +60,8 @@ func TestDispatcherWritesPayloadAndCommonEnvironment(t *testing.T) {
 	dispatcher := New([]config.Hook{{Event: "run.completed", Argv: helperArgv(), Timeout: "1s"}}, io.Discard)
 	want := Payload{
 		Event: "run.completed", PRURL: "https://github.com/acme/repo/pull/7", RunID: "run-7",
-		Outcome: "approved", Profile: "work", PassNumber: 2, ArtifactDir: "/tmp/run-7", DryRun: false,
+		Author: "piekstra", Outcome: "approved", Profile: "work", PassNumber: 2,
+		ArtifactDir: "/tmp/run-7", DryRun: false,
 	}
 	dispatcher.Dispatch(want)
 	dispatcher.Drain()
@@ -74,7 +75,7 @@ func TestDispatcherWritesPayloadAndCommonEnvironment(t *testing.T) {
 	if err != nil {
 		t.Fatalf("read env: %v", err)
 	}
-	wantEnv := "run.completed\nhttps://github.com/acme/repo/pull/7\nrun-7\napproved\nwork\n2\n/tmp/run-7\nfalse"
+	wantEnv := "run.completed\nhttps://github.com/acme/repo/pull/7\nrun-7\npiekstra\napproved\nwork\n2\n/tmp/run-7\nfalse"
 	if string(envBody) != wantEnv {
 		t.Fatalf("env = %q, want %q", envBody, wantEnv)
 	}