feat(hooks): report the pull-request author in lifecycle payloads - #555
Merged
Conversation
Hook payloads identified the pull request only by URL, so a notifier that wanted to address the author had to re-fetch the pull request from the host on every verdict, in a path that is meant to cost nothing. Add `author`, the author's git-host login, plus the matching `CR_AUTHOR` environment variable. The value is observed from the pull-request snapshot every run already reads, so it costs no extra host call and covers review, respond, and gate early-exit alike. The first non-empty login wins: a run reads the pull request repeatedly, and a later unauthored snapshot must not erase an identity the hooks already reported. Because the observation rides the provider progress wrapper, that wrapper no longer requires a progress logger; span creation is now nil-safe so a profile with hooks and no logger still observes provider calls. Closes #554
piekstra-dev
approved these changes
Aug 6, 2026
piekstra-dev
left a comment
Collaborator
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: fee439d75caf
Profile: reviewer - Posting as: piekstra-dev
Summary
| Reviewer | Findings |
|---|---|
| go:implementation-tests | 0 |
| policies:conventions | 0 |
| documentation:docs | 0 |
| structure:repo-health | 0 |
Reviewer Coverage
| Reviewer | Status | Inspected | Skipped | Constraints |
|---|---|---|---|---|
| go:implementation-tests | complete_broad | internal/app/hook_dispatch.go, internal/app/hook_dispatch_test.go, internal/app/provider_progress.go, internal/app/runtime.go, internal/hooks/hooks.go, internal/hooks/hooks_test.go | unavailable | git diff against the base SHA was unavailable in this environment (git commands were blocked), so review was performed by reading the full current contents of each assigned file and cross-checking against the PR intent/change-map rather than a line-level unified diff. |
| policies:conventions | complete_broad | README.md, internal/app/hook_dispatch.go, internal/hooks/hooks.go | unavailable | No ../cli-common/docs or ../.github convenience copies were present in the workbench, so cross-repo shared standards were checked against repo-local convention only, not the canonical breadcrumb docs. |
| documentation:docs | complete_broad | README.md | unavailable | unavailable |
| structure:repo-health | complete_broad | internal/app/hook_dispatch.go, internal/app/provider_progress.go, internal/app/runtime.go | unavailable | unavailable |
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 3m 13s | $2.37 | claude-sonnet-5 | cr 0.10.268
| Field | Value |
|---|---|
| Model | claude-sonnet-5 |
| Reviewers | go:implementation-tests, policies:conventions, documentation:docs, structure:repo-health |
| Engine | claude_cli · claude-sonnet-5 |
| Reviewed by | cr · piekstra-dev |
| Duration | 3m 13s wall · 6m 31s compute |
| Cost | $2.37 |
| Tokens | 140 in / 24.4k out |
Per-workstream usage
| Workstream | Model | In | Out | Cache read | Cache create | Cost | Duration |
|---|---|---|---|---|---|---|---|
| orchestrator-selection | claude-sonnet-5 | 6 | 2.1k | 37.7k | 24.0k | $0.19 | 30s |
| go:implementation-tests | claude-sonnet-5 | 50 | 9.6k | 862.5k | 57.4k | $0.75 | 2m 19s |
| policies:conventions | claude-sonnet-5 | 28 | 3.8k | 401.8k | 22.7k | $0.31 | 57s |
| documentation:docs | claude-sonnet-5 | 22 | 3.0k | 289.1k | 34.5k | $0.34 | 56s |
| structure:repo-health | claude-sonnet-5 | 28 | 5.5k | 564.9k | 67.2k | $0.66 | 1m 33s |
| orchestrator-rollup | claude-sonnet-5 | 6 | 374 | 60.8k | 16.8k | $0.12 | 13s |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #554
What
Adds
authorto the lifecycle hook payload — the pull request author's git-host login (GitHub login, GitLab username) — and the matchingCR_AUTHORenvironment variable.Why
Hook payloads identify the pull request only by URL. A notifier that wants to address the person whose PR was reviewed — @-mentioning them in a Slack verdict post, say — has to re-fetch the pull request from the host on every verdict. That is an extra API call per run in a path whose whole contract is that it costs nothing and cannot fail the review.
cr already reads
gitprovider.PR.Authoron every run, for every provider, before planning. This reports what the run already knows.How
The author is observed where the pull request is read — the provider progress wrapper, which already wraps both the read and posting providers and already feeds the hook dispatcher elsewhere (
withProgressPlanner,newPipelineTaskProgress). No new provider layer, no extra host call, and review, respond, and gate early-exit are covered by the same seam.Two consequences worth calling out for review:
withProgressProviderreturns the bare provider only when there is neither a logger nor an enabled dispatcher, and span creation went nil-safe (p.start). A dispatcher with no configured hooks is still dropped, so the wrapper is not installed for profiles that use neither.Contract
authoris present from the moment the run reads the pull request — every event afterrun.started— and is omitted when a run fails before that read. Documented in the README hooks section alongside the other common fields.Deliberately not included: display name, email, avatar, or any chat-handle mapping. One stable host-native identifier is what a consumer keys a mapping on; translating it to a Slack/Teams member ID is the consumer's job.
Verification
make lintclean;make testpasses exceptTestPiRPCReviewerExtensionLoadsInInstalledPi, which fails identically onmainin this environment (the locally installedpirejects--no-builtin-tools/--no-approve).TestReviewHooksFanOutFromExistingProgressSeamsextended to read the PR through the wrapped provider and assertrun.startedcarries no author while every later event carries it.TestHookAuthorKeepsTheFirstIdentityReadFromTheProviderpins the first-wins rule against a second, unauthored read.TestHookProviderWrapperIsSkippedWhenNoHooksAreConfiguredpins that a hookless profile keeps the unwrapped provider.