Skip to content

return Failure response when agent does not support extension - #109

Merged
wiktor-k merged 2 commits into
wiktor-k:mainfrom
71rd:extension-failure
Jul 14, 2026
Merged

return Failure response when agent does not support extension#109
wiktor-k merged 2 commits into
wiktor-k:mainfrom
71rd:extension-failure

Conversation

@71rd

@71rd 71rd commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

fix for #108

@71rd
71rd marked this pull request as draft June 18, 2026 18:48
@71rd
71rd force-pushed the extension-failure branch from 6484a3e to fec8a18 Compare June 18, 2026 18:50
@wiktor-k

Copy link
Copy Markdown
Owner

Hi,

Thanks for the contribution 🙏 we definitely want this merged! I just need a moment to check if we don't need to support this for other commands too and think a bit about tests.

Hopefully that's not a problem. I'll be back 👋

Signed-off-by: git@71rd.net <git@71rd.net>
@71rd
71rd force-pushed the extension-failure branch from fec8a18 to 8f2908f Compare June 23, 2026 16:36
@71rd

71rd commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

thanks for the response.
I'm not in a hurry, i just wanted to remove a few unnecessary warnings in ssh-agent-mux 😃
i suspect it makes sense to also add it to the blocking module so i also handled the case there.
if you find something else that should be added or changed about this PR i'm happy to help

@jcspencer

Copy link
Copy Markdown
Collaborator

This looks correct to me; the agent protocol has been recently codified as RFC9987, and section 5.6 defines this as the expected response when an extension either doesn’t exist, or the agent doesn’t support extensions at all.

One thing that I am noticing though is that we don’t seem to cover SSH_AGENT_EXTENSION_FAILURE (or, for us, ExtensionFailure) here either… I think this would be good to add as separate branch here too, likely with it’s own distinct AgentError variant for consumers to process.

Comment thread src/blocking.rs
Signed-off-by: git@71rd.net <git@71rd.net>
@71rd
71rd force-pushed the extension-failure branch from e2c8c49 to d523f80 Compare July 8, 2026 17:55
@71rd

71rd commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

thanks @jcspencer, seems like a good suggestion.

i added the case for client.rs and blocking.rs

@71rd

71rd commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

please ignore the earlier push mix 🙈
it should be fixed now

@71rd
71rd marked this pull request as ready for review July 8, 2026 18:03
@wiktor-k
wiktor-k merged commit 1ed4766 into wiktor-k:main Jul 14, 2026
18 checks passed
@wiktor-k

Copy link
Copy Markdown
Owner

Phew, merged! Sorry for the delay and thank you very much for your contribution! 🙇

It may take a moment to get into a new release since I've got another idea for something else but we'll get there... eventually 😅

Have a nice day! 👋

@71rd
71rd deleted the extension-failure branch July 14, 2026 23:47
@71rd

71rd commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

All good :D
Thank you for maintaining the library
Wish you also have a nice day

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants