Skip to content

Share runtime skill rendering and selection - #29943

Closed
jif-oai wants to merge 3 commits into
jif/activate-selected-capability-skillsfrom
jif/prepare-selected-capability-snapshots
Closed

Share runtime skill rendering and selection#29943
jif-oai wants to merge 3 commits into
jif/activate-selected-capability-skillsfrom
jif/prepare-selected-capability-snapshots

Conversation

@jif-oai

@jif-oai jif-oai commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Why

Host and extension skills currently use different rendering and selection helpers. Before one step snapshot can own every source, those helpers need to understand the same neutral catalog entries without losing the mature host behavior.

Doing this separately keeps the dynamic cutover small and reviewable.

What changed

  • Generalize the existing bounded skills renderer to source-neutral catalog entries.
  • Preserve host path aliases, scope ordering, description shrinking, omission warnings, and the existing render report/metrics.
  • Move exact-locator and plain-name selection onto the shared catalog model.
  • Share the bounded skill-instruction/read result used by the existing injection path.
  • Adapt the current extension path without changing when skills refresh or who owns model context yet.
  • Use the mature fair allocator for extension catalogs too, so one oversized description cannot consume the whole list.
  • Keep catalog order deterministic and enforce an 8 KB byte cap even for multibyte text.
  • Ignore a plain $name when it also names a connector; an exact skill locator still works.

Example

The renderer now accepts entries such as:

host         deploy -> (file: /repo/.codex/skills/deploy/SKILL.md)
executor     deploy -> (environment resource: skill://demo@1/skills/deploy/SKILL.md)
orchestrator search -> (orchestrator resource: skill://orchestrator/search/SKILL.md)

It still applies one hard metadata budget, shortens descriptions before dropping skill names, and keeps host aliases when they save space. Absolute paths and aliases are evaluated independently, so a long absolute path cannot hide a usable alias.

This PR does not change refresh timing or ownership. It does make the existing extension catalog use the same bounded rendering rules as host skills. The next PR uses these shared primitives to remove the parallel owners and refresh the complete snapshot per model step.

@jif-oai
jif-oai requested a review from a team as a code owner June 25, 2026 01:27

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

Copy link
Copy Markdown
Contributor

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: 70e6930d9b

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

Comment thread codex-rs/ext/connectors/src/selected.rs Outdated
Comment on lines +145 to +149
let selected_capabilities = activation.snapshot().selected_capabilities().clone();
let connector_snapshot = self
.snapshot_for_selected_capabilities(&selected_capabilities, cache.as_ref())
.await;
connector_state.publish(connector_snapshot);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Badge Expose activation snapshots to connector consumers

When a thread is initialized with SelectedCapabilityActivation, this path only publishes the computed connectors into ConnectorSnapshotState; the existing runtime still reads ConnectorSnapshot from ExtensionDataInit in core/src/mcp.rs (connector_snapshot_for_thread and the selected-MCP routing check). In that activation mode the selected connector declarations are therefore invisible to tool construction and hosted-app routing even though they were resolved here, so selected plugin apps will be omitted or routed as standalone MCP servers until the active snapshot is stored in the type consumers actually read (or those consumers are switched to ConnectorSnapshotState).

Useful? React with 👍 / 👎.

Comment thread codex-rs/ext/connectors/Cargo.toml Outdated
codex-utils-path-uri = { workspace = true }
serde_json = { workspace = true }
thiserror = { workspace = true }
tokio = { workspace = true, features = ["sync"] }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Badge Refresh the Bazel lockfile for new Rust deps

This patch adds new Rust dependencies/features in Cargo.toml but only commits Cargo.lock; Bazel's crate universe will still be based on the old MODULE.bazel.lock, so the Bazel lock drift check will fail until just bazel-lock-update is run and the resulting lockfile diff is included.

AGENTS.md reference: AGENTS.md:L37-L39

Useful? React with 👍 / 👎.

@jif-oai
jif-oai force-pushed the jif/prepare-selected-capability-snapshots branch from 70e6930 to 9ebb05c Compare June 25, 2026 11:18
@jif-oai jif-oai changed the title Prepare selected capability runtime snapshots Share runtime skill rendering and selection Jun 25, 2026
@jif-oai
jif-oai changed the base branch from jif/share-selected-capability-bindings to jif/activate-selected-capability-skills June 25, 2026 11:18
@jif-oai
jif-oai force-pushed the jif/activate-selected-capability-skills branch from d04c1d5 to 6c9dde7 Compare June 25, 2026 12:07
@jif-oai
jif-oai force-pushed the jif/prepare-selected-capability-snapshots branch from 9ebb05c to 0a8b48c Compare June 25, 2026 12:07
@jif-oai
jif-oai force-pushed the jif/activate-selected-capability-skills branch from 6c9dde7 to dbc53c5 Compare June 25, 2026 13:07
@jif-oai

jif-oai commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Folded into #29960. The skill source, rendering, selection, and per-step World State changes are now one coherent PR with one runtime boundary.

@jif-oai jif-oai closed this Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant