Hey @bendavies, off the top of my head I'm not sure. In the meantime, what permissions do you have on your fine grained PAT, then we can try to reproduce?

@williammartin i added Read on EVERYTHING, to test that checks:read wasn't hidden away somewhere in another option.

I was able to recreate this also with gh run view:

➜  gh run view 8099062527
⣾failed to get annotations: HTTP 403: Resource not accessible by personal access token (https://api.github.com/repos/williammartin-test-org/test-repo/check-runs/22133946540/annotations)

I checked this out internally and unfortunately the platform doesn't support adding checks permissions to a Fine Grained PAT right now, so this is blocked. I've left a comment on the internal issue to here to aid in prioritisation.

I've created #8843 in the meantime to document this more clearly in the CLI.

Is it a requirement for you to use fine grained PATs for these commands? We could consider the addition of a flag to skip annotations if that is the only bit that is currently failing, though I'd prefer not to add permanent flags for temporary situations if possible.

Thanks for the investigation.

Is it a requirement for you to use fine grained PATs for these commands?

well, they are nice in that they allow scoping to an org, so i prefer to use them.

You could also gracefully skip over annotations if they 403, while still showing the run, informing the user that there were no permissions for annotations.

That's an interesting idea and I'm leaning in favour of it. My only concern is whether we want to try and be smart about the type of token in use. For example, if it were a legacy PAT would we error, would we be more informative, or would we just have a generalised error message to help differentiate the steps (or lack of) that a user could take to fix it.

Affected by this as well. Hoping for a solution 🙏

Labelling this help wanted if someone would like to do some design work around #8842 (comment). There's other interesting questions like "what should the exit code be" if there is only partial success. I'm not sure we have a pattern for this anywhere in the CLI yet.

the platform already does error handling with informative error messaging so I'm not sure we should introduce some custom error handling in the cli.

as is: failed to get annotations: HTTP 403: Resource not accessible by personal access token

• failed to get annotations: : cli hints at what part went wrong
• HTTP 403: Resource not accessible by personal access token: platform error hints at my access token, this could be more verbose

to me it's clear something is wrong with my access token so I need to debug my access token against the annotations endpoint.

the platform could add more info as to why it fails for example HTTP 403: Resource not accessible by personal access token. Annotations are not supported yet in Fine Grained Tokens (https://docs.github.com/en/rest/authentication/permissions-required-for-fine-grained-personal-access-tokens).

This way we don't need to duplicate error messaging logic, the platform is the single source of truth and the client, in this case the cli, just throws the error or handles the error however it likes.

optional error suppression

I follow @bendavies in evaluating certain flows and suppressing some errors when it's not the critical data needed for a command.

for the run watch and run view command we fetch 3 endpoints, depending on the endpoint we suppress the error we receive from the platform

• GET run: most important data 👉 throw error
• GET jobs: needed for the command to be useful 👉 throw error
• GET annotations: useful but when unavailable shouldn't block us from seeing the jobs 👉 suppress error, show warning

exit codes

strictly speaking, when we cannot access a resource that requires permission we should exit with 2. But I think in the case were we would optionally depend on annotations, I think 0 is more appropriate because the most important data is there, given there is also a warning about the missing data.

ultimately, when the platform adds support for annotations in FGT the warning will be gone and the platform possibly doesn't need to provide extra info about the error which makes it more security by obscurity

Thanks @wingleung, your write up makes sense to me! I don't know how easy it is to change the platform error message in this case as I believe there is a lot of 🌈 magic 🌈 in the authorization code. That said, it seems like if we choose to suppress the error on GET annotations and provide our own warning, then that change has less value for us in the CLI.

I agree with your statement about exit codes.

Would you be interested in opening a PR for this?

indeed, it would be more for debugging faster why I couldn't fetch the annotations with my token. and I would also understand if platform people don't want this level of detail in their error messaging.

I can take a look later this week 🙏

Looking at this a bit closer, presumably this is also an issue for gh run view which also render annotations?

@williammartin thanks for the heads up! PR updated with changes to the view command as well

@bendavies this should go out in the next release but if you'd like to try it now and provide feedback then you can build from source.