Skip to content

fix: agent when profile#13235

Merged
jif-oai merged 4 commits intomainfrom
jif/agent-profile
Mar 3, 2026
Merged

fix: agent when profile#13235
jif-oai merged 4 commits intomainfrom
jif/agent-profile

Conversation

@jif-oai
Copy link
Collaborator

@jif-oai jif-oai commented Mar 2, 2026

No description provided.

@jif-oai
Copy link
Collaborator Author

jif-oai commented Mar 2, 2026

@codex review

Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0fbebfdf3b

ℹ️ 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".

@jif-oai
Copy link
Collaborator Author

jif-oai commented Mar 2, 2026

@codex review

Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0558a2f2ae

ℹ️ 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".

@jif-oai
Copy link
Collaborator Author

jif-oai commented Mar 2, 2026

@codex review

@chatgpt-codex-connector
Copy link
Contributor

Codex Review: Didn't find any major issues. Hooray!

ℹ️ 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".

Document how sub-agent roles preserve the caller's active profile and
model provider unless the role explicitly overrides them. Also explain
that multi-agent spawn config is rebuilt from live turn state before
role-specific config is layered on top.

Co-authored-by: Codex <noreply@openai.com>
@joshka-oai
Copy link
Collaborator

Follow-up on the previously dismissed P1 review: I checked the current branch state, and that concern is stale rather than current.

apply_role_to_config does not blindly pin the existing provider whenever a role omits top-level model_provider / profile. The current code only preserves the old provider when the role neither:

  • sets top-level profile
  • sets top-level model_provider
  • rewrites the active profile's model_provider inside [profiles.<active>]

That last case is handled by role_updates_active_profile_provider in codex-rs/core/src/agent/role.rs, and there is now a direct regression test for it: apply_role_uses_active_profile_model_provider_update.

I pushed a docs-only follow-up commit that makes this ownership rule explicit in role.rs and in the multi-agent spawn config path, so the intended behavior is easier to review and less likely to be misread later.

@joshka-oai
Copy link
Collaborator

PR Description

Problem

Applying an agent role rebuilds config from layered TOML. Before this change, a role that did not
explicitly set profile or model_provider could still cause the spawned agent to lose the
caller’s active profile/provider selection during that rebuild. That made a small role config look
like a harmless patch while still being able to knock a child agent back to the default provider.

Mental model

A spawned agent should start from the parent turn’s effective runtime config and then optionally add
a role layer. Roles are patch-like: they only take over model selection when they explicitly name a
profile, explicitly name a model_provider, or edit the currently active profile’s provider in
place. If they do not do one of those things, the parent turn still owns provider selection.

Non-goals

This change does not redefine role precedence, add new role syntax, or change how built-in roles
are chosen. It also does not broaden the multi-agent API surface or add new observability signals;
it makes the existing contract explicit and tests the intended behavior.

Tradeoffs

The preservation rule is inferred from raw role TOML before the merged config is rebuilt. That keeps
behavior aligned with current layering semantics, but it means the contract lives partly in targeted
key inspection rather than a single typed abstraction. The docs in this follow-up commit call that
heuristic out so future edits do not accidentally simplify it away.

Architecture

apply_role_to_config resolves the selected role file, validates it with the normal config loader,
and inserts it as a SessionFlags layer. Before reloading config, it decides whether the parent
turn must keep ownership of active_profile and model_provider. build_agent_spawn_config in
the multi-agent handler explains the other half of the contract: spawned agents first inherit live
turn state such as provider, approval policy, sandbox, and cwd, and only then accept role-level
overrides.

Observability

There are no new logs. The fastest debug path is to inspect the spawned agent’s config snapshot for
active_profile, model_provider_id, and approval policy, then compare that against the role TOML
and the parent turn context.

Tests

Coverage exercises the three model-selection cases that matter: a role that inherits the current
profile/provider, a role that replaces the active profile, and a role that directly replaces the
provider or edits the active profile’s provider in place. The existing multi-agent spawn test also
documents that the child agent should preserve turn-owned runtime state while layering role config
on top.

@jif-oai can you validate that this is an accurate summary of the problem / change? If so, it would
be worth folding the key parts into the main PR description/comment so reviewers do not have to
reconstruct the role/provider ownership rules from the diff.

@jif-oai jif-oai merged commit cacefb5 into main Mar 3, 2026
30 of 31 checks passed
@jif-oai jif-oai deleted the jif/agent-profile branch March 3, 2026 09:20
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants