Fix/GitHub disconnect - #122
Conversation
a765983 to
9889411
Compare
ieliofficial
left a comment
There was a problem hiding this comment.
Backend shape is right and both revoked paths on /storages are tested.
It only covers /storages though. /storage/name-availability (auth.js:117)
and /storage/create (auth.js:147) still answer a revoked token with a 400
and no GITHUB_AUTH_REVOKED code, so the frontend falls back to matching
the error string. Can those two use githubAccessRevoked() as well? That's
also what #121 asks for.
One other thing, not in the body: Cancel on Connect now signs you out
(App.jsx:376). Right for the revoked case, but is it right for someone
who just clicked Connect by mistake?
Those two and I'll merge.
…t and remove redundant checks in storage routes
|
Close, but getOctokit has a bug in the one path this PR exists for. It calls githubAccessRevoked(res), but getOctokit only takes clients and makeAuthRouter has no res in scope, so that branch throws a ReferenceError. The catch checks err.status === 401 and a ReferenceError has none, so a missing client comes back as a 500 "Failed to list storages" instead of the 401. Tests pass because the revoked test supplies githubClient: { mock: true }, so a client exists and that branch never runs. Pass res in, and have the caller return early rather than assigning the guard result to octokit, otherwise the handler carries on and tries to send a second response. Also /storage/create still has its own inline check returning a 400 with no code, which is exactly the string matching #121 is meant to remove. One sequencing note: #104 is approved and deletes /storage/name-availability, so once it lands, rebase on main and you will drop the work on that endpoint and the ConnectView conflict in one go. No rush on this until then. |
closes #121