feat: opt ChatGPT auth into agent identity#19049
Conversation
3254ce5 to
0324e8a
Compare
42e2d28 to
3192e48
Compare
a4a465a to
92cf11e
Compare
0324e8a to
2685c9e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92cf11ed3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| RolloutItem::Compacted(_) | ||
| | RolloutItem::EventMsg(_) | ||
| | RolloutItem::SessionMeta(_) | ||
| | RolloutItem::SessionState(_) => true, |
There was a problem hiding this comment.
Exclude SessionState from forked rollout history
keep_forked_rollout_item retains RolloutItem::SessionState, so a fork copies parent task-state lines into the child rollout. On a later resume, restore_persisted_agent_task replays the latest session-state entry and can restore the parent task_id into the forked thread, defeating independent task identity for forks.
Useful? React with 👍 / 👎.
|
High level not sure we want to persist the task id in rollout state. Ik in initial reviews of the old stack i flagged this as problematic due to the deeper problem (having different tasks for same session id), but idt this is the solution we'd want long term. ReasoningIt seems orthogonal/like a (potentially unstable) implementation detail that our deeper auth logic shouldn't care about, and we can't gate rollout item changes on feature flag. Rollout data really matters & we'd want to keep very good hygiene. If we can, let's default to making task id ephemeral and shield it as much as possible from core session code. Solution MusingAs a related note, it'd be so awesome if we could actually choose the task id name or establish some link between a session id and a task id. That way we get consistency backend-side rather than having to concern a lot of our app-server code with this stuff. SuggestionSo in the AuthEnum stack, we have pretty much everything related to auth abstracted into the 'Auth' provider. Almost every call basically only has to say 'hey give me an auth provider and have that inject headers'. So I think what we can do when enabling agent-identity feature is adding a different auth provider. What'd be amazing as an abstraction is if we could store some of this ephemeral stuff in-memory on the auth object (e.g. hashmap of session ids to task ids) and basically have the provider take our 'auth' and potentially a session id hint. |
2685c9e to
55dced3
Compare
92cf11e to
cfa9996
Compare
ae79fa8 to
492eec5
Compare
03963fc to
8810088
Compare
6ffcd6e to
228cb99
Compare
8810088 to
ef4a89f
Compare
228cb99 to
994dfd3
Compare
ef4a89f to
6b3e08d
Compare
a07016b to
70afafb
Compare
363df7e to
1d62ce5
Compare
70afafb to
0e340c9
Compare
1d62ce5 to
f220d79
Compare
0e340c9 to
d8a54ed
Compare
f220d79 to
134085f
Compare
2cc6695 to
76f7f2b
Compare
134085f to
d13cbcc
Compare
76f7f2b to
82ac2e2
Compare
7026cbc to
d7289e0
Compare
82ac2e2 to
b280037
Compare
d7289e0 to
a83dc38
Compare
54e0629 to
a79c53b
Compare
2b9ca9a to
7f15c05
Compare
Stack
This is PR 2 of the simplified HAI single-run-task stack:
#19054 collapsed out of the active stack because the simplified design no longer needs a separate background/control-plane task helper.
Summary
This PR adds the disabled-by-default path for normal ChatGPT-login Codex sessions to obtain Agent Identity runtime auth through the Codex backend. Existing Agent Identity JWT startup mode remains a separate path and does not require the feature flag.
What changed:
use_agent_identityfeature flag and config schema entryAgentIdentityAuthPolicyso call sites chooseJwtOnlyorChatGptAuthinstead of passing a bare booleanauth.jsonso process restarts reuse the same identityuse_agent_identityis enabledThis PR intentionally does not switch model inference over to
AgentAssertionauth. The provider-auth integration lands in the next PR.Testing
just test -p codex-login