Add context command support for chat turns#1124
Conversation
Greptile SummaryThis PR introduces
Confidence Score: 4/5Safe to merge; the feature is well-tested and fails gracefully, but there are a few quality issues worth addressing before the command sees heavy production use. The core logic is correct and all error paths return None rather than blocking chat. Three issues stand out: (1) a successful command that produces no output logs a warn!, which will mislead operators who conditionally emit context; (2) resolve_project_context and run_context_command are awaited serially even though they are fully independent, so every chat turn pays both latencies in sequence; (3) working_dir is passed as None at every call site, meaning the command always runs from the server process cwd — operators writing relative-path scripts will be surprised. None of these change the correctness of the feature, but the warn! spam and serial latency are likely to surface quickly in production. crates/common/src/context_command.rs (warn on empty output) and crates/chat/src/service/types.rs (sequential context resolution)
|
| Filename | Overview |
|---|---|
| crates/common/src/context_command.rs | New module: runs a shell command via sh/cmd, captures stdout, enforces a 30 s timeout. Empty-but-successful output triggers warn! (should be debug!); working_dir is always None at every call site. |
| crates/chat/src/service/types.rs | Adds resolve_turn_context and merge_context_sections; the two async sources are awaited sequentially rather than via tokio::join!, adding avoidable latency on every chat turn. |
| crates/gateway/src/external_agents.rs | Calls run_context_command and passes the result into context_from_history_with_project_context; straightforward and well-tested. |
| crates/external-agents/src/runtimes/codex.rs | CodexAppServerSession now forwards context snapshot via build_process_input and wraps the full turn in a top-level timeout; also adds /params/delta as a new extract_message fallback. |
| crates/config/src/schema/chat.rs | Adds context_command: Option field to ChatConfig with serde default; clean change. |
Sequence Diagram
sequenceDiagram
participant Client
participant ChatService
participant ProjectContext as Project Context (DB/FS)
participant ContextCmd as context_command (shell)
participant LLM as LLM / External Agent
Client->>ChatService: send_turn(prompt)
ChatService->>ProjectContext: resolve_project_context()
ProjectContext-->>ChatService: "Option<String>"
ChatService->>ContextCmd: run_context_command() [30s timeout]
ContextCmd-->>ChatService: "Option<String>"
ChatService->>ChatService: merge_context_sections()
ChatService->>LLM: prompt + merged context
LLM-->>ChatService: response
ChatService-->>Client: response
Reviews (1): Last reviewed commit: "Add context command support for chat tur..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if text.trim().is_empty() { | ||
| warn!("context_command produced no output"); | ||
| None |
There was a problem hiding this comment.
warn on empty output will spam logs for conditional commands
A context command that deliberately produces no output under some conditions (e.g., it only emits context when the system is degraded, or it gates on a feature flag) will fire this warn! on every "quiet" turn. Because the command exit code is 0, this is not a failure — the operator chose to emit nothing. Treating an empty-but-successful result as a warning will mislead on-call engineers and pollute logs. debug! (or silently returning None) is the right level here.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| /// Resolve all dynamic prompt context for a turn. | ||
| pub(in crate::service) async fn resolve_turn_context( | ||
| &self, | ||
| session_key: &str, | ||
| conn_id: Option<&str>, | ||
| ) -> Option<String> { | ||
| let project_context = self.resolve_project_context(session_key, conn_id).await; | ||
| let command_context = moltis_common::context_command::run_context_command( | ||
| self.config.chat.context_command.as_deref(), | ||
| None, | ||
| ) | ||
| .await; | ||
| merge_context_sections(project_context, command_context) | ||
| } |
There was a problem hiding this comment.
resolve_project_context (a DB/filesystem call) and run_context_command (a subprocess that may run up to 30 s) are awaited one after the other. Their results are independent, so they can be fetched in parallel with tokio::join!, cutting every chat-turn's latency overhead by the slower of the two rather than their sum. With a 30-second command timeout the serial worst case is notably worse.
| pub async fn run_context_command( | ||
| command: Option<&str>, | ||
| working_dir: Option<&Path>, | ||
| ) -> Option<String> { | ||
| let command = command.map(str::trim).filter(|value| !value.is_empty())?; | ||
|
|
||
| let mut cmd = shell_command(command); | ||
| if let Some(dir) = working_dir { | ||
| cmd.current_dir(dir); | ||
| } |
There was a problem hiding this comment.
working_dir is always None at every call site
Both resolve_turn_context (chat service) and the gateway ExternalAgentChatService call run_context_command(..., None). The working_dir parameter therefore has no effect today, but operators configuring commands like ./scripts/gen-context.sh or cat .context will get surprising failures because the script runs from the server process's cwd, not the project directory. Consider passing the active project's working directory at call sites, or at least documenting that the command runs in the server process cwd.
- Downgrade empty-output log from warn! to debug! — operators may conditionally emit context, so silence is not an error. - Resolve project context and context command concurrently via tokio::join! instead of sequentially, avoiding double latency on every chat turn. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
Adds an optional
chat.context_commandthat runs before each chat turn and appends stdout to the prompt context. This lets deployments inject generated runtime context without manually pasting it into each session.The implementation covers:
Validation
cargo fmt --all -- --checkcargo test -p moltis-common context_command -- --nocapturecargo test -p moltis-config context_command -- --nocapturecargo test -p moltis-chat merge_context_sections -- --nocapturecargo test -p moltis-gateway context_from_history_includes_project_context -- --nocapturecargo test -p moltis-external-agents runtimes::codex::tests -- --nocapture