Policy bot stuck on `Commit hash does not have a pushed date`

[joseparajelesGL]

Hello, we are seeing errors on policy bot where it's stuck on trying to get the pushed date

Is there anything we can do to debug this issue?

Attached is the stack trace

failed to compute approval status: failed to list commits: Commit f3bed522f4 does not have a pushed date. This is usually caused by delays in the GitHub API
github.com/palantir/policy-bot/policy/approval.(*Rule).filteredCommits
	/home/runner/work/policy-bot/policy-bot/policy/approval/approve.go:345
github.com/palantir/policy-bot/policy/approval.(*Rule).IsApproved
	/home/runner/work/policy-bot/policy-bot/policy/approval/approve.go:204
github.com/palantir/policy-bot/policy/approval.(*Rule).Evaluate
	/home/runner/work/policy-bot/policy-bot/policy/approval/approve.go:141
github.com/palantir/policy-bot/policy/approval.(*RuleRequirement).Evaluate
	/home/runner/work/policy-bot/policy-bot/policy/approval/evaluate.go:63
github.com/palantir/policy-bot/policy/approval.(*OrRequirement).Evaluate
	/home/runner/work/policy-bot/policy-bot/policy/approval/evaluate.go:86
github.com/palantir/policy-bot/policy/approval.(*AndRequirement).Evaluate
	/home/runner/work/policy-bot/policy-bot/policy/approval/evaluate.go:146
github.com/palantir/policy-bot/policy/approval.(*evaluator).Evaluate
	/home/runner/work/policy-bot/policy-bot/policy/approval/evaluate.go:39
github.com/palantir/policy-bot/policy.evaluator.Evaluate
	/home/runner/work/policy-bot/policy-bot/policy/policy.go:80
github.com/palantir/policy-bot/server/handler.(*EvalContext).EvaluatePolicy
	/home/runner/work/policy-bot/policy-bot/server/handler/eval_context.go:125
github.com/palantir/policy-bot/server/handler.(*Details).ServeHTTP
	/home/runner/work/policy-bot/policy-bot/server/handler/details.go:132
github.com/bluekeyes/hatpear.Try.func1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/bluekeyes/hatpear/hatpear.go:84
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
goji%2eio.dispatch.ServeHTTP
	/home/runner/work/policy-bot/policy-bot/vendor/goji.io/dispatch.go:17
github.com/palantir/policy-bot/server/handler.RequireLogin.func1.1
	/home/runner/work/policy-bot/policy-bot/server/handler/login.go:95
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
goji%2eio.(*Mux).ServeHTTP
	/home/runner/work/policy-bot/policy-bot/vendor/goji.io/mux.go:74
goji%2eio.dispatch.ServeHTTP
	/home/runner/work/policy-bot/policy-bot/vendor/goji.io/dispatch.go:17
goji%2eio.(*Mux).ServeHTTP
	/home/runner/work/policy-bot/policy-bot/vendor/goji.io/mux.go:74
goji%2eio.dispatch.ServeHTTP
	/home/runner/work/policy-bot/policy-bot/vendor/goji.io/dispatch.go:17
github.com/bluekeyes/hatpear.Recover.func1.1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/bluekeyes/hatpear/hatpear.go:113
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
github.com/bluekeyes/hatpear.Catch.func1.1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/bluekeyes/hatpear/hatpear.go:60
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
github.com/palantir/go-baseapp/baseapp.AccessHandler.func1.1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/palantir/go-baseapp/baseapp/middleware.go:88
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
github.com/rs/zerolog/hlog.RequestIDHandler.func1.1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/rs/zerolog/hlog/hlog.go:187
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
github.com/palantir/go-baseapp/baseapp.NewMetricsHandler.func1.1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/palantir/go-baseapp/baseapp/middleware.go:55
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
github.com/rs/zerolog/hlog.NewHandler.func1.1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/rs/zerolog/hlog/hlog.go:29
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
goji%2eio.(*Mux).ServeHTTP
	/home/runner/work/policy-bot/policy-bot/vendor/goji.io/mux.go:74

[ab77]

According to this, a breaking change was made to Commit.pushedDate and pushedDate appears to be no longer supported.

[esmelser-apex]

We are also experiencing this issue and would love to see a fix. ➕ 1️⃣

[johnwonkim]

+1 as well.

[z-x63]

We are hitting the issue as well. Don't have context into the repo but thanks to @ab77 did some digging in the code and found PushedDate being used:

policy-bot/pull/github.go

Line 925 in 51b4afb

PushedDate *time.Time

Looking at the graphql docs on Commit that field did get deprecated: https://docs.github.com/en/graphql/reference/objects#commit

I went through all the fields looking for possible replacements and found these fields:
authoredDate
committedDate

Could consider swapping out the pushDate for the above two fields but one possible issue I see:

• if the pushDate was used to determine when to reset approvals, using the other two fields allows someone to author/commit early but hold the change locally
• The user would be able to submit v1 of the changes (but already have v2 locally). Get the approval for v1 then push out v2. Since the v2 author/commit time was before the approval, it wouldn't reset the approvals

[asvoboda]

Thanks for the report. Its definitely a shame that the field has been removed from the graphql api:

Breaking A change will be made to Commit.pushedDate.
Description:
pushedDate will be removed.

Reason:
pushedDate is no longer supported.

You are correct @zliu2-wish that the PushedDate is used to determine approvals with the invalidate_on_push option, so this breaking change is particularly problematic.

[asvoboda]

I'm going to file a ticket with our enterprise reps to better understand what options we have on the GH side (#2241100).

I believe this should only affect policies where invalidate_on_push is used. I think the only short term option at the moment is to write a policy that has a fallback OR condition where a maintainer/admin has approved. I'll continue thinking about this.

[asvoboda]

I think we'll want to create an option to disable the push date loading for commits that can be toggled at the server level. Right now we attempt to load the pushed dates for commits even for rules that don't use the invalidate_on_push option, which is contributing to slowness in requests.

[asvoboda]

From support:

The Commit.pushedDate field was deprecated because the information calculated and presented is incorrect. The noticed was first announced in our changelog here and then deprecation happen 2023-07-01 as you already noted.

As I understand, there is currently no way to map a commit to a push, and establish that it's the first time the commit has been seen. There isn't a similar date object elsewhere in the API that can replace this.

We hope one day to offer some mapping from commit to push as it is on our radar but this isn't something that will be prioritized in the short/medium term.

[DepickereSven]

From support:

The Commit.pushedDate field was deprecated because the information calculated and presented is incorrect. The noticed was first announced in our changelog here and then deprecation happen 2023-07-01 as you already noted.

As I understand, there is currently no way to map a commit to a push, and establish that it's the first time the commit has been seen. There isn't a similar date object elsewhere in the API that can replace this.

We hope one day to offer some mapping from commit to push as it is on our radar but this isn't something that will be prioritized in the short/medium term.

So this would mean that for now there is no solution in the near future to be able to use invalidate_on_push?

[asvoboda]

Thats correct; it looks like on github.com support for invalidate on push has been entirely broken.

[DepickereSven]

Thats correct; it looks like on github.com support for invalidate on push has been entirely broken.

That's sad to hear because that was powerful in combination with the property ignore_commits_by.
So now when we switch back to letting GitHub decide to invalidate on push that property will no longer work either.

[asvoboda]

More information from support:

We discovered that the Commit GraphQL object returns a PushedDate field. This field is calculated in a problematic way. We do not have a way to tie a commit back to a specific push, so we cannot reliably calculate the first time we see a commit and which push it belongs to.

Instead, this field attempts to do it by finding the first Push in the pushes table across all repositories that has an after SHA that matches the commit in question. This doesn't work if the commit is somewhere in the middle of a group of commits in a push. It's possible that the commit's SHA isn't ever a push's after SHA. It's also possible that we do find a push with the matching after SHA, but it's not the first time we saw the commit.

All this to say, the field is very unreliable, and not accurate.

This probably explains why policy-bot needs a significant amount of code to handle pushedDate unreliability in https://github.com/palantir/policy-bot/blob/develop/pull/github.go#L856-L875

[samandmoore]

Wow. Well this is quite unfortunate. It's especially weird because GitHub seems to support similar behavior with their "dismiss stale reviews" branch protection rule setting. It clearly dismisses reviews older than the latest pushed changes on a PR.