Cache stable executor skills and project them per model step - #29960
Closed
jif-oai wants to merge 13 commits into
Closed
Cache stable executor skills and project them per model step#29960jif-oai wants to merge 13 commits into
jif-oai wants to merge 13 commits into
Conversation
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1531dc319e
ℹ️ 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
force-pushed
the
jif/activate-selected-capability-skills
branch
from
June 25, 2026 11:18
1531dc3 to
d04c1d5
Compare
jif-oai
changed the base branch from
jif/cover-selected-capability-lifecycle
to
jif/thread-scoped-executor-connectors
June 25, 2026 11:18
jif-oai
force-pushed
the
jif/activate-selected-capability-skills
branch
from
June 25, 2026 12:07
d04c1d5 to
6c9dde7
Compare
jif-oai
force-pushed
the
jif/activate-selected-capability-skills
branch
from
June 25, 2026 13:07
6c9dde7 to
dbc53c5
Compare
jif-oai
force-pushed
the
jif/thread-scoped-executor-connectors
branch
from
June 25, 2026 13:07
6ed45b0 to
740c8d8
Compare
This was referenced Jun 25, 2026
jif-oai
force-pushed
the
jif/activate-selected-capability-skills
branch
from
June 25, 2026 16:16
0015e2c to
cffa959
Compare
jif-oai
force-pushed
the
jif/activate-selected-capability-skills
branch
from
June 25, 2026 16:45
cffa959 to
addf6b5
Compare
…e-selected-capability-skills
jif-oai
added a commit
that referenced
this pull request
Jun 25, 2026
…step (#29856) ## Why `selectedCapabilityRoots` is durable thread intent: “use this capability root from environment `worker`.” The important product assumption is: > One environment ID always names the same logical executor and stable contents. `worker` does not silently change from executor A to an unrelated executor B. The process-local connection handle for `worker` can still be replaced while Codex is running, though, for example when `environment/add` registers a fresh handle for the same logical environment. The thread should persist only the stable selection. Each model step should pair that selection with the exact ready handle captured for that step. ## The boundary ```text persisted thread intent plugin@1 -> environment "worker" | | capture the current step v model-step view unavailable, or plugin@1 + worker's exact captured ready handle ``` The environment ID is the stable identity and cache key. The `Arc<Environment>` is only a process-local handle retained so consumers of one model step use the same captured environment. It is never persisted and it does not imply different environment contents. ## What changes ### Persist the stable selection Selected roots are written into `SessionMeta` and restored with the thread. Forked subagents inherit the same selections, including bounded-history forks. Only stable data is persisted: root ID, environment ID, and root path. ### Capture readiness together with the exact handle The environment snapshot records: ```rust environment_id -> Some(Arc<Environment>) // ready in this step environment_id -> None // still starting in this step ``` This prevents readiness and execution from coming from different registry snapshots. For example: ```text step snapshot: worker -> handle A, ready environment/add: worker -> fresh handle B for the same logical environment current step: plugin@1 still uses captured handle A ``` Without carrying handle A in the snapshot, the resolver could combine “A was ready” with handle B and treat B as ready before it had finished starting. This does not change cache invalidation. Stable capability metadata remains identified by environment ID and capability root. Replacing a process-local handle under the same stable environment ID does not invalidate or rediscover that metadata. ### Resolve availability per model step - A ready captured environment produces resolved roots using its captured handle. - A starting, missing, or failed environment is omitted from that step. - A selected lazy environment that is outside the turn's captured environment set is asked to start, and a later step can observe it as ready. - No capability files are scanned here. Transient transport disconnects remain the remote client's reconnect concern. This PR models initial attachment/readiness; it does not add live socket-connectivity state. ## Example ```text thread selection: plugin@1 -> environment "worker" step 1: worker is starting -> plugin@1 unavailable step 2: worker is ready -> plugin@1 resolves through worker's captured handle step 3: fresh local handle -> current step remains pinned; a later step captures its own view ``` Temporary unavailability does not discard the durable selection. Later PRs can retain stable metadata caches while projecting only currently available capabilities into model-visible World State. ## Compatibility The app-server request shape does not change. Older rollouts without `selected_capability_roots` deserialize to an empty list. ## Stack 1. **This PR:** persist stable selected roots and resolve them through an exact model-step handle. 2. #29960: cache stable skill metadata and project available skills into World State. 3. #29946: cache stable plugin declarations and manage the separate live MCP runtime.
jacks0n
pushed a commit
to jacks0n/codex
that referenced
this pull request
Jul 6, 2026
…step (openai#29856) ## Why `selectedCapabilityRoots` is durable thread intent: “use this capability root from environment `worker`.” The important product assumption is: > One environment ID always names the same logical executor and stable contents. `worker` does not silently change from executor A to an unrelated executor B. The process-local connection handle for `worker` can still be replaced while Codex is running, though, for example when `environment/add` registers a fresh handle for the same logical environment. The thread should persist only the stable selection. Each model step should pair that selection with the exact ready handle captured for that step. ## The boundary ```text persisted thread intent plugin@1 -> environment "worker" | | capture the current step v model-step view unavailable, or plugin@1 + worker's exact captured ready handle ``` The environment ID is the stable identity and cache key. The `Arc<Environment>` is only a process-local handle retained so consumers of one model step use the same captured environment. It is never persisted and it does not imply different environment contents. ## What changes ### Persist the stable selection Selected roots are written into `SessionMeta` and restored with the thread. Forked subagents inherit the same selections, including bounded-history forks. Only stable data is persisted: root ID, environment ID, and root path. ### Capture readiness together with the exact handle The environment snapshot records: ```rust environment_id -> Some(Arc<Environment>) // ready in this step environment_id -> None // still starting in this step ``` This prevents readiness and execution from coming from different registry snapshots. For example: ```text step snapshot: worker -> handle A, ready environment/add: worker -> fresh handle B for the same logical environment current step: plugin@1 still uses captured handle A ``` Without carrying handle A in the snapshot, the resolver could combine “A was ready” with handle B and treat B as ready before it had finished starting. This does not change cache invalidation. Stable capability metadata remains identified by environment ID and capability root. Replacing a process-local handle under the same stable environment ID does not invalidate or rediscover that metadata. ### Resolve availability per model step - A ready captured environment produces resolved roots using its captured handle. - A starting, missing, or failed environment is omitted from that step. - A selected lazy environment that is outside the turn's captured environment set is asked to start, and a later step can observe it as ready. - No capability files are scanned here. Transient transport disconnects remain the remote client's reconnect concern. This PR models initial attachment/readiness; it does not add live socket-connectivity state. ## Example ```text thread selection: plugin@1 -> environment "worker" step 1: worker is starting -> plugin@1 unavailable step 2: worker is ready -> plugin@1 resolves through worker's captured handle step 3: fresh local handle -> current step remains pinned; a later step captures its own view ``` Temporary unavailability does not discard the durable selection. Later PRs can retain stable metadata caches while projecting only currently available capabilities into model-visible World State. ## Compatibility The app-server request shape does not change. Older rollouts without `selected_capability_roots` deserialize to an empty list. ## Stack 1. **This PR:** persist stable selected roots and resolve them through an exact model-step handle. 2. openai#29960: cache stable skill metadata and project available skills into World State. 3. openai#29946: cache stable plugin declarations and manage the separate live MCP runtime.
Contributor
|
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. |
This was referenced Jul 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The model must see the same skills that Codex can read from the current executor. Environment availability can change between sampling steps, but an environment ID and the contents behind its selected root are stable.
Therefore skill metadata should be discovered once, not reread every time the environment reconnects or the model samples again.
The flow
The same snapshot decides what the model sees and which current environment handle reads a selected skill body.
Stable cache identity
The cache key is stable data only:
It deliberately excludes the process-local environment
Arc.Each cached entry also owns the stable
SkillSourceIdentityused for instruction deduplication. Reconnecting the same environment therefore does not make an unchanged skill look like a new instruction that must be injected again.Catalogs containing warnings are not cached, so transient read failures can retry. There is intentionally no file-watcher or filesystem invalidation: selected environment contents are stable for the session.
Metadata versus reads
Metadata is reusable, but a skill body is read through the current ready connection:
This avoids repeated catalog RPCs without converting executor resources into ambient host paths.
World State behavior
World State assembly remains in core in this PR. The skills extension continues to own its orchestrator source and
skills.list/skills.readtools. Moving the World State section fully into the extension still requires a general per-step World State contributor API and is intentionally separate from this cache contract.Verification
The remote-environment integration coverage starts with the environment unavailable, observes the skill after it becomes ready, then samples again and verifies that executor reads do not increase after the initial catalog discovery and selected-body read.
Stack