Add hooks/list app-server RPC#19778
Conversation
- anomalyco/opencode#24574 ignore: split up reasoning transforms (merge-as-is) - anomalyco/opencode#24573 fix: vertex anthropic toolStreaming default (merge-as-is) - openai/codex#19778 hooks/list + hooks/config/write APIs (merge-after-nits) - openai/codex#19772 permissions: derive config defaults as profiles (merge-after-nits)
Co-authored-by: Codex <noreply@openai.com>
…ry' into dev/abhinav/hooks-list-config # Conflicts: # codex-rs/app-server/src/codex_message_processor.rs
…ry' into dev/abhinav/hooks-list-config # Conflicts: # codex-rs/core-plugins/src/loader.rs # codex-rs/hooks/src/engine/discovery.rs
| configured_marketplace_upgrade_state: RwLock<ConfiguredMarketplaceUpgradeState>, | ||
| non_curated_cache_refresh_state: RwLock<NonCuratedCacheRefreshState>, | ||
| cached_enabled_outcome: RwLock<Option<PluginLoadOutcome>>, | ||
| cached_enabled_outcome: RwLock<Option<(bool, PluginLoadOutcome)>>, |
There was a problem hiding this comment.
need this in case the flag changes, we can't return the same cached results
temporary while plugin_hooks is under development, will rip all this out once the safety + trust has landed
There was a problem hiding this comment.
Can you add a comment suggesting the meaning of this bool so people don't need to read the code?
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 425cd2d72b
ℹ️ 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".
| ) | ||
| .await; | ||
| let hooks = codex_hooks::list_hooks(codex_hooks::HooksConfig { | ||
| feature_enabled: config.features.enabled(Feature::CodexHooks), |
There was a problem hiding this comment.
Use per-cwd feature flags when listing hooks
hooks_list_response resolves a config layer stack for each requested cwd, but it gates plugin hooks and codex hooks using config.features loaded once before the loop. If different cwds have different feature settings (e.g. features.codex_hooks or features.plugin_hooks), the API returns incorrect hook inventories for non-primary paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I think codex is just getting tripped up by the naming
- we check the features from the actual
cwdthat codex is running from - the
cwds that get provided tohooks/list(alsoskills/list) is just a list of dirs to look forhooks.jsonfiles. we don't wanna respect any potential features in those dirs
## Why Plugins can bundle lifecycle hooks, but Codex previously only discovered hooks from user, project, and managed config layers. This adds the plugin discovery and runtime plumbing needed for plugin-bundled hooks while keeping execution behind the `plugin_hooks` feature flag. ## What - Discovers plugin hook sources from each plugin's default `hooks/hooks.json`. - Supports `plugin.json` manifest `hooks` entries as either relative paths or inline hook objects. - Plumbs discovered plugin hook sources through plugin loading into the hook runtime when `plugin_hooks` is enabled. - Marks plugin-originated hook runs as `HookSource::Plugin`. - Injects `PLUGIN_ROOT` and `CLAUDE_PLUGIN_ROOT` into plugin hook command environments. - Updates generated schemas and hook source metadata for the plugin hook source. ## Stack 1. This PR - #19705 2. #19778 3. #19840 4. #19882 ## Reviewer Notes - Core logic is in `codex-rs/core-plugins/src/loader.rs` and `codex-rs/hooks/src/engine/discovery.rs` - Moved existing / adding new tests to `codex-rs/core-plugins/src/loader_tests.rs` hence the large diff there - Otherwise mostly plumbing and minor schema updates ### Core Changes The `codex-rs/core` changes are limited to wiring plugin hook support into existing core flows: - `core/src/session/session.rs` conditionally pulls effective plugin hook sources and plugin hook load warnings from `PluginsManager` when `plugin_hooks` is enabled, then passes them into `HooksConfig`. - `core/src/hook_runtime.rs` adds the `plugin` metric tag for `HookSource::Plugin`. - `core/config.schema.json` picks up the new `plugin_hooks` feature flag, and `core/src/plugins/manager_tests.rs` updates fixtures for the added plugin hook fields. --------- Co-authored-by: Codex <noreply@openai.com>
…t-config # Conflicts: # codex-rs/app-server-protocol/schema/json/ClientRequest.json # codex-rs/core-plugins/src/loader.rs # codex-rs/hooks/src/engine/discovery.rs # codex-rs/hooks/src/engine/mod.rs
…t-config # Conflicts: # codex-rs/app-server-protocol/schema/json/ClientRequest.json # codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json # codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json # codex-rs/app-server-protocol/schema/typescript/v2/index.ts # codex-rs/hooks/src/lib.rs
…nto dev/abhinav/hooks-list-config
|
PR reads well! One minor concern. Can you pull on this thread:
So my understanding of this issue:
What happens now: The RPC correctly loads repo B’s hook definitions via repo B’s ConfigLayerStack. Then it passes feature_enabled: config.features.enabled(Feature::CodexHooks) from the global config, which still reflects repo A Result: hooks/list says “no hooks” for repo B |
…t-config # Conflicts: # codex-rs/app-server/src/codex_message_processor.rs # codex-rs/core/src/plugins/manager.rs # codex-rs/core/src/session/handlers.rs
…t-config # Conflicts: # codex-rs/core/src/plugins/manager.rs
…nto dev/abhinav/hooks-list-config
Why
We need a way to list the available hooks to expose via the TUI and App so users can view and manage their hooks
What
hooks/listfor one or morecwdvalues that returns discovered hook metadataStack
Review Notes
The generated schema files account for most of the raw diff, these files have the core change:
hooks/src/engine/discovery.rsbuilds the inventory entries during hook discovery while leaving runtime handlers focused on execution.app-server/src/codex_message_processor.rswireshooks/listinto the app-server flow for each requestedcwd.app-server-protocol/src/protocol/v2.rsdefines the new v2 request/response payloads exposed on the wire.Core Changes
core/src/plugins/manager.rsaddsplugins_for_layer_stack(...)soskills/listandhooks/listcan resolve plugin state for each requestedcwd