- Notifications
You must be signed in to change notification settings - Fork1.1k
chore: refactor instance identity to be a SessionTokenProvider#19566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
spikecurtis commentedAug 27, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This stack of pull requests is managed byGraphite. Learn more aboutstacking. |
1b453cd to421347bCompare6f7bb8d toad7200fCompare421347b to7b1affeCompare7789799 tod9ee61fCompare7b1affe toa13a334Comparead45ac9 toce4943cCompare53e1b76 to7ea81d2Compare
mafredri left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
For the most part, this looks like a good and a useful refactor 👍🏻.
I'd like to see something done about the root command logic/flag, however (see comment). Other than that just minor suggestions.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| @@ -0,0 +1,97 @@ | |||
| package agentsdk | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Suggestion (optional): Consider moving into sub-package, assuming there aren't any blocking dependencies to this package that would cause a circular import.
ce4943c to608b392Compare7ea81d2 to192c81eCompare59667e7 to54f6878Compare54f6878 to6cddf93Compare
mafredri left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This turned out nice, and love the explicit flag docs this gives us, thanks for implementing! Two minor suggestions but nothing blocking![]()
| // to check if the command was invoked. | ||
| ifgitauth.CheckCommand(i.Args,i.Environ.ToOS()) { | ||
| returnr.gitAskpass().Handler(i) | ||
| returngitAskpass(hiddenAgentAuth).Handler(i) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nice solution 👍🏻
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
6cddf93 to68c9194Compare1354d84 intomainUh oh!
There was an error while loading.Please reload this page.
spikecurtis commentedSep 3, 2025
Merge activity
|

Uh oh!
There was an error while loading.Please reload this page.
Refactors Agent instance identity to be a SessionTokenProvider.
Refactors the CLI to create Agent clients via a centralized function, rather than add-hoc via individual command handlers and their flags.
This allows commands besides
coder agent, but which still use the agent identity, to support instance identity authentication.Fixes#19111 by unifying all API requests to go thru the SessionTokenProvider for auth credentials.