Register agent identities behind use_agent_identity#15588
Register agent identities behind use_agent_identity#15588adrian-openai wants to merge 3 commits into
Conversation
5a580fd to
e0d4f9e
Compare
| } | ||
| } | ||
|
|
||
| pub(crate) async fn ensure_registered_identity(&self) -> Result<Option<StoredAgentIdentity>> { |
There was a problem hiding this comment.
Could be a future thing, but If the agent is unable to register, shouldn't codex hang until it is capable of registering? I'd assume that an enterprise user wouldn't be able to do much with an unregistered agent.
There was a problem hiding this comment.
I think that's pretty reasonable. But only if the feature flag is on, of course!
|
|
||
| // Start the watcher after SessionConfigured so it cannot emit earlier events. | ||
| sess.start_file_watcher_listener(); | ||
| sess.start_agent_identity_registration(); |
There was a problem hiding this comment.
We should probably try to start registration after the user has gone through the onboarding flow and/or logged in, because no credentials would be available yet for this session. In this current flow, the user would need to restart the codex client in order for the session to get the credentials and actually succeed in registering an agent.
There was a problem hiding this comment.
We should probably try to start registration after the user has gone through the onboarding flow and/or logged in, because no credentials would be available yet for this session. In this current flow, the user would need to restart the codex client in order for the session to get the credentials and actually succeed in registering an agent.
Interesting! That's a good catch. Let me update that.
|
|
||
| impl AgentIdentityBinding { | ||
| fn from_auth(auth: &CodexAuth, forced_workspace_id: Option<String>) -> Option<Self> { | ||
| if !auth.is_chatgpt_auth() { |
There was a problem hiding this comment.
Does this mean that we don't support AuthTokens either?
There was a problem hiding this comment.
We support only chatgpt auth, which is the auth token, right?
There was a problem hiding this comment.
My understanding is that there were two Auth flows that used tokens in codex, where one was used more directly. Looking at the current version of the auth, I'd say this is fine.
There was a problem hiding this comment.
It looks like both of these flows are considered.. the same auth flow? So it's fine!
| debug!( | ||
| url = %url, | ||
| status = %status, | ||
| "agent identity registration endpoint unavailable at candidate URL; trying fallback" |
There was a problem hiding this comment.
Well, it falls back to the other URL, right? Probably don't need to do that, though.
|
Closing this pull request because it has had no updates for more than 14 days. If you plan to continue working on it, feel free to reopen or open a new PR. |
Stack
Summary
AgentIdentityManagerand the agent-registration flow behinduse_agent_identityTesting