fix: support composable client hooks - #786
Conversation
Greptile SummaryThis PR expands lifecycle hook support and carries client terminal identity into shared-server hook execution. The main changes are:
Confidence Score: 4/5Mostly safe, with one contained bug in initial connection terminal-env metadata. Hook parsing and execution changes are localized and covered by tests. The first Files Needing Attention:
What T-Rex did
|
| Filename | Overview |
|---|---|
| crates/jcode-app-core/src/server/client_lifecycle.rs | Adds task-local client terminal env scoping around session lifecycle and turn-processing paths; initial connection metadata still stores an empty terminal env. |
| crates/jcode-base/src/hooks.rs | Introduces ordered hook command execution and task-local client terminal env application for hook processes. |
| crates/jcode-config-types/src/lib.rs | Adds HookCommands with scalar/array serde support and converts hook config fields to use it. |
| crates/jcode-terminal-launch/src/lib.rs | Adds helper to replace stale terminal identity env vars and export client-prefixed aliases. |
| crates/jcode-base/src/config/env_overrides.rs | Updates lifecycle hook env overrides to accept scalar commands or TOML-style command arrays. |
| crates/jcode-base/src/config_tests.rs | Covers hook command array parsing and env override behavior for lifecycle hooks. |
| crates/jcode-base/src/config.rs | Re-exports the new HookCommands config type alongside existing config types. |
| crates/jcode-base/src/terminal_launch.rs | Re-exports apply_client_terminal_env for hook process setup. |
Sequence Diagram
sequenceDiagram
participant Client
participant Server as handle_client
participant Agent
participant Hooks
participant Cmd as Hook command(s)
Client->>Server: Subscribe(working_dir, terminal_env)
Server->>Server: "active_terminal_env = terminal_env"
Server->>Hooks: with_client_terminal_env(active_terminal_env)
Hooks->>Agent: create/resume/clear/process session
Agent->>Hooks: dispatch_observer / run_pre_tool_gate
Hooks->>Cmd: "apply_client_terminal_env + JCODE_HOOK_* env"
loop configured commands
Hooks->>Cmd: spawn observer or await gate
end
Cmd-->>Hooks: exit status (gate only)
Hooks-->>Agent: allow/block decision
Agent-->>Server: events/result
Server-->>Client: ServerEvent stream
Comments Outside Diff (1)
-
crates/jcode-app-core/src/server/client_lifecycle.rs, line 540 (link)Store initial terminal env
active_terminal_envis populated from the firstSubscribe, but this connection record still storesterminal_env: Vec::new()until a laterSubscriberequest updates it. On the normal initial-subscribe path, shared-server spawn/focus code that readsClientConnectionInfo.terminal_envkeeps seeing an empty snapshot, so hooks launched for that connection lose the initiating pane identity.Prompt To Fix With AI
This is a comment left during a code review. Path: crates/jcode-app-core/src/server/client_lifecycle.rs Line: 540 Comment: **Store initial terminal env** `active_terminal_env` is populated from the first `Subscribe`, but this connection record still stores `terminal_env: Vec::new()` until a later `Subscribe` request updates it. On the normal initial-subscribe path, shared-server spawn/focus code that reads `ClientConnectionInfo.terminal_env` keeps seeing an empty snapshot, so hooks launched for that connection lose the initiating pane identity. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Prompt To Fix All With AI
### Issue 1
crates/jcode-app-core/src/server/client_lifecycle.rs:540
**Store initial terminal env**
`active_terminal_env` is populated from the first `Subscribe`, but this connection record still stores `terminal_env: Vec::new()` until a later `Subscribe` request updates it. On the normal initial-subscribe path, shared-server spawn/focus code that reads `ClientConnectionInfo.terminal_env` keeps seeing an empty snapshot, so hooks launched for that connection lose the initiating pane identity.
```suggestion
terminal_env: active_terminal_env.clone(),
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix: support composable client hooks (fi..." | Re-trigger Greptile
Summary
Root cause
Hook configuration stored only one string, forcing integrations to replace or wrap an existing command. Hook processes also inherited the long-lived server's startup environment rather than the client snapshot already sent in
Subscribe, so later panes could be attributed to the wrong terminal.Validation
cargo check -p jcode-config-types -p jcode-terminal-launch -p jcode-base -p jcode-app-corecargo test -p jcode-base hooks -- --nocapturecargo test -p jcode-terminal-launch client_terminal_env_replaces_inherited_identity_and_exports_aliasescargo test -p jcode-base hooks_config_defaults_and_parses_from_tomlcargo test -p jcode-base test_env_override_lifecycle_hookscargo fmt --allAddresses #759 without closing it from triage.
--- — Jcode agent (automated triage), on behalf of @1jehuang