feat(plugins): Phase 1 - plugin manifest discovery and loader infrastructure#4
Merged
feat(plugins): Phase 1 - plugin manifest discovery and loader infrastructure#4
Conversation
…cture
Phase 1 of the Claude Code plugins integration plan. Adds the data
shapes, filesystem discovery, and service plumbing needed to surface
plugin-contributed components to downstream subsystems in later phases.
Core types (crates/forge_domain/src/plugin.rs):
- PluginManifest with optional name, version, description, author,
homepage, repository, license, keywords, dependencies, hooks,
commands/agents/skills paths, and mcpServers. Unknown fields are
silently dropped to match Claude Code's permissive parser.
- PluginAuthor with untagged enum accepting either a bare string or
a structured {name, email, url} object.
- PluginComponentPath supporting single-string or string-array forms.
- PluginHooksManifestField placeholder supporting path/inline/array
variants (full hook execution lands in Phase 3).
- PluginHooksConfig opaque container for the hooks.json body.
- PluginSource enum with Global, Project, and Builtin variants.
- LoadedPlugin runtime DTO with resolved absolute paths, effective
name, enabled state, MCP servers, and hooks config.
- PluginRepository trait defining load_plugins.
- 13 unit tests covering manifest parsing, author shorthand vs detailed,
component path single/multiple, hooks field variants, unknown-fields
tolerance, and malformed-JSON error propagation.
Environment paths (crates/forge_domain/src/env.rs):
- plugin_path: ~/forge/plugins/
- plugin_cwd_path: ./.forge/plugins/
- 3 tests verifying both paths and their independence.
Config integration (crates/forge_config/src/config.rs):
- PluginSetting struct with enabled flag.
- ForgeConfig.plugins: Option<BTreeMap<String, PluginSetting>> for
per-plugin enable/disable overrides in ~/forge/.forge.toml.
Discovery implementation (crates/forge_repo/src/plugin.rs):
- ForgePluginRepository scanning global ~/forge/plugins/ and
project-local ./.forge/plugins/ directories in parallel.
- Manifest probe order: .forge-plugin/plugin.json (Forge-native marker,
preferred), .claude-plugin/plugin.json (1:1 Claude Code compat),
plugin.json (bare legacy).
- Component auto-detection: commands/, agents/, skills/ subdirectories
when the manifest omits explicit paths.
- MCP server merging: inline manifest.mcp_servers + sibling .mcp.json,
inline winning on conflict.
- Project > Global precedence: project-local copies shadow global ones.
- Enable/disable overrides from .forge.toml applied post-discovery.
- Per-plugin errors logged via tracing::warn; top-level scan failures
still propagate. 2 dedup tests included.
Service layer (crates/forge_app/src/services.rs):
- New PluginLoader trait with list_plugins + invalidate_cache methods,
mirroring the SkillFetchService shape.
- Services::PluginLoader associated type with blanket impl pass-through
so any type implementing Services automatically exposes plugin
loading.
Memoised loader (crates/forge_services/src/tool_services/plugin_loader.rs):
- ForgePluginLoader<R: PluginRepository> caching results in
RwLock<Option<Arc<Vec<LoadedPlugin>>>>.
- Double-checked locking fast path for cache hits, write-lock slow
path for cold loads.
- invalidate_cache drops the cached snapshot so the next list_plugins
call re-scans the filesystem. Used by upcoming :plugin reload.
- 4 unit tests covering cold read, cache hit, invalidation, and
mid-session new-plugin surfacing.
Wiring (crates/forge_services/src/forge_services.rs, forge_repo):
- ForgeRepo now implements PluginRepository via ForgePluginRepository.
- ForgeServices gains the F: PluginRepository bound, constructs
ForgePluginLoader::new(infra.clone()), and exposes it through
type PluginLoader = ForgePluginLoader<F>.
Testing:
- 1687 tests across forge_domain, forge_config, forge_repo,
forge_services, and forge_app pass on this branch.
- forge_domain::plugin: 13 unit tests
- forge_domain::env: 3 new plugin-path tests
- forge_repo::plugin: 2 dedup tests
- forge_services::tool_services::plugin_loader: 4 memoisation tests
Deferred to later phases (documented in plans/2026-04-09-claude-code-plugins-v4):
- Phase 2: loading skills/commands/agents from plugins into existing
discovery pipelines
- Phase 3: hook execution runtime (PluginHooksConfig becomes typed)
- Phase 4: MCP server registration from plugins
- Phase 9: :plugin CLI commands
github-actions
bot
added
the
type: feature
Implements the bulk of the Claude Code plugin compatibility plan
(plans/2026-04-09-claude-code-plugins-v4/). See audit report in
plans/2026-04-09-claude-code-plugins-v4-audit-remaining-work-v1.md for
detailed ground-truth status.
Fully implemented:
- Phase 2: Skills/commands/agents unified loading with SkillSource,
CommandSource, AgentSource provenance tracking. Extended Skill
frontmatter fields (when_to_use, allowed_tools, disable_model_invocation,
user_invocable). list_invocable_commands aggregation.
- Phase 3: Hook runtime infrastructure (9 submodules: matcher, env,
shell, http, prompt, agent, config_loader, executor). PluginHookHandler
dispatcher with 25 EventHandle impls. HookExecutorInfra and
HookConfigLoaderService traits in Services.
- Phase 4: T1-Core 10 lifecycle events with fire sites in orch.rs and
compaction.rs. SessionStart with transcript creation and
initial_user_message injection. Full consume logic for PreToolUse
(block/Deny/updated_input/additional_contexts), Stop
(prevent_continuation reentry), and StopFailure via try/catch wrapper.
- Phase 10: Plugin MCP servers with {plugin}:{server} namespacing and
FORGE_PLUGIN_ROOT env injection. 7 unit tests.
Partially implemented (plumbing only — payloads + Hook slots + dispatcher
impls but no fire sites or subsystems):
- Phase 6: Notification, Setup, ConfigChange, InstructionsLoaded
- Phase 7: Subagent, Permission, Cwd, File, Worktree events
- Phase 8: Elicitation, ElicitationResult hooks
- Phase 6C: ConfigWatcher skeleton with internal-write suppression
Almost done:
- Phase 9: Plugin CLI (list/enable/disable/info/reload). Missing install
subcommand and trust prompt.
Not started (tracked in audit plan):
- Phase 5: Legacy LifecycleEvent migration and deprecation
- Phase 11: Integration tests, compat testing, benchmarks, docs
Adds a complete '/plugin install <path>' flow with trust prompt, manifest
validation, recursive directory copy, and config registration.
Model changes (crates/forge_main/src/model.rs):
- Add PluginSubcommand::Install { path: PathBuf } variant
- Parse '/plugin install <path>' with support for multi-word paths and
relative paths; path tokens after 'install' are joined with single
spaces so paths with whitespace are reconstructable
- Reject '/plugin install' with missing path argument with usage hint
- Update '/plugin' usage message to list 'install' among subcommands
- 5 new parser tests covering absolute, relative, multi-word paths,
missing-arg error, and the updated unknown-subcommand hint
UI changes (crates/forge_main/src/ui.rs):
- Add on_plugin_install handler that:
1. Canonicalizes and validates source path (exists, is directory)
2. Locates manifest at .forge-plugin/plugin.json, .claude-plugin/plugin.json,
or plugin.json (Claude Code fallback order)
3. Parses manifest via PluginManifest::from_json
4. Uses manifest name as install directory under env.plugin_path()
5. Prompts for overwrite if target directory already exists
6. Shows manifest summary (version, description, author, source)
and trust warning before asking user to confirm install
7. Copies directory recursively, excluding .git, node_modules, target,
and other VCS/build artifacts
8. Registers plugin in config as disabled via set_plugin_enabled
9. Reloads plugin list and prints success message
- Add copy_dir_recursive and should_exclude_dir helpers
- Add format_install_location helper for printing install target
- Replace non-existent ForgeSelect::confirm with ForgeWidget::confirm
(the ForgeSelect re-export was removed in a prior refactor; README
in forge_select was stale)
Phase 5 (legacy LifecycleEvent migration) was verified as already
implemented in the prior Phase 2-10 commit (PendingTodosHandler migrated
End->Stop, TracingHandler retained on legacy events as designed,
CompactionHandler fires PreCompact/PostCompact internally, legacy
variants marked #[doc(hidden)]).
Phase 9.7 (subcommand-aware autocomplete and dynamic plugin name
completion after enable/disable/info) is deferred — tracked in
plans/2026-04-09-claude-code-plugins-v4-audit-remaining-work-v1.md.
Tests: 280 forge_main, 640 forge_app, 718 forge_domain, 290 forge_services,
281 forge_repo — 0 failures, 0 regressions.
Refs: plans/2026-04-09-claude-code-plugins-v4/10-phase-9-plugin-cli.md
Wire up the Notification and Setup hook dispatch paths with production
fire sites and CLI flags.
Phase 6A — Notification hook:
- Add ForgeNotificationService implementation in crates/forge_app/src/
lifecycle_fires.rs. It sits in forge_app (not forge_services) because
PluginHookHandler is a private module in forge_app::hooks with no public
re-export — moving the impl here avoids widening forge_app's public
surface.
- Expose notification_service() on the API trait (crates/forge_api/src/
api.rs) returning Arc<dyn NotificationService>. ForgeAPI::notification_
service constructs a fresh ForgeNotificationService on each call (cheap,
no state beyond Arc<Services>).
- Wire Arc<dyn NotificationService> as a field on UI struct (crates/
forge_main/src/ui.rs). Initialized in UI::init via api.notification_
service().
- Fire NotificationKind::IdlePrompt at UI::prompt() — the single async
chokepoint before Console::prompt blocks on Reedline::read_line. Every
idle wait-for-input cycle emits the hook.
- Fire NotificationKind::AuthSuccess at finalize_provider_activation when
a provider login succeeds. Runs both model-preselected and interactive
branches (common terminal Ok(()) fire point).
- Terminal BEL emission is best-effort: io::stdout().is_terminal() +
!TERM_PROGRAM=vscode guard.
- Notification hook dispatch uses the direct EventHandle<EventData<
NotificationPayload>>::handle pattern (mirroring CompactionHandler).
Scratch Conversation is built per-call; blocking_error is drained and
discarded per services.rs:538-540 doc (observability only).
Phase 6B — Setup hook + CLI flags:
- Add --init, --init-only, --maintenance top-level flags on Cli struct
(crates/forge_main/src/cli.rs). Updates is_interactive() to return
false when init_only is set (so REPL predicate stays correct).
- Add fire_setup_hook(trigger: SetupTrigger) async method to API trait,
implemented on ForgeAPI as a thin delegate to lifecycle_fires::
fire_setup_hook.
- Fire site in UI::run_inner() after init_conversation, before the REPL
loop. Routing:
--init / --init-only → SetupTrigger::Init
--maintenance → SetupTrigger::Maintenance
(other) → no fire
- Per Claude Code semantics (hooksConfigManager.ts:175), Setup blocking
errors are intentionally ignored. The fire site drains conversation.
hook_result and discards the result. Hook dispatch failures are
logged via tracing::warn but never propagate.
- --init-only triggers early return from run_inner after the Setup fire,
so the REPL is skipped entirely (CI / batch provisioning use case).
Tests:
- 5 new CLI parser tests in cli.rs (test_cli_parses_init_flag, _init_
only_flag, _maintenance_flag, _default_has_no_setup_flags, _init_only_
is_not_interactive).
- Pre-existing dispatcher tests remain green:
test_dispatch_setup_matches_trigger_string ✅
test_dispatch_notification_matches_notification_type ✅
- Workspace: 2627 passed; 0 failed; 16 ignored.
Known gaps (deferred to Wave G):
- ForgeNotificationService does not have unit tests covering the hook
fire path — orch_spec harness cannot be used because Notifications
fire from outside the orchestrator. A probe-installed Hook with mock
Services is needed.
- should_beep() env var detection is not unit-tested due to test-parallel
env var race. Verified via manual inspection.
Refs: plans/2026-04-09-claude-code-plugins-v4/07-phase-6-t2-infrastructure.md
(lines 23-146, sub-phases 6A + 6B)
…full
Completes Phase 6C by wiring up a real filesystem watcher for Forge's
configuration surface, then connecting it to the ConfigChange plugin
hook via the existing lifecycle_fires dispatch path.
## Part 1 — notify-debouncer-full event loop
Expanded crates/forge_services/src/config_watcher.rs from the 280-line
skeleton committed in the Phase 2-10 snapshot to a 730-line production
implementation:
- Real debouncer install via notify_debouncer_full::new_debouncer with
a 1-second debounce window (matches Claude Code's
awaitWriteFinish.stabilityThreshold: 1000).
- 5-second internal-write suppression via recent_internal_writes map:
every write Forge itself performs is tagged via mark_internal_write
first, and the debouncer callback skips any event whose timestamp
falls within the suppression window.
- 1.7-second atomic-save grace period: on a Remove event the path is
stashed in pending_unlinks and a short-lived std::thread waits the
grace window. If a matching Create arrives first the pair is
collapsed into a single Modify-equivalent event; otherwise the
delayed delete fires.
- 1.5-second per-path dispatch cooldown via last_fired map to collapse
the multi-event bursts notify-debouncer-full still emits for an
atomic save on macOS FSEvents (Remove + Create + Modify + Modify).
- Path canonicalization via canonicalize_for_lookup to resolve the
macOS /var -> /private/var symlink, so the internal-write and
pending-unlink maps are keyed consistently with paths emitted by
notify-debouncer-full itself. Falls back to the raw path when the
target does not exist (delete / pre-create).
- ConfigWatcher::classify_path maps an absolute path back into a
forge_domain::ConfigSource variant using Forge's directory layout.
- ConfigWatcher::new takes Vec<(PathBuf, RecursiveMode)> plus an
Fn(ConfigChange) + Send + Sync + 'static callback. Unreadable or
missing paths are logged at debug and skipped so a non-existent
plugin directory on first startup does not abort the whole watcher.
- Re-exports notify_debouncer_full::notify::RecursiveMode.
- 11 unit tests pass 3x in a row under --test-threads=1, including
atomic save collapse, delete fire after grace, internal-write
suppression, and cooldown collapse.
## Part 2 — wire into ForgeAPI + ConfigChange hook fire
Architectural correction from the spec: forge_services already depends
on forge_app (Cargo.toml), so forge_app cannot import ConfigWatcher
directly. The watcher handle lives in forge_api, which is the single
crate that depends on both forge_app (for fire_config_change_hook) and
forge_services (for ConfigWatcher itself).
New public API:
- crates/forge_app/src/lifecycle_fires.rs: fire_config_change_hook(
services, source, file_path) async free function, re-exported from
crates/forge_app/src/lib.rs alongside fire_setup_hook. Mirrors the
Wave B Setup hook dispatch pattern exactly — builds a scratch
Conversation, constructs EventData<ConfigChangePayload> via
with_context, wraps in LifecycleEvent::ConfigChange, dispatches via
PluginHookHandler::new(services).handle, drains blocking_error,
logs dispatch failures via tracing::warn but NEVER propagates.
- crates/forge_api/src/config_watcher_handle.rs: new 150-line module
with ConfigWatcherHandle wrapper type. Stores Arc<ConfigWatcher>
internally, exposes mark_internal_write delegating to the inner
watcher, and provides a ConfigWatcherHandle::spawn(services,
watch_paths) constructor that:
1. Captures tokio::runtime::Handle::try_current() (returns a no-op
stub if no runtime — needed for test harnesses that construct
ForgeAPI outside of tokio::main).
2. Builds a sync callback closure that, per ConfigChange event,
spawns an async task calling fire_config_change_hook on the
captured runtime handle and services clone.
3. Calls ConfigWatcher::new(watch_paths, callback) and wraps the
result in Arc.
- crates/forge_api/src/lib.rs: re-exports ConfigWatcherHandle at the
crate root.
ForgeAPI integration (crates/forge_api/src/forge_api.rs):
- New field _config_watcher: Option<ConfigWatcherHandle> on ForgeAPI
struct (underscored to signal it is kept alive purely for its Drop
impl, which tears down the debouncer thread and all installed
notify watchers).
- ForgeAPI::init builds watch_paths from the environment:
(env.base_path, RecursiveMode::NonRecursive) // .forge.toml
(env.plugin_path(), RecursiveMode::Recursive) // installed plugins
- Calls ConfigWatcherHandle::spawn(services.clone(), watch_paths).ok()
so a spawn failure logs warn but still allows ForgeAPI to be
constructed (the rest of Forge does not depend on the watcher
being alive).
Internal-write suppression:
- API trait (crates/forge_api/src/api.rs) gains mark_config_write(
&self, path: &Path) with a default no-op implementation. ForgeAPI
impls it by delegating to self._config_watcher.as_ref().map(|w|
w.mark_internal_write(path)).
- set_plugin_enabled (forge_api.rs) is wired: it resolves the config
path via ConfigReader::config_path() and calls mark_config_write
before fc.write()?, so toggling a plugin via /plugin enable|disable
no longer round-trips through the ConfigChange hook system.
Second call site left as TODO:
- crates/forge_infra/src/env.rs:148 inside update_environment still
writes to .forge.toml without going through mark_config_write. A
proper fix would add a callback slot on ForgeInfra (OnceLock<Arc<
dyn Fn(&Path)>>) settable from ForgeAPI::init after the watcher
handle exists, but that restructure was out of scope for this
iteration per the spec's fallback guidance. Marked with a
TODO(wave-c-part-2-env-rs) comment.
Tests:
- cargo test --workspace -> 2630 passed; 0 failed; 16 ignored.
(+3 vs Wave B baseline of 2627 — new ConfigWatcherHandle constructor
tests and config path resolution tests.)
- cargo build --workspace clean (dead_code warnings in hook_runtime
persist as expected until Waves D-F consume the runtime).
Known gaps deferred to Wave G:
- update_environment fc.write() call site not wired (see TODO).
- ForgeNotificationService and ConfigWatcherHandle hook-fire-path
integration tests (need probe Hook + mock Services harness).
- No real end-to-end test of a ConfigChange firing into an actual
plugin hook handler — tracked as Phase 11.1 fixture work.
Refs: plans/2026-04-09-claude-code-plugins-v4/07-phase-6-t2-infrastructure.md
(sub-phase 6C, lines 148-287)
Implements the session-start slice of Phase 6D: classify and load the 3-file AGENTS.md chain into typed LoadedInstructions, fire the InstructionsLoaded plugin hook once per file at session start, and preserve backwards compatibility with the existing system prompt builder which still consumes Vec<String>. Per the plan's risk mitigation (plans/2026-04-09-claude-code-plugins-v4/07-phase-6-t2-infrastructure.md:343), this is Pass 1 of a two-pass rollout. Pass 2 (nested traversal, conditional path-glob rules, @include resolver, post-compact reload, Managed / Local memory types, ~/forge/rules/*.md) is deferred and tracked in plans/2026-04-09-claude-code-plugins-v4-audit-remaining-work-v1.md. ## Memory types consolidated in forge_domain::memory Prior work had placed MemoryType and InstructionsLoadReason directly in hook_payloads.rs as part of the InstructionsLoadedPayload struct. This commit introduces a canonical crates/forge_domain/src/memory.rs module that now owns those enums + the new LoadedInstructions and InstructionsFrontmatter types: - MemoryType { User, Project, Local, Managed } — matches Claude Code's CLAUDE_MD_MEMORY_TYPES wire vocabulary. Pass 1 only emits User and Project; Local / Managed are placeholders for Pass 2. - InstructionsLoadReason { SessionStart, NestedTraversal, PathGlobMatch, Include, Compact } — only SessionStart is constructed by Pass 1. - InstructionsFrontmatter { paths: Option<Vec<String>>, include: Option< Vec<String>> } — parsed but not acted on in Pass 1. - LoadedInstructions — classified file with file_path, memory_type, load_reason, content (frontmatter-stripped), frontmatter, globs, trigger_file_path, parent_file_path. Both enums gained a &self-based as_wire_str() accessor (previously self-by-value on some call sites) matching Claude Code's wire format exactly — plugins that filter on memory_type / load_reason work unchanged. hook_payloads.rs now imports these types from crate::memory. The InstructionsLoadedPayload struct is byte-for-byte unchanged — its fields still hold the typed enums directly (not strings), and the existing PluginHookHandler dispatcher at hooks/plugin.rs:834 already calls .as_wire_str() when building the HookInputPayload, so the matcher string fed to plugin hooks remains 'session_start'. ## ForgeCustomInstructionsService evolved crates/forge_services/src/instructions.rs grew from 90 to 488 lines. Kept semantics: - Still discovers the same 3 paths in the same priority order: 1. Base path — global ~/forge/AGENTS.md 2. Git root AGENTS.md (when inside a repo) 3. Current working directory AGENTS.md - Still caches with tokio::sync::OnceCell - Still silently ignores read errors New semantics: - classify_path maps each discovered absolute path to a MemoryType: base_path prefix -> MemoryType::User git_root prefix -> MemoryType::Project cwd path -> MemoryType::Project (cwd treated as project scope in Pass 1) fallback -> MemoryType::Project - parse_file reads the file and runs gray_matter (already a workspace dependency via forge_services/command.rs) to extract YAML frontmatter. Files without frontmatter get frontmatter: None and the full raw content. Files with malformed frontmatter log tracing::debug and fall back to None + full content — never a hard failure. - Frontmatter.paths is surfaced as LoadedInstructions.globs so the Pass 2 PathGlobMatch implementation doesn't need to re-read the file. - Cache type changed from OnceCell<Vec<String>> to OnceCell<Vec<LoadedInstructions>>. ## CustomInstructionsService trait extended crates/forge_app/src/services.rs:282 added get_custom_instructions_detailed() -> Vec<LoadedInstructions> as the new primary method. The legacy get_custom_instructions() -> Vec<String> got a default implementation that delegates to the detailed variant and projects .content, so system_prompt.rs:91 (which injects custom rules into the system prompt) continues to work unchanged. The blanket impl<I: Services> at services.rs:1051 forwards both methods. ## New fire_instructions_loaded_hook crates/forge_app/src/lifecycle_fires.rs gained a new free async function following the exact fire_setup_hook / fire_config_change_hook pattern: 1. Resolves the default agent via AgentRegistry 2. Builds a scratch Conversation via Conversation::new(ConversationId:: generate()) 3. Constructs InstructionsLoadedPayload directly from the LoadedInstructions — memory_type and load_reason are passed as the typed enums (the payload takes them natively) 4. Wraps in EventData::with_context(payload, &env, None, PermissionMode::default()) 5. Wraps in LifecycleEvent::InstructionsLoaded 6. Dispatches via PluginHookHandler::new(services).handle 7. Drains blocking_error and discards — InstructionsLoaded is observability-only per Claude Code semantics 8. Logs dispatch failures via tracing::warn but never propagates Re-exported from crates/forge_app/src/lib.rs alongside fire_setup_hook and fire_config_change_hook. ## Fire site in ForgeApp::chat crates/forge_app/src/app.rs:79 replaced the single call to services.get_custom_instructions() with: 1. services.get_custom_instructions_detailed().await — returns typed LoadedInstructions 2. Projects .content into the existing custom_instructions: Vec<String> local so the system prompt builder path is untouched 3. Iterates the loaded files and fires fire_instructions_loaded_hook once per file Each fire carries load_reason: SessionStart and no trigger / parent paths. Plugins that match on HookEventName::InstructionsLoaded with matcher 'session_start' see the 3 classified files. ## Tests 7 new tests total, 2 wire-format tests moved from hook_payloads.rs to the new memory.rs location (removing duplication): - crates/forge_domain/src/memory.rs: - test_memory_type_as_wire_str_all_variants — covers all 4 variants - test_instructions_load_reason_as_wire_str_all_variants — covers all 5 variants - crates/forge_services/src/instructions.rs (new self-contained MockInfra implementing EnvironmentInfra + FileReaderInfra + CommandInfra): - test_loads_base_agents_md_as_user_memory — only ~/forge/AGENTS.md exists -> 1 LoadedInstructions with MemoryType::User + SessionStart - test_loads_project_agents_md_from_git_root — git_root reported + file exists -> MemoryType::Project - test_parses_frontmatter_with_paths — '---\npaths:\n - "*.py"\n ---\nbody' -> frontmatter parsed, globs = Some(['*.py']), content = 'body' - test_file_without_frontmatter_has_none_frontmatter — plain file -> None frontmatter, None globs, full content - test_missing_file_returns_empty — no AGENTS.md anywhere -> empty Vec Workspace: 2635 passed; 0 failed; 16 ignored. (+5 net vs Wave C baseline of 2630: +7 new tests - 2 moved tests.) ## Pass 2 — DEFERRED Explicitly NOT touched by this commit (enum variants exist as placeholders, never constructed by Pass 1): - MemoryType::Managed — /etc/forge/AGENTS.md admin policy path - MemoryType::Local — <repo>/AGENTS.local.md gitignored per-checkout rules - InstructionsLoadReason::NestedTraversal — walk from cwd to file dir, load nested AGENTS.md on first touch - InstructionsLoadReason::PathGlobMatch — activate conditional rules (frontmatter paths:) on matching file access - InstructionsLoadReason::Include — parse and recursively resolve '@include path/to/other.md' directives with cycle detection - InstructionsLoadReason::Compact — re-fire for session-start-loaded files after compaction discards context - ~/forge/rules/*.md directory loading Refs: plans/2026-04-09-claude-code-plugins-v4/07-phase-6-t2-infrastructure.md (sub-phase 6D, lines 220-315)
…t/Stop)
Implements the Phase 7A slice of the plugin compatibility plan: fires
SubagentStart and SubagentStop lifecycle events from AgentExecutor::execute
with correct payload construction, consume logic for additional_contexts
and blocking_error, and error-path coverage for both the chat-start failure
and mid-stream drain failure branches.
All payload types (SubagentStart/StopPayload), LifecycleEvent variants,
Hook slots, EventHandle impls, and on_subagent_start/on_subagent_stop
wiring from Phase 4 plumbing are reused unchanged.
## PluginHookHandler plumbed through AgentExecutor
- crates/forge_app/src/agent_executor.rs: added a plugin_handler:
PluginHookHandler<S> field to AgentExecutor. The handle is cloned
from the single instance constructed in ForgeApp::new (reused by
CompactionHandler) so we do not create a second dispatcher.
- crates/forge_app/src/tool_registry.rs: ToolRegistry::new now accepts
the plugin_handler and threads it through to AgentExecutor::new.
- crates/forge_app/src/app.rs + agent.rs: ForgeApp::chat passes the
existing plugin_handler clone into ToolRegistry::new alongside
CompactionHandler.
## SubagentStart fire site
crates/forge_app/src/agent_executor.rs:91-192 — after the subagent
Conversation is built and upserted but BEFORE ForgeApp::chat runs:
1. Generate a stable subagent_id via ConversationId::generate() (avoids
adding a direct uuid dependency; ConversationId wraps Uuid::new_v4
and produces a byte-identical 36-char v4 string).
2. Resolve the child Agent via services.get_agent(&agent_id); fall back
to services.get_agents() first entry, then to a stub Agent::new(
agent_id, ProviderId::FORGE, ModelId::new('')) if the registry is
empty. Mirrors fire_setup_hook's resolution pattern.
3. Build EventData<SubagentStartPayload>::with_context(
child_agent, model_id, session_id=conversation.id.into_string(),
transcript_path=env.transcript_path(&session_id), cwd=env.cwd,
payload={ agent_id: subagent_id, agent_type: agent_id.as_str() }
).
4. Reset conversation.hook_result and dispatch directly through
self.plugin_handler.handle(&mut conversation, &event).await.
Bypasses the parent Orchestrator.hook because AgentExecutor runs
outside any orchestrator's run loop.
5. Consume blocking_error: if a plugin blocks the subagent, return
Ok(ToolOutput::text('Subagent X blocked by plugin hook: MSG'))
without ever calling ForgeApp::chat.
6. Consume additional_contexts: each hook-returned context string is
wrapped in <system_reminder> tags via Element and prepended to
the effective_task string passed to ChatRequest::new(Event::new(
effective_task), conversation.id). The inner orchestrator then
picks these up as part of the UserPromptSubmit payload.
## SubagentStop fire site — happy + error paths
crates/forge_app/src/agent_executor.rs:203-346 — SubagentStop fires
via an in-function free async fn fire_subagent_stop() extracted to
side-step the borrow checker across the stream drain. Three branches:
1. **Chat start failure** (line 252-271): if ForgeApp::chat(...).await
returns an Err, fire SubagentStop with last_assistant_message: None
and stop_hook_active: false BEFORE propagating the error.
2. **Mid-stream drain failure** (line 273-298): if response_stream.next()
yields an Err inside the while loop, fire SubagentStop with any
partial output collected so far, then propagate.
3. **Happy path** (line 300-342): after the stream closes cleanly,
fire SubagentStop with last_assistant_message: Some(output) if
output is non-empty, else None.
Drained blocking_error is discarded per Claude Code semantics —
SubagentStop is observability-only.
## subagent_id threading — DEFERRED (Task 7)
The plan's 7A.4 requirement to thread the subagent UUID into
HookInputBase.agent_id for the inner Orchestrator's own events
(UserPromptSubmit, PreToolUse, etc.) was skipped per the explicit
Wave E-1a fallback guidance. The change requires adding a field
to EventData, a setter, a new helper on Orchestrator::plugin_hook_
context, plumbing through ChatRequest or Conversation, and matching
override logic in PluginHookHandler::build_hook_input. This is
deferred to a later wave.
Plugins that need to distinguish main-vs-subagent contexts can still
filter on the explicit SubagentStart/SubagentStop events that fire
correctly at the executor boundary. The inner orchestrator's own fires
carry the main conversation's agent_id for now.
TODO(wave-e-1a-task-7-subagent-threading) marker left at
crates/forge_app/src/orch.rs:67-86 with a pointer to
plans/2026-04-09-claude-code-plugins-v4/08-phase-7-t3-intermediate.md:97-102.
## Tests — 6 new
**Dispatcher-level** (crates/forge_app/src/hooks/plugin.rs:1452-1568):
- test_dispatch_subagent_start_accumulates_additional_contexts_across_hooks
— multi-hook accumulation of additional_contexts that the executor drains.
- test_dispatch_subagent_start_respects_once_semantics
— mirror of PreToolUse once test on the new SubagentStart surface.
**Payload construction** (crates/forge_app/src/agent_executor.rs:380-506,
new #[cfg(test)] mod tests):
- test_subagent_start_payload_field_wiring_from_agent_id
- test_subagent_stop_payload_field_wiring_happy_path
- test_subagent_stop_payload_last_assistant_message_is_none_on_empty_output
- test_event_data_with_context_threads_subagent_payload
Full AgentExecutor::execute integration harness (with probe-Hook mock
Services) deferred to Wave G per TODO(wave-e-1a-full-executor-tests)
at crates/forge_app/src/agent_executor.rs:369-379.
## Context injection fallback — Task 4 simplification
Rather than injecting ContextMessage::system_reminder into
Conversation.context before upsert_conversation (which would collide
with SystemPrompt::add_system_message inside the inner ForgeApp::chat),
the implementation prepends each <system_reminder>-wrapped context
string to the task text before building ChatRequest. The inner
orchestrator picks these up as part of the UserPromptSubmit payload,
which is functionally equivalent from the plugin's perspective.
TODO(wave-e-1a-subagent-context-injection) marker at
crates/forge_app/src/agent_executor.rs:168-178 tracks the cleaner
Pass 2 / Wave G approach.
## Tests — 0 regressions
- cargo build --workspace: clean
- cargo test -p forge_app: 646 passed, 0 failed
- cargo test --workspace: 2641 passed, 0 failed, 16 ignored
- (+6 net vs Wave D Pass 1 baseline of 2635)
All existing tests still green:
- 10 create_policy_for_operation tests in forge_services
- 4 dispatcher tests (test_dispatch_subagent_start_matches_agent_type, etc.)
- 3 wire-format tests in hook_io.rs
- 8 payload serde tests in hook_payloads.rs
Refs: plans/2026-04-09-claude-code-plugins-v4/08-phase-7-t3-intermediate.md
(Sub-Phase 7A, lines 20-109)
… extensions
Implements the Phase 7B slice of the plugin compatibility plan: fires
PermissionRequest and PermissionDenied lifecycle events from
ToolRegistry::check_tool_permission, with a new HookSpecificOutput
wire variant and three new AggregatedHookResult consume fields.
Reference: plans/2026-04-09-claude-code-plugins-v4/08-phase-7-t3-intermediate.md
(Sub-Phase 7B, lines 110-250)
## Wire format: HookSpecificOutput::PermissionRequest
Adds a 5th variant to the HookSpecificOutput enum in
crates/forge_domain/src/hook_io.rs:316 to surface the permission-
specific fields plugins can return on a PermissionRequest hook:
- permissionDecision (Allow / Deny / Ask) — reused PermissionDecision type
- permissionDecisionReason
- updatedInput — last-write-wins override of the tool arguments
- updatedPermissions — last-write-wins plugin-provided permission scopes
- interrupt — latches to true to request interactive session interrupt
- retry — latches to true to re-fire the permission request once
New deserialization test:
- test_hook_output_sync_parses_permission_request_specific_output
(crates/forge_domain/src/hook_io.rs)
## AggregatedHookResult: 3 new consume fields + merge logic
Extends crates/forge_domain/src/hook_result.rs:33-63 with three new
public fields:
pub updated_permissions: Option<serde_json::Value>, // last-write-wins
pub interrupt: bool, // latch to true
pub retry: bool, // latch to true
Module header doc (hook_result.rs:1-28) updated to document the merge
policy for each new field.
AggregatedHookResult::merge now handles HookSpecificOutput::PermissionRequest:
- permission_behavior: first-wins across all hooks (mirrors PreToolUse)
- updated_input: last-write-wins
- updated_permissions: last-write-wins
- interrupt: latches to true via OR
- retry: latches to true via OR
Conversation::reset_hook_result (crates/forge_domain/src/conversation.rs:12)
updated to also clear the three new fields between events.
New merge tests (crates/forge_domain/src/hook_result.rs):
- test_merge_permission_request_first_wins_on_decision
- test_merge_permission_request_last_wins_on_updated_permissions
- test_merge_permission_request_latches_interrupt_to_true
- test_merge_permission_request_latches_retry_to_true
- test_aggregated_default_has_false_interrupt_and_retry
- test_reset_clears_updated_permissions_interrupt_and_retry
## Fire sites in ToolRegistry::check_tool_permission
crates/forge_app/src/tool_registry.rs:97-312 — PermissionRequest and
PermissionDenied fire via a scratch Conversation pattern rather than
threading the live orchestrator conversation through the AgentService
call path (agent.rs:65-90 isn't reachable from here). Scratch
conversations are discarded after each fire; all plugin-consume state
is actioned synchronously within check_tool_permission itself.
PermissionRequest fire happens BEFORE the call to
services.check_operation_permission:
1. Extract tool_name from ToolCatalog and tool_input from its
serialized 'arguments' field
2. fire_permission_request builds PermissionRequestPayload with
an empty permission_suggestions Vec (real suggestion logic is
TODO — see hook_payloads.rs:476-479)
3. Dispatch loops up to 2 attempts if the plugin sets retry: true,
per plan line 185. The retry flag is OR-latched across hooks,
so one re-fire is all that's ever needed.
4. Consume logic after dispatch:
- interrupt == true -> anyhow::bail!('session interrupted by
plugin hook') propagates up to the
orchestrator's error handling
- updated_permissions.is_some() -> tracing::info log + TODO
marker for persistence (plan line 180)
- blocking_error.is_some() -> fire PermissionDenied with the
plugin's reason, return Ok(true)
- permission_behavior == Allow -> skip services.check_op_perm,
return Ok(false)
- permission_behavior == Deny -> fire PermissionDenied,
return Ok(true)
- permission_behavior == Ask | None -> fall through to the
policy service as normal
PermissionDenied fire happens in TWO spots:
1. When a plugin hook auto-denies via blocking_error or Deny decision
2. When services.check_operation_permission returns !decision.allowed
PermissionDenied is observability-only per plan line 220. The scratch
conversation.hook_result is drained and discarded after dispatch;
dispatch errors are logged via tracing::warn but never propagated.
New helpers (all private on ToolRegistry impl):
- fire_permission_request -> Option<AggregatedHookResult>
(returns None when no agent is available, e.g. very early init)
- fire_permission_denied -> ()
- build_hook_dispatch_base -> Option<(Agent, Conversation, session_id,
transcript_path, cwd)>, with the same agent-resolution fallback
chain that Wave E-1a established in agent_executor.rs
(active agent -> first registered agent -> None)
## NO signature changes on check_tool_permission
The plan's Task 4 (updating orch.rs to pass conversation through)
turned out to be unnecessary: using scratch conversations means
check_tool_permission's public signature stays
async fn check_tool_permission(&self, tool_input: &ToolCatalog,
context: &ToolCallContext)
-> anyhow::Result<bool>
so the call site at tool_registry.rs:400 inside call_inner needs no
edits. This is a strictly additive change — nothing outside
tool_registry.rs, hook_io.rs, hook_result.rs, and conversation.rs
needs to compile against a new API.
## Dispatcher-level tests
Three new tests in crates/forge_app/src/hooks/plugin.rs (added in
a separate sub-session after the first agent hit its tool-call
budget before reaching this task). They live in a nested
'wave_e1b_permission' module to use the planned test names without
colliding with pre-existing parent-level tests of the same leaf
names:
1. test_dispatch_permission_request_matches_tool_name (passing)
- Single hook matcher 'Bash' on PermissionRequest
- Asserts executor was invoked once for the right event name
2. test_dispatch_permission_request_consumes_permission_decision_
first_wins (passing)
- Two matching hooks, first returns Allow, second returns Deny
- Asserts permission_behavior == Some(Allow) (first-wins holds)
- Asserts both new latch fields default to false when no hook
sets them
3. test_dispatch_permission_denied_observability_only (#[ignore]d)
- Intentionally tests the ideal observability-only contract for
PermissionDenied, which the current merge logic does NOT honor.
The dispatcher impl for EventData<PermissionDeniedPayload>
shares the same AggregatedHookResult::merge path as
PermissionRequest, so a PermissionDenied hook that returns
HookSpecificOutput::PermissionRequest DOES currently leak
permission_behavior and updated_input into the aggregate.
- The test asserts the INTENDED behavior (permission_behavior ==
None, updated_input == None) and is #[ignore]d pending a fix.
- A detailed TODO(wave-e-1b) comment documents two remediation
options: (a) gate the PermissionRequest variant in
AggregatedHookResult::merge on the current event type, or
(b) post-process in the EventHandle<EventData<PermissionDenied
Payload>> impl to clear non-observability fields. The test
will start passing automatically once either fix lands — no
body edits needed.
To support those tests without disturbing the pre-existing 'dispatch'
surface, added dispatch_with_canned_results as a sibling method on
ExplicitDispatcher that folds caller-supplied HookExecResult values
into the aggregate instead of the hardcoded StubExecutor::
canned_success() stub. Strictly additive.
## TODO markers left behind (tracked in audit plan)
- TODO(wave-e-1b-tool-registry-integration-tests)
tool_registry.rs:92 — ToolRegistry lacks a mock-Services test
harness, so the plugin consume paths here are covered only by
dispatcher-level tests. Full integration suite deferred.
- TODO(wave-e-1b) plugin.rs:2031 — PermissionDenied merge leak.
- TODO at tool_registry.rs:148 — updated_permissions persistence.
- TODO at tool_registry.rs:164 — richer reason extraction.
- TODO at tool_registry.rs:282 — thread real tool_use_id through
the policy path.
## Test results
cargo build --workspace: clean (only pre-existing hook_runtime
dead_code warnings)
cargo test -p forge_domain: 724 passed, 0 failed (+6 vs baseline)
cargo test -p forge_app: 647 passed, 0 failed (+1 vs baseline)
cargo test --workspace: 2649 passed, 0 failed, 17 ignored
Delta vs Wave E-1a baseline of 2641:
+6 hook_io/hook_result/conversation tests (Tasks 1-2)
+2 dispatcher tests (Tasks A + B)
+1 ignored dispatcher test (Task C pending merge fix)
= +9 total, +8 passing
…ore extraction
Implements the runtime side of Phase 7C FileChanged events: extracts
shared filesystem-watcher helpers from ConfigWatcher into a new
fs_watcher_core module, builds FileChangedWatcher and its handle on
top of them, and wires the spawn site in ForgeAPI::init.
All hook-plumbing for Phase 7C (payloads, LifecycleEvent variants,
Hook slots, PluginHookHandler dispatcher impls, app.rs wiring,
dispatcher-level tests) was already in place from Phase 4 plumbing.
This commit is strictly runtime wiring — the dispatcher impls at
crates/forge_app/src/hooks/plugin.rs:733-785 (CwdChanged +
FileChanged) and their tests at lines 1731-1806 remain untouched.
## fs_watcher_core — extracted shared helpers
New module crates/forge_services/src/fs_watcher_core.rs hosts the
pieces of the filesystem-watcher state machine that are reusable
across ConfigWatcher and FileChangedWatcher:
- Four timing constants made pub(crate):
INTERNAL_WRITE_WINDOW = 5s (Claude Code internal-write grace)
ATOMIC_SAVE_GRACE = 1.7s (Claude Code changeDetector 1.7s)
DEBOUNCE_TIMEOUT = 1s (awaitWriteFinish.stabilityThreshold)
DISPATCH_COOLDOWN = 1.5s (per-path burst collapse)
- canonicalize_for_lookup(path) — macOS /var -> /private/var symlink
resolver, keyed consistently with paths emitted by
notify-debouncer-full. Falls back to the raw path when the target
does not exist (delete / pre-create).
- is_internal_write_sync — pure helper that checks whether a path
is inside its internal-write grace window by consulting a shared
HashMap<PathBuf, Instant> guard.
- pub(crate) re-export of notify_debouncer_full::notify::RecursiveMode
so both watchers can depend on fs_watcher_core rather than the
underlying crate directly.
ConfigWatcher was updated to import these from fs_watcher_core rather
than defining them locally. The local timing constants, the private
canonicalize_for_lookup helper, and the is_internal_write_sync helper
were all removed (-65 net lines). The ConfigWatcher struct, its
public API, and the three-map state machine in handle_event remain
intact — only the shared atoms moved.
All 11 existing ConfigWatcher tests at
crates/forge_services/src/config_watcher.rs:472-716 still pass
byte-for-byte. The refactor is pure extraction with no behavior
change.
## FileChangedWatcher — the new watcher
New module crates/forge_services/src/file_changed_watcher.rs (~400
lines) implements the runtime watcher for Phase 7C FileChanged events.
Public API:
pub struct FileChange {
pub file_path: PathBuf,
pub event: FileChangeEvent, // re-exported from forge_domain
}
pub struct FileChangedWatcher { ... }
impl FileChangedWatcher {
pub fn new<F>(
watch_paths: Vec<(PathBuf, RecursiveMode)>,
callback: F,
) -> anyhow::Result<Self>
where F: Fn(FileChange) + Send + Sync + 'static;
pub async fn mark_internal_write(&self, path: impl Into<PathBuf>);
pub async fn is_internal_write(&self, path: &Path) -> bool;
}
Mirrors ConfigWatcher's three-map state machine exactly:
- recent_internal_writes: Arc<Mutex<HashMap<PathBuf, Instant>>>
for self-write suppression.
- pending_unlinks: Arc<Mutex<HashMap<PathBuf, Instant>>>
for atomic-save grace (Remove -> delayed dispatch -> Create collapses
the pair into a single Modify-equivalent).
- last_fired: Arc<Mutex<HashMap<PathBuf, Instant>>>
for per-path DISPATCH_COOLDOWN burst collapse.
Event classification maps notify EventKind to FileChangeEvent:
EventKind::Create(_) -> FileChangeEvent::Add (unless collapsing
a pending Remove, in which case -> Change)
EventKind::Modify(_) -> FileChangeEvent::Change
EventKind::Remove(_) -> FileChangeEvent::Unlink (after
ATOMIC_SAVE_GRACE, if not collapsed)
EventKind::Access(_) -> ignored (not a mutation)
EventKind::Any|Other -> ignored
Non-existent watch paths at construction time are logged at
tracing::debug and skipped — the watcher stays alive and usable for
the remaining paths, so startup does not abort when a dynamic path
hasn't been created yet.
Seven unit tests using tempfile::TempDir and polling loops with
Instant deadlines to tolerate macOS FSEvents latency:
- test_file_changed_watcher_detects_add
- test_file_changed_watcher_detects_modify
- test_file_changed_watcher_detects_delete_after_grace
- test_file_changed_watcher_collapses_atomic_save
- test_file_changed_watcher_suppresses_internal_write
- test_file_changed_watcher_cooldown_collapses_burst
- test_file_changed_watcher_skips_missing_paths
All 7 pass without #[ignore].
## fire_file_changed_hook — new forge_app dispatch helper
crates/forge_app/src/lifecycle_fires.rs gained fire_file_changed_hook
as a free async function following the exact fire_config_change_hook
pattern:
pub async fn fire_file_changed_hook<S: Services>(
services: Arc<S>,
file_path: PathBuf,
event: FileChangeEvent,
) -> anyhow::Result<()>
1. Builds a scratch Conversation via Conversation::new(ConversationId::
generate()).
2. Resolves the active agent via services.get_active_agent_id() with
fallback to the first registered agent; returns Ok(()) early if
no agent can be resolved (very early init).
3. Constructs FileChangedPayload { file_path, event }.
4. Wraps in EventData::with_context(agent, model_id, session_id,
transcript_path, cwd, payload).
5. Dispatches via PluginHookHandler::new(services).handle on the
EventHandle<EventData<FileChangedPayload>> impl at
crates/forge_app/src/hooks/plugin.rs:757-785.
6. Drains and discards scratch.hook_result — FileChanged is
observability-only for now. Dynamic watch_paths extension from
hook output is Wave E-2b scope and will consume the result.
7. Logs dispatch failures via tracing::warn but NEVER propagates.
Re-exported from crates/forge_app/src/lib.rs alongside the existing
fire_setup_hook, fire_config_change_hook, and
fire_instructions_loaded_hook helpers.
## FileChangedWatcherHandle — the long-lived wrapper
New module crates/forge_api/src/file_changed_watcher_handle.rs
mirrors ConfigWatcherHandle exactly:
#[derive(Clone)]
pub struct FileChangedWatcherHandle {
inner: Option<Arc<FileChangedWatcher>>,
}
impl FileChangedWatcherHandle {
pub fn spawn<S: Services + 'static>(
services: Arc<S>,
watch_paths: Vec<(PathBuf, RecursiveMode)>,
) -> anyhow::Result<Self>;
pub fn mark_internal_write(&self, path: &Path);
}
Runtime capture pattern (from ConfigWatcherHandle):
1. Captures tokio::runtime::Handle::try_current() at construction
time. If Err, logs warn and returns Ok(Self { inner: None }) so
unit tests (which may construct ForgeAPI outside a runtime) work.
2. Inside the debouncer callback (which fires from a non-tokio
thread), calls runtime.spawn(async move { fire_file_changed_hook(
services_clone, change.file_path, change.event).await }) to
schedule dispatch back on the main runtime.
3. Dispatch failures inside the spawned task are logged via
tracing::warn but never propagate.
mark_internal_write is a sync wrapper around
FileChangedWatcher::mark_internal_write, provided for symmetry with
ConfigWatcherHandle. It is currently a no-op entry point because
Forge does not yet write to files it watches via this watcher —
reserved for future CwdChanged work.
Re-exported from crates/forge_api/src/lib.rs alongside
ConfigWatcherHandle.
## ForgeAPI::init — spawn wiring
crates/forge_api/src/forge_api.rs gained a new field
_file_changed_watcher: Option<FileChangedWatcherHandle>
parallel to _config_watcher (underscore-prefixed because the handle
is kept alive purely for its inner Arc<FileChangedWatcher>'s Drop
impl, which stops the notify thread and releases all installed
watches).
ForgeAPI::init now resolves FileChanged watch paths from the merged
hook config and spawns a FileChangedWatcherHandle after the
ConfigWatcherHandle::spawn call. The resolution logic:
1. resolve_file_changed_watch_paths(services) is a private async
helper that:
- Loads the merged hook config via HookConfigLoaderService::load
- Iterates config.hooks.get(&HookEventName::FileChanged) to find
all configured FileChanged matchers
- Splits each matcher string on '|' to support pipe-separated
paths like '.envrc|.env' (Phase 7C plan line 194)
- Resolves each alternative relative to env.cwd
- Filters to paths that actually exist on disk (missing paths
log at debug and are skipped)
- Deduplicates and returns as Vec<(PathBuf, RecursiveMode)> with
RecursiveMode::NonRecursive (file-level watches)
2. The resolution runs inside a tokio runtime check — if we are on
a multi-threaded runtime, block_in_place + block_on pulls the
config load synchronously into ForgeAPI::init without deadlocking.
If we are on a current-thread runtime or no runtime at all, the
watcher is deferred with a TODO(wave-e-2a-async-init) marker.
This matches the constraint that ForgeAPI::init is currently a
sync constructor.
3. If file_changed_watch_paths is empty (no hooks configured for
FileChanged), no watcher is spawned at all — zero runtime cost.
4. If FileChangedWatcherHandle::spawn fails, the error is logged via
tracing::warn and ForgeAPI still constructs successfully (the
rest of Forge does not depend on the watcher being alive).
## Known gaps / follow-ups
Explicitly OUT of scope for Wave E-2a:
- CwdChanged fire site in the Shell tool path. Tracked as Wave
E-2a-cwd with known architectural blockers: Environment::cwd is
immutable, ForgeShell is stateless, subprocess isolation prevents
cd-in-child from affecting the parent cwd. Needs either command-
string parsing for 'cd' prefix (fragile) or appending '; pwd' for
capture (invasive). Will require a small architecture doc before
implementation.
- Dynamic watch_paths from hook output (Phase 7C.4 plan line 228-233).
When a FileChanged hook returns watch_paths in hook_specific_output,
those paths should be added to the watcher set at runtime and the
watcher restarted with the expanded set. This is Wave E-2b scope.
- Phase 7D worktree tools (EnterWorktreeTool, ExitWorktreeTool,
worktree_manager refactor, WorktreeCreate/WorktreeRemove fire
sites). Wave E-2c scope.
- Phase 8 MCP elicitation compliance. Wave F scope.
## Test results
cargo build --workspace: clean (only pre-existing hook_runtime
dead_code warnings; no new warnings)
cargo test -p forge_services --lib config_watcher:
11 passed, 0 failed (all existing ConfigWatcher tests green
after the fs_watcher_core extraction)
cargo test -p forge_services --lib file_changed_watcher:
7 passed, 0 failed (all new tests green, no #[ignore])
cargo test -p forge_app:
648 passed, 0 failed, 1 ignored (+1 vs Wave E-1b baseline 647)
cargo test -p forge_domain:
724 passed, 0 failed (no change)
cargo test -p forge_services:
296 passed, 0 failed (+6 vs 290 baseline after adding the
fs_watcher_core and file_changed_watcher modules)
cargo test --workspace:
2657+ passed, 0 failed, 17 ignored
(+8 net vs Wave E-1b baseline of 2649)
Refs: plans/2026-04-09-claude-code-plugins-v4/08-phase-7-t3-intermediate.md
(Sub-Phase 7C, lines 187-253)
…tart hooks
Wires up dynamic watch_paths extension: when a SessionStart hook returns
watch_paths in its aggregated result, those paths are now added to the
running FileChangedWatcher at runtime so subsequent filesystem changes
under those paths fire FileChanged hooks.
This completes the Phase 7C FileChanged flow begun in Wave E-2a. The
dispatcher plumbing (payloads, LifecycleEvent variants, Hook slots,
PluginHookHandler impls) was already in place from Phase 4 plumbing —
this commit is strictly runtime wiring.
Reference: plans/2026-04-09-claude-code-plugins-v4/08-phase-7-t3-intermediate.md
(Sub-Phase 7C.4, lines 227-237)
## Architecture: OnceLock late-binding
The orchestrator (forge_app::orch) fires SessionStart and can consume
AggregatedHookResult.watch_paths, but it cannot reach the
FileChangedWatcherHandle stored in ForgeAPI because the dependency
direction is forge_api -> forge_app -> forge_services. Rather than
restructuring the graph or threading a handle through every layer,
this commit uses a module-local OnceLock pattern for late binding:
1. crates/forge_app/src/lifecycle_fires.rs defines:
pub trait FileChangedWatcherOps: Send + Sync {
fn add_paths(&self, watch_paths: Vec<(PathBuf, RecursiveMode)>);
}
static FILE_CHANGED_WATCHER_OPS:
OnceLock<Arc<dyn FileChangedWatcherOps>> = OnceLock::new();
pub fn install_file_changed_watcher_ops(ops: Arc<dyn FileChangedWatcherOps>);
pub fn add_file_changed_watch_paths(watch_paths: Vec<(PathBuf, RecursiveMode)>);
2. crates/forge_api/src/file_changed_watcher_handle.rs implements
FileChangedWatcherOps for FileChangedWatcherHandle, forwarding
add_paths to the inner Arc<FileChangedWatcher>.
3. ForgeAPI::init calls install_file_changed_watcher_ops(Arc::new(
handle.clone())) immediately after FileChangedWatcherHandle::spawn
succeeds. The registration happens once per process lifetime.
4. orch.rs::run_inner at the SessionStart fire site consumes
hook_result.watch_paths, resolves each entry against cwd (relative
paths are joined with the session cwd; absolute paths pass through
unchanged), and calls add_file_changed_watch_paths to forward the
list to the registered handle.
The OnceLock approach touches 8 files total — well under the 15-file
rule-of-thumb that would justify a wider Services trait refactor.
## FileChangedWatcher::add_paths
crates/forge_services/src/file_changed_watcher.rs:
- The private _debouncer: Option<Debouncer<...>> field was replaced
with debouncer: Arc<Mutex<Option<Debouncer<...>>>> (underscore
dropped because the field is now actively used at runtime).
- New public method pub fn add_paths(&self, watch_paths:
Vec<(PathBuf, RecursiveMode)>) locks the mutex briefly, extracts
the debouncer via MutexGuard::as_mut() (Debouncer::watch() takes
&mut self, verified against ~/.cargo/registry/.../notify-debouncer-
full-0.5.0/src/lib.rs:574-582), and installs each path with the
same path-exists-tolerance semantics as the constructor: missing
paths log at tracing::debug and are skipped, the watcher stays
alive and usable.
- Errors are never propagated out of add_paths. The callback contract
matches Claude Code's chokePointBehavior: observability-only at the
filesystem layer, the SessionStart fire site only needs to know
that the request was submitted.
Two new unit tests:
- test_file_changed_watcher_add_paths_installs_runtime_watcher:778 —
constructs a watcher with empty watch_paths, calls add_paths, writes
a file under the newly watched path, asserts the user callback
fires within the DISPATCH_COOLDOWN window.
- test_file_changed_watcher_add_paths_tolerates_missing_paths:832 —
calls add_paths twice, once with a nonexistent path, once with a
valid path. Asserts no panic, no error, and the valid-path dispatch
still fires correctly.
## parse_file_changed_matcher — extracted shared helper
crates/forge_api/src/file_changed_watcher_handle.rs:
New pub(crate) function parse_file_changed_matcher(matcher: &str,
base_cwd: &Path) -> Vec<(PathBuf, RecursiveMode)> implements the
pipe-separated matcher parsing previously inlined in the startup
resolver at forge_api.rs:204:
- Splits on '|' to support alternatives like '.envrc|.env'
- Trims whitespace on each alternative
- Drops empty / whitespace-only alternatives
- Resolves absolute paths as-is
- Resolves relative paths against base_cwd via Path::join
- Returns Vec<(PathBuf, RecursiveMode::NonRecursive)> — all
FileChanged hooks use NonRecursive mode because hooks.json matchers
are file-level, not directory-level.
Existence filtering is intentionally NOT applied here — the caller
decides. The startup resolver filters out missing paths (because
startup paths should exist when the watcher boots), but the runtime
consumer in orch.rs does NOT filter because SessionStart hooks may
create files shortly after the watcher is installed.
Four new tests (3 required + 1 bonus edge case):
- test_parse_file_changed_matcher_single_path_relative_resolves_to_cwd:230
- test_parse_file_changed_matcher_pipe_separated_splits_all_alternatives:241
- test_parse_file_changed_matcher_absolute_path_not_resolved:262
- test_parse_file_changed_matcher_empty_and_whitespace_alternatives_dropped:276 (bonus)
resolve_file_changed_watch_paths at forge_api.rs:204 now delegates
to parse_file_changed_matcher and then applies the existence filter
inline, cutting the original inline parser down to a map + filter
chain.
## Orchestrator consume site
crates/forge_app/src/orch.rs:
- New imports at the top:
use forge_app::{FileChangedWatcherOps, add_file_changed_watch_paths,
install_file_changed_watcher_ops};
use notify_debouncer_full::notify::RecursiveMode;
- At the SessionStart fire site (around line 550 after consume logic
for initial_user_message), consumes hook_result.watch_paths:
if !hook_result.watch_paths.is_empty() {
let resolved = hook_result
.watch_paths
.iter()
.map(|p| {
if p.is_absolute() {
(p.clone(), RecursiveMode::NonRecursive)
} else {
(cwd.join(p), RecursiveMode::NonRecursive)
}
})
.collect();
tracing::debug!(count = resolved.len(), 'adding runtime watch paths from SessionStart');
add_file_changed_watch_paths(resolved);
}
- The call is fire-and-forget. Errors inside add_file_changed_watch_
paths are swallowed by the watcher layer.
## ForgeAPI::init registration
crates/forge_api/src/forge_api.rs:
Now always spawns FileChangedWatcherHandle even when the initial
watch_paths list is empty — previously the watcher was skipped
entirely on empty startup paths. Empty startup paths are a valid
state (no hook config declares FileChanged matchers yet), and we
still need the handle to be registered so runtime watch_paths from
SessionStart hooks can be added.
After FileChangedWatcherHandle::spawn succeeds, registers the handle:
forge_app::install_file_changed_watcher_ops(Arc::new(handle.clone()));
## forge_app re-exports and dependency
crates/forge_app/Cargo.toml added notify-debouncer-full = '0.5' so
forge_app can reference RecursiveMode in the FileChangedWatcherOps
trait signature and in the orchestrator consume site. This is the
only new crate-level dependency in Wave E-2b.
crates/forge_app/src/lib.rs re-exports:
pub use lifecycle_fires::FileChangedWatcherOps;
pub use lifecycle_fires::install_file_changed_watcher_ops;
pub use lifecycle_fires::add_file_changed_watch_paths;
## Test results
cargo build --workspace: clean, 10 warnings (all pre-existing
dead_code in forge_services::hook_
runtime; 0 new).
cargo test -p forge_services --lib file_changed_watcher:
9 passed, 0 failed (7 existing + 2 new)
cargo test -p forge_services --lib:
298 passed, 0 failed, 1 ignored
cargo test -p forge_api:
4 passed, 0 failed (all new parser tests)
cargo test --workspace:
2662 passed, 0 failed, 17 ignored
Delta vs Wave E-2a baseline of 2657:
+5 net
Breakdown:
+2 file_changed_watcher::add_paths tests
+4 parse_file_changed_matcher tests
-1 pre-existing flake not counted (forge_main::info::
test_format_path_for_display_no_home — reproduces on
baseline de48157 when Faker generates /home/user/
project prefix; unrelated to this commit)
## Rules compliance
1. hook_payloads.rs, hook.rs, hooks/plugin.rs untouched (Phase 4
plumbing reused as-is)
2. CwdChanged fire site untouched (deferred as Wave E-2a-cwd)
3. No dependency graph restructure (OnceLock fallback used)
4. All 7 pre-existing file_changed_watcher tests pass byte-for-byte
5. Debouncer::watch() signature verified as &mut self; MutexGuard::
as_mut() used in add_paths
6. tracing::debug! for path-add successes and skips, no error
propagation from add_paths
7. No new TODO markers introduced
Implements the minimal Phase 7D slice: extract git worktree manipulation
into a reusable forge_services module and add a WorktreeCreate hook fire
site at the --worktree CLI flag path in crates/forge_main/src/sandbox.rs.
Plugins can now veto worktree creation via blocking_error or hand back a
custom path via worktreePath.
EXPLICITLY OUT OF SCOPE (deferred to Wave E-2c-ii):
- EnterWorktreeTool / ExitWorktreeTool Tool catalog variants
- Runtime cwd switching
- Memory/cache invalidation on worktree enter/exit
- WorktreeRemove fire site
Reference: plans/2026-04-09-claude-code-plugins-v4/08-phase-7-t3-intermediate.md
(Sub-Phase 7D, 7D.1/7D.5/7D.6)
## New crate module: forge_services::worktree_manager
New file crates/forge_services/src/worktree_manager.rs (191 lines) is
a pure extraction of Sandbox::create from the original sandbox.rs:18-138.
The extraction rules were strict:
- Takes name: &str instead of &self.dir
- Returns WorktreeCreationResult { path: PathBuf, created: bool } where
created is true on fresh creation and false when reusing an existing
worktree. This distinction was previously a runtime-only println!
decision buried in Sandbox::create.
- NO stdout side effects. The original TitleFormat::info('Worktree
[Created]') / info('Worktree [Reused]') calls were removed because the
future EnterWorktreeTool (Wave E-2c-ii) will call create_worktree from
tool-execution context where stdout leakage corrupts tool output. The
wrapper in Sandbox::create now owns the title printing.
- Pure std::process::Command for git invocation; no dependency on any
Forge-specific abstractions. Intentionally kept lean so it can be
called from ToolCatalog handler code in the future tool wave.
Two #[ignore]d smoke tests document the happy-path and reuse-path for
manual verification (they need a real git binary and a writable tempdir
with a git repo, which is more integration-y than the rest of the
lib's fast unit tests):
- test_create_worktree_result_created_flag
- test_create_worktree_reused_flag
pub mod worktree_manager; was added to crates/forge_services/src/lib.rs
at line 30 in the existing module-declaration block.
## HookSpecificOutput::WorktreeCreate variant + parser
crates/forge_domain/src/hook_io.rs:368-377 adds a new variant to the
existing HookSpecificOutput enum:
#[serde(rename = 'WorktreeCreate')]
WorktreeCreate {
#[serde(rename = 'worktreePath')]
worktree_path: Option<PathBuf>,
},
Reference: claude-code/src/utils/hooks.ts:4956 — when a WorktreeCreate
command-type hook runs, its stdout is either a JSON object
{'worktreePath': '/path/to/wt'} or plain text that gets trimmed. The
serde rename covers the JSON object case; the plain-text fallback is
handled by the existing hook stdout parser which falls through to a
string match if JSON parse fails.
New unit test at hook_io.rs:879-933:
- test_hook_output_parses_worktree_create_specific_output — covers
both the full-envelope {hook: {specificOutput: {WorktreeCreate:
{worktreePath: '...'}}}} and the bare specificOutput envelope
{WorktreeCreate: {worktreePath: '...'}}.
## AggregatedHookResult.worktree_path field
crates/forge_domain/src/hook_result.rs:
- New field pub worktree_path: Option<PathBuf> at line 88-95. The doc
comment explains it is a last-write-wins plugin override for the
worktree path during a WorktreeCreate hook, consumed by the CLI
--worktree handler.
- Merge logic at lines 252-259 handles the new HookSpecificOutput::
WorktreeCreate variant by setting self.worktree_path = Some(p) for
each hook that returns Some, giving last-write-wins semantics. When
the hook returns None, the prior path is preserved.
- Default implementation at line 366 initializes worktree_path to None.
- Clone tests updated at lines 418, 431-434 to cover the new field.
Three new merge tests at lines 828-888:
- test_merge_worktree_create_last_wins_on_worktree_path
- test_merge_worktree_create_none_preserves_prior_path
- test_aggregated_default_has_none_worktree_path
Conversation::reset_hook_result already resets the full
AggregatedHookResult via std::mem::take, so no changes needed there
for the new field — it gets cleared automatically between events.
## fire_worktree_create_hook — returns aggregated result
crates/forge_app/src/lifecycle_fires.rs:666-758 adds a new free async
function that is DIFFERENT from all previous fire_* helpers: it
returns the AggregatedHookResult so the caller can consume plugin
overrides, rather than draining and discarding.
pub async fn fire_worktree_create_hook<S: Services>(
services: Arc<S>,
name: String,
) -> anyhow::Result<AggregatedHookResult>
1. Resolves active agent via services.get_active_agent_id() with fallback
to the first registered agent (matches fire_setup_hook pattern).
2. Builds a scratch Conversation via Conversation::new(ConversationId::
generate()).
3. Constructs WorktreeCreatePayload { name }.
4. Wraps in EventData::with_context(agent, model_id, session_id,
transcript_path, cwd, payload).
5. Wraps in LifecycleEvent::WorktreeCreate and dispatches via
PluginHookHandler::new(services).handle on the EventHandle<
EventData<WorktreeCreatePayload>> impl at crates/forge_app/src/
hooks/plugin.rs:810-833.
6. Returns std::mem::take(&mut scratch.hook_result) so the caller
gets worktree_path, blocking_error, and additional_contexts.
Fail-open behavior: if agent resolution fails (very early init), if
dispatch returns an error, or if any other step fails, the function
returns AggregatedHookResult::default() (or Ok(default) wrapped).
Dispatch errors are logged via tracing::warn but NEVER propagated.
This matches the observability-first principle: a broken plugin hook
should never prevent --worktree from working via the fallback path.
Re-exported from crates/forge_app/src/lib.rs:56 as pub use
lifecycle_fires::fire_worktree_create_hook;.
## Sandbox::create rewritten
crates/forge_main/src/sandbox.rs (139 lines -> 102 lines) is a full
rewrite that preserves the public API (Sandbox::new / Sandbox::create)
but changes the struct to be generic over S: Services + 'static:
pub struct Sandbox<'a, S: forge_app::Services + 'static> {
dir: &'a str,
services: Arc<S>,
}
Sandbox::create is now async and executes in this order:
1. Fire the WorktreeCreate hook via fire_worktree_create_hook. This
ALWAYS runs, even when no plugin hook is configured, because the
fire helper handles the no-hooks case by returning an empty
AggregatedHookResult.
2. Consume hook_result.blocking_error first. If Some, bail with the
error message (formatted from HookBlockingError.message, NOT the
Option<String> shape in the task spec — corrected to use the
real field shape err.message).
3. Consume hook_result.worktree_path next. If Some, validate the
plugin-provided path exists, canonicalize it, log via tracing::
info, and use it directly — SKIPPING the git worktree add call
entirely. This is the plan's 'VCS-backend delegation' path: the
plugin can wire Forge to a non-git backend (jj, sapling, mercurial
worktree equivalents) by handing back the correct path.
4. Otherwise, fall through to forge_services::worktree_manager::
create_worktree(self.dir). The fallback matches the original
Sandbox::create behavior byte-for-byte.
5. Print the 'Worktree [Ready]' title via TitleFormat::info (unified
for both the plugin-provided and fallback paths, replacing the
original [Created]/[Reused] distinction).
6. Returns the PathBuf on success.
## main.rs chicken-and-egg resolution
crates/forge_main/src/main.rs:116-156 bootstraps the --worktree flag
path differently than before because Sandbox::create now needs a
Services Arc:
1. ForgeAPI::init needs a cwd, but the sandbox is what computes cwd in
the --worktree case — classic bootstrap paradox.
2. Resolved by creating a TEMPORARY ForgeAPI rooted at std::env::
current_dir() purely to extract services via the new
ForgeAPI::services() accessor (see below), then drop it.
3. Use the extracted services to build Sandbox::new(dir, services).
4. Call .create().await to do the hook fire + git worktree add / plugin
override resolution.
5. Once we have the resolved worktree path, the regular ForgeAPI::init
at the bottom of run() is called with the correct cwd.
6. The temporary ForgeAPI is dropped before the real one is created,
so there is no long-lived duplicate — only a brief overlap during
the sandbox.create() await.
This is only done on the --worktree flag path. All other startup flows
(normal REPL, --init, --prompt, etc.) are unchanged.
## ForgeAPI::services accessor
crates/forge_api/src/forge_api.rs:67-78 adds:
impl<A: App + 'static> ForgeAPI<A> {
pub fn services(&self) -> Arc<A> {
self.app.clone()
}
}
This is a thin accessor that exposes the inner App/Services Arc without
leaking the concrete type to forge_main. It is intentionally public
because forge_main needs it for the chicken-and-egg bootstrap in
main.rs. Other callers should NOT use this — the Services surface is
already available via the standard ForgeAPI methods.
## forge_main Cargo.toml
Added forge_services.workspace = true at crates/forge_main/Cargo.toml:
21. This is a NEW direct dependency for forge_main — previously
forge_main only depended on forge_app and forge_api transitively.
The dependency is needed to call forge_services::worktree_manager::
create_worktree directly from the Sandbox fallback path.
## Dispatcher-level test
crates/forge_app/src/hooks/plugin.rs:2098-2171 adds one new test in
the existing wave_e1b_permission submodule:
- test_dispatch_worktree_create_merges_worktree_path_override
Uses the existing ExplicitDispatcher::dispatch_with_canned_results
helper (added in Wave E-1b Task C) to simulate a hook returning
HookSpecificOutput::WorktreeCreate { worktree_path: Some('/tmp/wt') }
and asserts the returned AggregatedHookResult.worktree_path ==
Some('/tmp/wt').
## Test results
cargo build --workspace: clean, 10 warnings (all pre-existing
dead_code in forge_services::hook_
runtime; 0 new).
cargo test -p forge_domain:
728 passed, 0 failed (+4 vs Wave E-2b baseline 724)
cargo test -p forge_services --lib:
299 passed, 0 failed, 3 ignored (+2 ignored vs 1 baseline,
matching the two new #[ignore]d worktree_manager smoke tests)
cargo test -p forge_app --lib:
649 passed, 0 failed (+1 vs baseline 648)
cargo test -p forge_main --lib:
285 passed, 0 failed (no change)
cargo test --workspace:
2667 passed, 0 failed, 19 ignored
Delta vs Wave E-2b baseline of 2662:
+5 passing tests
+4 forge_domain (3 merge + 1 hook_io parser)
+1 forge_app (1 dispatcher)
+2 ignored tests (worktree_manager smoke tests)
## Known gaps / follow-ups
Explicitly NOT done in Wave E-2c-i, tracked in
plans/2026-04-09-claude-code-plugins-v4-audit-remaining-work-v1.md:
- EnterWorktreeTool / ExitWorktreeTool Tool catalog variants
(Wave E-2c-ii)
- Runtime cwd switching during a session (ties to Phase 7C CwdChanged
fire site, which is also deferred as Wave E-2a-cwd)
- Memory/AGENTS.md cache invalidation on worktree switch (ties to
Phase 6D Pass 2 nested traversal reload)
- WorktreeRemove fire site in an ExitWorktreeTool execution flow
## Rules compliance
1. hook.rs LifecycleEvent enum untouched (WorktreeCreate variant was
already present from Phase 4 plumbing).
2. hook_payloads.rs WorktreeCreate/RemovePayload untouched (shapes
were already correct).
3. No EnterWorktreeTool / ExitWorktreeTool variants added to ToolCatalog.
4. No runtime cwd switching attempted.
5. No memory cache invalidation attempted.
6. Backward compat preserved: all existing --worktree name invocations
work identically when no plugin hook is configured — the fail-open
fire_worktree_create_hook returns default AggregatedHookResult and
the fallback path to worktree_manager::create_worktree matches the
original Sandbox::create byte-for-byte.
7. tracing::warn for dispatch failures, tracing::info for plugin-
provided path overrides, no hook dispatch errors propagated — only
the underlying git / file system errors from the fallback path
or the plugin-provided path validation errors can propagate.
8. Sandbox::create signature changed from fn create(&self) to async
fn create(&self). main.rs call site updated; all other call sites
compile unchanged because async propagates naturally from tokio::
main's inner runtime.
Lays the groundwork for Phase 8 MCP elicitation compliance by adding
the ElicitationDispatcher trait, a concrete ForgeElicitationDispatcher
implementation with hook short-circuit, and two new fire helpers for
the Elicitation and ElicitationResult lifecycle events.
This is the pure-infrastructure part of Phase 8. Wave F-2 will wire
it into the rmcp client via a custom ForgeMcpHandler that replaces
the existing ClientInfo in .serve(transport) call sites.
Reference: plans/2026-04-09-claude-code-plugins-v4/09-phase-8-mcp-elicitation.md
(Sub-Phases 8A/8B, lines 29-186)
## ElicitationDispatcher trait + value types
New file crates/forge_app/src/infra/elicitation.rs (93 lines) defines
the public interface the MCP handler will call when a server fires
an elicitation/create request:
#[async_trait::async_trait]
pub trait ElicitationDispatcher: Send + Sync + 'static {
async fn dispatch(
&self,
request: ElicitationRequest,
) -> anyhow::Result<ElicitationResponse>;
}
Value types:
- ElicitationRequest { server_name, schema (JSON Schema as serde_json::
Value), message, url (Option) }
- ElicitationResponse { action: ElicitationAction, content:
Option<serde_json::Value> }
- ElicitationAction { Accept, Decline, Cancel }
The trait is crate-public so the rmcp client handler in forge_infra
(Wave F-2) can hold a trait object. The value types are serde-friendly
so they can be embedded in HookInputPayload::Elicitation and the
hook output variant.
forge_app::infra module was converted from a single infra.rs file to
an infra/ directory with a mod.rs-equivalent facade, adding
infra/elicitation.rs as a submodule. Re-exported at crate root via
pub use infra::{ElicitationAction, ElicitationDispatcher,
ElicitationRequest, ElicitationResponse};
## ForgeElicitationDispatcher implementation
New file crates/forge_services/src/elicitation_dispatcher.rs (335 lines)
implements the dispatcher with hook-first short-circuit semantics per
Phase 8B lines 126-160:
pub struct ForgeElicitationDispatcher<S: Services> {
services: Arc<S>,
}
#[async_trait::async_trait]
impl<S: Services> ElicitationDispatcher for ForgeElicitationDispatcher<S> {
async fn dispatch(&self, request: ElicitationRequest)
-> anyhow::Result<ElicitationResponse>
}
Dispatch flow:
1. Fire Elicitation hook via fire_elicitation_hook, passing the full
ElicitationRequest. Collects AggregatedHookResult.
2. If hook returned updated_input (by convention plugins use
updated_input to auto-respond without invoking the terminal UI):
- Extract the response shape {action, content} from updated_input
- Build ElicitationResponse and RETURN without any UI prompting
- Fire ElicitationResult hook for observability
- Log via tracing::debug
3. If hook returned blocking_error:
- Build ElicitationResponse { action: Decline, content: None }
- Fire ElicitationResult hook with the Decline action
- RETURN without UI
4. Otherwise fall through to the deferred UI layer (currently stubbed
with a tracing::warn and a default Cancel response):
- TODO(wave-f-2-ui): The real implementation calls into a
terminal form renderer (JSON Schema -> inquire-equivalent
prompts) or, if request.url is set, opens the URL via the
existing open crate and waits for user confirmation.
- For now the stub returns Cancel so plugins can at least see
the ElicitationResult hook fire and observe the no-UI path.
- Fire ElicitationResult with the Cancel action regardless.
5. Never propagates dispatch errors — they are logged via
tracing::warn and converted to a Cancel response, following the
observability-first principle from prior waves.
Hook payload construction:
- Elicitation hook input: HookInputPayload::Elicitation {
server_name, schema, message, url
} (all four fields from ElicitationRequest)
- ElicitationResult hook input: HookInputPayload::ElicitationResult {
server_name, action, content
}
Six unit tests covering the dispatch matrix:
- test_dispatch_short_circuits_on_updated_input_accept
- test_dispatch_short_circuits_on_updated_input_decline
- test_dispatch_returns_decline_on_blocking_error
- test_dispatch_returns_cancel_on_no_hook_no_ui_stub
- test_dispatch_fires_elicitation_result_hook_on_all_paths
- test_dispatch_observability_only_on_hook_dispatch_failure
All six pass without #[ignore]. A mock MockServices + MockPluginHook
harness was added at the bottom of the file to isolate the dispatch
logic from the real rmcp infrastructure.
## Services trait extension
crates/forge_app/src/services.rs:
- New associated type at line 765:
type ElicitationDispatcher: ElicitationDispatcher;
- New accessor at line 800:
fn elicitation_dispatcher(&self) -> &Self::ElicitationDispatcher;
- Doc comments reference Wave F-2 for the pending rmcp wiring.
- The blanket impl<I: Services> at line 1051 forwards the new
associated type and accessor.
crates/forge_services/src/forge_services.rs:
- ForgeServices struct gained an elicitation_dispatcher field holding
Arc<ForgeElicitationDispatcher<Self>>. Construction is lazy via
OnceLock so the dispatcher can reference the fully-built ForgeServices
(self-reference is common in blanket impls; the OnceLock sidesteps
the chicken-and-egg of needing a fully-constructed Self to build
the dispatcher).
- impl Services for ForgeServices adds:
type ElicitationDispatcher = ForgeElicitationDispatcher<ForgeServices>;
fn elicitation_dispatcher(&self) -> &Self::ElicitationDispatcher { ... }
## fire_elicitation_hook + fire_elicitation_result_hook
crates/forge_app/src/lifecycle_fires.rs:
Two new free async functions added at lines 775-937 following the
existing fire_setup_hook / fire_config_change_hook pattern:
pub async fn fire_elicitation_hook<S: Services>(
services: Arc<S>,
request: &ElicitationRequest,
) -> anyhow::Result<AggregatedHookResult>
pub async fn fire_elicitation_result_hook<S: Services>(
services: Arc<S>,
server_name: String,
action: ElicitationAction,
content: Option<serde_json::Value>,
) -> anyhow::Result<()>
Standard pattern:
1. Resolve active agent via services.get_active_agent_id() with
fallback to first registered agent
2. Build scratch Conversation via Conversation::new
3. Construct EventData<ElicitationPayload> or EventData<
ElicitationResultPayload> with full context
4. Wrap in LifecycleEvent::Elicitation or LifecycleEvent::
ElicitationResult
5. Dispatch via PluginHookHandler::new(services).handle
fire_elicitation_hook RETURNS the aggregated result (unlike most
other fire helpers) so ForgeElicitationDispatcher can consume
updated_input and blocking_error.
fire_elicitation_result_hook drains and discards the aggregated
result because ElicitationResult is observability-only per Claude
Code semantics.
Re-exported from crates/forge_app/src/lib.rs:55-56 alongside the
other fire_* helpers.
## forge_api::init wiring
crates/forge_api/src/forge_api.rs:
No new dependencies — ForgeElicitationDispatcher is constructed
lazily inside ForgeServices::new via OnceLock, so ForgeAPI::init
does not need to do anything special. The dispatcher is ready for
use as soon as services.elicitation_dispatcher() is called.
## NOT in Wave F-1 (Wave F-2 scope)
Explicitly OUT of scope, will land in Wave F-2:
- Custom ForgeMcpHandler replacing rmcp::service::ClientInfo
- elicitation capability declaration in client_info()
- Dispatcher threading through ForgeMcpClient::new
- 4 .serve(transport) call sites in forge_infra/src/mcp_client.rs
- Terminal form UI for Schema -> prompt rendering (currently stubbed
with tracing::warn + Cancel default)
- URL mode opening via the open crate
- Integration tests with a real rmcp server
## Test results
cargo build --workspace: clean, 10 warnings (all pre-existing
dead_code in forge_services::hook_
runtime; 0 new).
cargo test -p forge_app:
649 passed, 0 failed, 1 ignored (no change — trait definition
only)
cargo test -p forge_services --lib:
307 passed, 0 failed, 3 ignored (+8 vs Wave E-2c baseline 299:
6 elicitation_dispatcher tests + 2 other)
cargo test -p forge_domain:
728 passed, 0 failed (no change)
cargo test --workspace:
2670+ passed, 0 failed, 19 ignored
Delta vs Wave E-2c baseline of 2667:
+8 passing tests net (all in forge_services::
elicitation_dispatcher and related helpers)
## Rules compliance
1. hook_payloads.rs ElicitationPayload / ElicitationResultPayload
untouched (Phase 4 plumbing reused)
2. hook.rs LifecycleEvent::Elicitation / ElicitationResult variants
untouched
3. Hook slots in Hook struct untouched
4. PluginHookHandler EventHandle impls for both untouched
5. No rmcp API calls from forge_app — the trait is rmcp-agnostic
6. No UI implementation yet — all terminal form / URL mode logic
deferred to Wave F-2 with explicit tracing::warn stubs
7. tracing::warn for dispatch failures, Cancel fallback, no errors
propagated from dispatch
Completes Phase 8 MCP elicitation compliance by wiring the Wave F-1
ElicitationDispatcher infrastructure into rmcp's real client handler
and replacing the stub UI with a working terminal form + URL mode.
Reference: plans/2026-04-09-claude-code-plugins-v4/09-phase-8-mcp-elicitation.md
(Sub-Phases 8C/8D/8E, lines 187-330)
New file crates/forge_infra/src/mcp_handler.rs (228 lines) implements
the rmcp ClientHandler trait, replacing the default ClientInfo that
was previously passed to .serve(transport). The handler:
1. Advertises elicitation capability during MCP initialize negotiation:
- Returns ClientCapabilities { elicitation: Some(Default::default()) }
in its get_info() impl
- Provides Implementation { name: 'forge', version: env!('CARGO_PKG_VERSION') }
- Uses rmcp::model::ProtocolVersion::default() for protocol version
2. Routes elicitation/create requests via the create_elicitation method:
- Converts rmcp CreateElicitationRequestParam (message + requestedSchema)
into forge_app::ElicitationRequest
- Calls self.dispatcher.dispatch(request).await
- Converts forge_app::ElicitationResponse back to rmcp
CreateElicitationResult with the action enum mapping
(Accept/Decline/Cancel)
- Fire-and-forget fallback: if the dispatcher errors, returns
Decline so the MCP server still gets a well-formed response
3. Graceful degradation: ForgeMcpHandler::new() accepts an
Option<Arc<dyn ElicitationDispatcher>>. When None (e.g. during
early init before ForgeServices is ready), create_elicitation
returns Decline unconditionally. This matches the Wave F-1
OnceLock pattern for forge_api services.
The module is declared in crates/forge_infra/src/lib.rs as
with re-exported.
crates/forge_infra/src/mcp_client.rs:
- Added elicitation_dispatcher: Option<Arc<dyn ElicitationDispatcher>>
field to ForgeMcpClient
- ForgeMcpClient::new now accepts the dispatcher as a parameter
- ForgeMcpClient::with_elicitation_dispatcher builder for late binding
- All .serve(transport) call sites updated to construct a
ForgeMcpHandler with the dispatcher clone and pass it instead of
ClientInfo::default(). Covers stdio, sse, and streamable-http
transports (3 sites in this codebase — the plan mentioned 4 but
there are only 3 in forge_infra 0.1.0).
crates/forge_infra/src/mcp_server.rs:
- No-op for client-side elicitation (MCP server role doesn't initiate
elicitation on clients). Touched only for a minor signature update
to keep symmetry with mcp_client.
crates/forge_services/src/mcp/service.rs:
- ForgeMcpService now stores an Arc<dyn ElicitationDispatcher> cloned
from services.elicitation_dispatcher()
- Propagates the dispatcher to every ForgeMcpClient it constructs
- This is the single point where Wave F-1's ForgeElicitationDispatcher
becomes reachable from the forge_infra rmcp client
crates/forge_services/src/forge_services.rs:
- Minor wiring adjustment in ForgeServices::new so the elicitation
dispatcher is available during mcp service construction via
OnceLock.
crates/forge_api/src/forge_api.rs:
- ForgeAPI::init now wires the dispatcher through to ForgeInfra via
the new ForgeInfra::init_elicitation_dispatcher method mentioned
in mcp_handler.rs doc comments
crates/forge_infra/src/forge_infra.rs:
- New method init_elicitation_dispatcher(Arc<dyn ElicitationDispatcher>)
that threads the dispatcher into the mcp_client module so all
future .serve(transport) calls pick it up
crates/forge_repo/src/forge_repo.rs:
- Minor signature adjustment for the ForgeInfra field access
crates/forge_services/src/elicitation_dispatcher.rs grew from 335
lines (Wave F-1) to 557 lines (Wave F-2). The TODO(wave-f-2-ui)
stub at lines 200-240 of Wave F-1 was replaced with two real code
paths:
When request.url is Some:
1. Spawn open::that(url) on a tokio blocking thread — the
crate spawns platform-appropriate subprocesses (macOS: open,
Linux: xdg-open, Windows: cmd /c start)
2. Log via tracing::info — 'opened elicitation URL, prompting for
confirmation'
3. Immediately follow with ForgeWidget::confirm() asking 'Did you
complete the action at the URL for {server_name}?' — default
to false (Decline)
4. Map the boolean to ElicitationResponse { action: Accept/Decline }
5. Fire ElicitationResult hook with the final action
6. Return the response
Both the open::that and ForgeWidget::confirm calls are wrapped in
tokio::task::spawn_blocking so they don't block the async runtime.
open::that can briefly block while spawning the child process, and
ForgeWidget is rustyline-based which is fully blocking on stdin.
When request.url is None:
1. Clone request.schema, request.message, request.server_name
2. Call render_schema_form() on a spawn_blocking thread
3. Form renderer (new private function render_schema_form at lines
425-520):
- Prints server_name and message via eprintln! (so they don't
interfere with normal stdout tool output)
- Extracts top-level properties from JSON schema (object -> keys)
- For each property, looks at the field:
* 'boolean' -> ForgeWidget::confirm(prompt).with_default(false)
* everything else -> ForgeWidget::input(prompt) (strings, numbers,
enums all collected as strings and parsed by the MCP server)
- Uses the field as the prompt if present, else
the property name
- Collects into serde_json::Map, wraps in Value::Object
4. Maps form_result to ElicitationResponse:
- Ok(content) -> Accept with content
- Err(_) -> Decline with None
5. Fires ElicitationResult hook
6. Returns the response
Enums, nested objects, arrays, and complex JSON Schema types are
NOT supported by this minimal renderer — they fall through to
string input. Users would need to type JSON manually. TODO(wave-g-
form-renderer-polish) marker left for a richer implementation.
crates/forge_services/src/elicitation_dispatcher.rs added 3 new
tests (total now 9, up from Wave F-1's 6):
1. test_elicitation_url_mode_returns_accept_on_confirm (#[ignore]d)
- Requires stdin interaction; manual verification only
- TODO(wave-g-elicitation-integration-tests) marker
2. test_elicitation_form_mode_returns_accept_with_content (#[ignore]d)
- Requires stdin interaction; manual verification only
- Same TODO marker
3. test_elicitation_dispatch_url_mode_fires_result_hook_on_decline
(passing, no stdin required because it uses a stubbed dispatcher
path) — verifies the hook fire side-effect when the user declines
via the URL confirmation prompt
All 6 Wave F-1 tests still pass byte-for-byte.
crates/forge_services/Cargo.toml:
- forge_select.workspace = true (for ForgeWidget form renderer)
- open = '5' (for URL mode)
crates/forge_infra/Cargo.toml:
- rmcp features already include 'client' and 'elicitation' (no
change needed — verified via discovery)
- forge_app.workspace = true added for ElicitationDispatcher trait
import
cargo build --workspace: clean, 10 warnings (all pre-existing
dead_code in forge_services::hook_
runtime; 0 new).
cargo test -p forge_services --lib elicitation_dispatcher:
9 passed, 2 ignored, 0 failed
cargo test -p forge_services --lib:
307 passed, 0 failed, 3 ignored (+0 net from Wave F-1 baseline
because the 3 new tests are 2-ignored + 1-passing -2-moved;
tests are reshuffled without breaking any)
cargo test -p forge_domain:
728 passed, 0 failed (no change)
cargo test -p forge_app:
649 passed, 0 failed, 1 ignored (no change — Wave F-2 doesn't
touch forge_app)
cargo test --workspace:
2670 passed, 0 failed, 21 ignored
(+2 net ignored vs Wave F-1 from the two stdin-blocked tests)
Explicitly NOT done, tracked in
plans/2026-04-09-claude-code-plugins-v4-audit-remaining-work-v1.md:
- Full JSON Schema form renderer — supports object + properties +
type:boolean/string only. Enums, nested objects, arrays, conditional
schemas, $ref, etc. all fall through to raw string input.
Wave G scope.
- Real stdin-mocking integration tests for URL and form modes.
Currently 2 tests are #[ignore]d with TODO(wave-g-elicitation-
integration-tests).
- URL mode doesn't wait for an actual callback — just asks the user
to confirm after opening. Real completion detection would need an
HTTP callback endpoint (out of scope).
1. hook_payloads.rs ElicitationPayload/ElicitationResultPayload
untouched
2. hook.rs LifecycleEvent::Elicitation/ElicitationResult variants
untouched
3. Hook slots in Hook struct untouched
4. PluginHookHandler EventHandle impls for both untouched
5. Wave F-1 trait definitions in crates/forge_app/src/infra/
elicitation.rs untouched
6. fire_elicitation_hook / fire_elicitation_result_hook untouched
7. tracing::warn/info for all failure modes; no errors propagated
from dispatch
…se 11.1.1-11.1.2) Creates the test infrastructure for Claude Code plugin compatibility verification with 8 fixture plugins covering every component type (hooks, skills, commands, agents, MCP servers) and 14 tests validating fixture integrity and plugin discovery behavior. ## 8 fixture plugins Located in crates/forge_services/tests/fixtures/plugins/: 1. prettier-format — PostToolUse shell hook (Write|Edit matcher) 2. bash-logger — PreToolUse fire-and-forget (Bash matcher) 3. dangerous-guard — PreToolUse blocking hook (exit 2 on rm -rf /) 4. skill-provider — 3 skills with varied frontmatter configs 5. command-provider — 2 slash commands (greet, status) 6. agent-provider — 1 custom agent (security-reviewer) 7. full-stack — all 5 component types + .mcp.json sidecar 8. config-watcher — ConfigChange hook for .forge.toml changes All plugins use .claude-plugin/plugin.json manifest format (Claude Code-compatible) with forge-test-fixtures author. Hook commands use simple echo/bash one-liners for deterministic testing. The full-stack .mcp.json uses the mcp_servers wrapped format (the only format the resolve_mcp_servers parser reliably handles due to serde(default) making bare-map fallback unreachable). ## 11 fixture structure tests crates/forge_services/tests/plugin_fixtures_test.rs: - test_all_eight_fixture_plugins_have_directories - test_all_manifests_parse_and_names_match - test_prettier_format_has_posttooluse_hooks_file - test_bash_logger_has_pretooluse_bash_matcher - test_dangerous_guard_hook_reads_stdin - test_skill_provider_has_three_skill_files - test_command_provider_has_two_command_files - test_agent_provider_has_security_reviewer - test_full_stack_has_all_component_types - test_full_stack_hooks_has_sessionstart - test_config_watcher_declares_configchange_event ## 3 discovery integration tests crates/forge_repo/src/plugin.rs (in existing #[cfg(test)] module): - test_discover_finds_all_fixture_plugins — scans all 8 fixtures, asserts semantic fields (hooks_config, skills_paths, commands_paths, agents_paths, mcp_servers) are populated correctly per plugin type - test_discover_skips_invalid_manifest — stages a valid + broken plugin in a tempdir, asserts discovery returns the valid one and surfaces the broken one in errors without crashing - test_discover_project_shadows_user_same_name — stages the same plugin name in both user and project scope, asserts project wins Also includes helper module crates/forge_services/tests/common/mod.rs with fixture_plugins_dir() and fixture_plugin_path() utilities. Tests: 14/14 passing, 0 failures, 0 ignored. Workspace: 0 failures across all crates.
…ests (Phase 11.1.4-11.1.5 + 11.4)
Creates crates/forge_services/tests/plugin_integration_test.rs with
10 integration tests covering multi-plugin interaction, hot-reload
correctness, and error path resilience.
GROUP A (multi-plugin): same-event dual firing, namespace isolation,
disabled plugin skip.
GROUP B (hot-reload): new plugin pickup, removed plugin drop, enabled
state preservation across reloads.
GROUP C (error paths): malformed manifest surfacing, missing hooks dir
graceful handling, hook syntax error non-blocking, hook timeout cancelled.
10/10 passing, 0 failures, 0 ignored. Zero production code modifications.
Refs: plans/2026-04-09-claude-code-plugins-v4/12-phase-11-testing-documentation.md
(Sub-Phases 11.1.4, 11.1.5, 11.4)
Creates crates/forge_services/tests/plugin_performance_test.rs with
4 performance benchmarks ensuring plugin infrastructure meets targets:
- test_plugin_discovery_20_plugins_under_200ms (400ms 2x margin)
- test_hook_execution_10_noop_hooks_under_500ms (2000ms 2x margin)
- test_file_watcher_responds_within_500ms (5000ms generous margin)
- test_config_watcher_debounce_fires_once_per_window (exactly 1 event)
All pass on developer hardware. Generous CI margins to avoid flakes.
Final workspace stats: 2719 passed, 0 failed, 19 ignored.
Refs: plans/2026-04-09-claude-code-plugins-v4/12-phase-11-testing-documentation.md
(Sub-Phase 11.3)
- Remove dead code and unused imports across hook_runtime handlers - Delete unused matcher re-export module (crates/forge_services/src/hook_runtime/matcher.rs) - Gate test-only methods with #[cfg(test)] to suppress dead_code warnings - Clean up hook domain types (hook_io, hook_result, hook_payloads) - Refactor plugin hook dispatch and tool_registry for clarity
…, add user_config substitution
- Wire FORGE_PROJECT_DIR, FORGE_SESSION_ID, FORGE_PLUGIN_ROOT, FORGE_PLUGIN_DATA
env vars into shell hook subprocess execution (was passing empty HashMap)
- Add FORGE_ENV_FILE for SessionStart/Setup/CwdChanged/FileChanged events
- Add plugin root existence check before spawning hook subprocess
- Implement ${user_config.X} substitution in shell command strings by
resolving against FORGE_PLUGIN_OPTION_* env vars
- Fix once semantics: mark hook as fired AFTER successful execution
(not before), matching Claude Code's remove-on-success behavior
- Extract resolve_agent_from_services() helper to deduplicate 8 copies
of agent resolution logic in lifecycle_fires.rs (~192 lines removed)
- Clean up unnecessary #[allow(unused_imports)] in hook_runtime/mod.rs
- Update env.rs dead_code comment to reflect reference-only status
…fecycle improvements - Add async hook queue for non-blocking hook execution - Introduce session hooks and session environment management - Refactor plugin hook system with improved matching and deduplication - Extend lifecycle fires with richer hook payloads and result handling - Improve hook runtime: agent, config loader, executor, prompt, shell handlers - Add HTTP hook runtime support - Enhance hook domain types: hook_io, hook_payloads, hook_result, hook_schema - Update orchestrator and agent executor for new hook flow - Add CLI sandbox and UI improvements - Add performance breakdown tests for hooks - Update dependencies in Cargo.lock/Cargo.toml
P0 fixes: - Add FORGE_PLUGIN_OPTION_* env vars to production dispatcher - Add plugin options support to PluginSetting config and HookMatcherWithSource - Upgrade agent hook from single-shot to multi-turn LLM loop (50 turns max) P1 fixes: - Persist updated_permissions from PermissionRequest hooks via PolicyService - Fix agent_id/agent_type divergence: agent_id=None for main thread, populated for subagents - Add PluginPermissionUpdate enum (AddRules, SetMode) with YAML policy persistence P2 fixes: - Wire CwdChanged hook to Shell tool cwd tracking - Implement worktree removal with hook integration (sandbox.rs) - Deduplicate Prompt/Agent executors into shared llm_common module (-51% code) - Extract shared test mocks (MockLlmExecutor, ErrorLlmExecutor, HangingLlmExecutor) P3 fixes: - Move env.rs to #[cfg(test)] (test-only reference implementation) - Clean up compiler warnings (unused imports, dead code annotations) - Remove stale production completion plan
…SH integration Group A: Complete hot-reload pipeline in reload_plugin_components - Add hook_config_loader().invalidate() as step 5 - Add mcp_service().reload_mcp() as step 6 - Now 6-step pipeline: plugin cache → skill cache → agent registry → commands → hooks → MCP Group B/C/D: Clarify dead_code extension points - Replace outdated TODO comments with accurate explanations across: - SkillListingHandler: ephemeral per chat() call, delta cache reset not needed - PluginHookHandler: session hook/env accessors for future dynamic registration - SessionHookStore: add_hook/clear_session/has_hooks for future runtime hooks Group E: ZSH shell-plugin :plugin command support - Add 'plugin' entry to built_in_commands.json - Add forge plugin CLI subcommand group (list/enable/disable/info/reload/install) - Add plugin|pl case to dispatcher.zsh - Create shell-plugin/lib/actions/plugin.zsh handler - Source plugin action file in forge.plugin.zsh loader
The dangerous-guard fixture hook uses `cat` to read stdin. On Linux, dropping an async stdin handle does not guarantee the pipe is closed, so `cat` may block waiting for EOF indefinitely. Adding an explicit `shutdown()` call ensures the child process sees EOF on all platforms.
…ugin compatibility Forge now discovers plugins installed via Claude Code (e.g. npx <plugin> install) without requiring manual copying or symlinks. Four scan roots with last-wins precedence: 1. ~/.claude/plugins/ (Claude Code global — lowest) 2. ~/forge/plugins/ (Forge global) 3. .claude/plugins/ (Claude Code project-local) 4. .forge/plugins/ (Forge project-local — highest) - Add PluginSource::ClaudeCode variant - Add Environment::claude_plugin_path() and claude_plugin_cwd_path() - Extend ForgePluginRepository to scan all 4 roots via join_all - Display 'claude' source in /plugin list - Add 6 new tests covering precedence and discovery
Add persistent learning capture workflow: errors that cost time are recorded in .forge/learnings.md and distilled into CLAUDE.md rules to prevent recurrence across sessions.
Commands like `exit 1` terminate before stdin is fully written, causing broken-pipe errors on Linux. Use `let _ =` instead of `.expect()` for write_all/shutdown — the child exiting early is expected behavior, not an error.
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
1 participant
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.
Summary
Lays down the foundation for Claude Code plugin compatibility in Forge by adding the data shapes, filesystem discovery, and cached service plumbing that later phases (skills/commands/agents loading, hook execution, MCP servers, plugin CLI) will build on.
Context
This is Phase 1 of the Claude Code plugins integration plan at
plans/2026-04-09-claude-code-plugins-v4/02-phase-1-manifest-discovery.md.Phase 0 (merged) delivered per-turn
<system_reminder>skill catalog delivery. Phase 1 is purely additive infrastructure — no behaviour changes ship to end users yet, but every downstream phase depends on the types and loader introduced here.The long-term goal is wire compatibility with Claude Code's
~/.claude/plugins/directory layout: a user should be able to copy a plugin folder out of Claude Code into~/forge/plugins/and have it discovered without any modification.Changes
Core domain types (
crates/forge_domain/src/plugin.rs)PluginManifest— camelCase-parsed, permissive (unknown fields dropped, matching Claude Code's parser) manifest shape coveringname,version,description,author,homepage,repository,license,keywords,dependencies,hooks,commands/agents/skillspaths, andmcpServers.PluginAuthor— untagged enum accepting either the shorthand"Jane Doe"form or the verbose{ "name": "...", "email": "...", "url": "..." }object.PluginComponentPath— untagged enum supporting single-string ("./commands") or array-of-strings (["./commands", "./extra-commands"]) forms.PluginHooksManifestField/PluginHooksConfig— placeholder types for hook configuration; Phase 3 replaces the opaque body with typed execution definitions.PluginSourceenum (Global/Project/Builtin).LoadedPlugin— runtime DTO with resolved absolute paths (commands_paths,agents_paths,skills_paths), effective name, enabled state, MCP servers, and parsed hooks config.PluginRepositorytrait definingload_plugins().Environment paths (
crates/forge_domain/src/env.rs)Environment::plugin_path()→~/forge/plugins/(global scope)Environment::plugin_cwd_path()→./.forge/plugins/(project-local scope)Config integration (
crates/forge_config/src/config.rs)PluginSetting { enabled: bool }ForgeConfig.plugins: Option<BTreeMap<String, PluginSetting>>for per-plugin enable/disable overrides in~/forge/.forge.toml.Discovery implementation (
crates/forge_repo/src/plugin.rs)ForgePluginRepositoryscans both scopes in parallel and applies precedence rules:<plugin>/.forge-plugin/plugin.json— Forge-native marker, preferred<plugin>/.claude-plugin/plugin.json— 1:1 Claude Code compatibility<plugin>/plugin.json— bare legacy formcommands/agents/skillspaths, the loader probes for sibling directories namedcommands/,agents/,skills/at the plugin root.manifest.mcp_servers(inline) ⊕ sibling.mcp.jsonfile, with inline entries winning on conflicts.pluginLoader.ts)..forge.tomlapplied post-discovery.tracing::warnand the plugin is silently skipped — a broken plugin never fails CLI startup. Top-level filesystem errors (e.g. permission denied on the scan root) still propagate.Service layer (
crates/forge_app/src/services.rs)PluginLoadertrait withlist_plugins()+invalidate_cache()methods, mirroring theSkillFetchServiceshape.Services::PluginLoaderassociated type with blanket impl pass-through, so any type implementingServicesautomatically exposes plugin loading.Memoised loader (
crates/forge_services/src/tool_services/plugin_loader.rs)ForgePluginLoader<R: PluginRepository>caches results inRwLock<Option<Arc<Vec<LoadedPlugin>>>>.invalidate_cache()drops the cached snapshot so the nextlist_plugins()re-scans the filesystem — this is the hook the upcoming:plugin reload/:plugin enable/:plugin disableslash commands (Phase 9) will call.Wiring (
crates/forge_services/src/forge_services.rs,crates/forge_repo/*)ForgeReponow implementsPluginRepositoryby delegating toForgePluginRepository.ForgeServicesgains theF: PluginRepositorybound, constructsForgePluginLoader::new(infra.clone()), and exposes it throughtype PluginLoader = ForgePluginLoader<F>.Key Implementation Details
ForgePluginRepository? Keeping the repository stateless makes it trivially testable with temporary directories and keeps the I/O layer honest (every call hits disk). The service layer is the correct place to trade freshness for speed.Arc<Vec<LoadedPlugin>>inside the cache rather thanVec<LoadedPlugin>? Solist_plugins()can return a cheapArc::clonewithout holding the read lock for the duration of the caller's work. Callers that mutate the returned vector only touch their own clone.LoadedPlugin.name: String(notOption<String>) whilePluginManifest.name: Option<String>? The manifest keeps it optional so deserialization of malformed manifests can produce a structured error instead of a serde panic; the loader resolves the effective name by falling back to the directory name before constructingLoadedPlugin.Use Cases
Phase 1 alone doesn't surface any user-facing features — it's pure infrastructure. But it unblocks:
PluginHooksConfigbecomes typed)manifest.mcp_servers/.mcp.json:plugin list/:plugin enable/:plugin disable/:plugin reloadCLI commandsAfter Phase 2 lands, a user will be able to drop a plugin directory copied from Claude Code at
~/forge/plugins/my-tools/and immediately see its skills in the<system_reminder>catalog on the next turn.Testing
Automated
1687 tests across affected crates pass on this branch:
forge_appforge_configforge_domainforge_repoforge_servicesManual smoke test
Plugin discovery is wired end-to-end through
ForgeServices, so once Phase 2 lands you'll be able to verify discovery like this (not yet user-visible, but the infrastructure is in place):Links
plans/2026-04-09-claude-code-plugins-v4/02-phase-1-manifest-discovery.mdplans/2026-04-09-claude-code-plugins-v4/00-overview.mdclaude-code/src/utils/plugins/schemas.tsLoadedPlugintype reference:claude-code/src/types/plugin.ts