feat: system config self-awareness, prompt inspect API, release version check#374
Merged
feat: system config self-awareness, prompt inspect API, release version check#374
Conversation
Add SystemInfo struct that gives channels awareness of their own configuration (models, context window, capabilities, MCP servers, warmup state, cron jobs, version, deployment). Move worker time context from task message into system prompt template. Add prompt inspect API endpoint (GET /api/channels/inspect). Add verify-version job to release workflow. Remove dead worker_time_context template.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughReplaces the template-based worker time context with a runtime-built SystemInfo and a live status_text threaded into prompt rendering and worker spawn flows; adds per-channel prompt snapshot storage, persistence APIs, UI, and snapshot capture controls; removes TemporalContext worker preamble helpers. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| State(state): State<Arc<ApiState>>, | ||
| Query(query): Query<PromptInspectQuery>, | ||
| ) -> Result<Json<serde_json::Value>, StatusCode> { | ||
| let channel_state = { |
Contributor
There was a problem hiding this comment.
This endpoint returns the full system prompt + conversation history; in deployments where auth_token is unset, /api/* is currently public. I’d strongly consider hard-gating this route behind auth to avoid accidental prompt/history exposure.
Suggested change
| let channel_state = { | |
| if state.auth_token.is_none() { | |
| return Err(StatusCode::NOT_FOUND); | |
| } | |
src/api/channels.rs
Outdated
|
|
||
| // ── History ── | ||
| let history = channel_state.history.read().await; | ||
| let history_json = serde_json::to_value(&*history).unwrap_or(serde_json::Value::Null); |
Contributor
There was a problem hiding this comment.
Minor: serde_json::to_value failing here probably indicates a bug worth surfacing as a 500 rather than silently returning null.
Suggested change
| let history_json = serde_json::to_value(&*history).unwrap_or(serde_json::Value::Null); | |
| let history_json = serde_json::to_value(&*history).map_err(|error| { | |
| tracing::warn!(%error, "failed to serialize channel history for inspect"); | |
| StatusCode::INTERNAL_SERVER_ERROR | |
| })?; |
src/api/channels.rs
Outdated
| let response = serde_json::json!({ | ||
| "channel_id": query.channel_id, | ||
| "system_prompt": system_prompt, | ||
| "system_prompt_chars": system_prompt.len(), |
Contributor
There was a problem hiding this comment.
system_prompt.len() is bytes; if the field is meant to be character count (name suggests so), use .chars().count() (or rename the field to *_bytes).
Suggested change
| "system_prompt_chars": system_prompt.len(), | |
| "system_prompt_chars": system_prompt.chars().count(), |
| /// | ||
| /// Workers get a lighter version: just time + model + context window. | ||
| /// No warmup, no cron, no bulletin — they don't need it. | ||
| pub fn render_for_worker(&self, current_time_line: &str) -> String { |
Contributor
There was a problem hiding this comment.
Small mismatch between the doc comment and output: render_for_worker doesn’t currently include the context window.
Suggested change
| pub fn render_for_worker(&self, current_time_line: &str) -> String { | |
| pub fn render_for_worker(&self, current_time_line: &str) -> String { | |
| let mut output = String::from("## System\n"); | |
| output.push_str(&format!("Time: {current_time_line}\n")); | |
| output.push_str(&format!("Model: {}\n", self.worker_model)); | |
| output.push_str(&format!("Context: {} tokens\n", self.context_window)); | |
| output | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/context_dump.rs (1)
365-375:⚠️ Potential issue | 🟡 MinorRender the worker dump with the same
status_textproduction uses.Production callers in
src/tools/spawn_worker.rs:396-427andsrc/agent/channel_dispatch.rs:418-447always passSome(system_info.render_for_worker(...)). PassingNoneon Lines 365-375 and 541-551 makes these dumps under-report the worker prompt and stops them from matching “what gets sent to the LLM on every turn.”Also applies to: 541-551
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/context_dump.rs` around lines 365 - 375, The test is calling render_worker_prompt with None for the status_text argument, causing dumps to miss the worker prompt; replace the None with Some(system_info.render_for_worker(...)) so the test uses the same status_text production as production callers. Locate the two test calls to render_worker_prompt (the blocks around lines 365-375 and 541-551) and pass the result of system_info.render_for_worker(...) (matching the arguments used in src production callers) as the status_text parameter instead of None so the dumped prompt matches what gets sent to the LLM.src/agent/channel_dispatch.rs (1)
617-645:⚠️ Potential issue | 🔴 CriticalOpenCode workers are missing temporal/status context injection.
The builtin worker path (lines 410-449) explicitly builds
worker_status_textfrom temporal context and system info, then passes it torender_worker_prompt()to create a system prompt. The OpenCode worker path (lines 617-645) skips this entirely—it constructs the worker without ever callingwith_system_prompt(), leavingsystem_prompt: None.When
OpenCodeWorker.run()sends the prompt request (line 364), it usessystem: self.system_prompt.clone(), which isNone. This silently drops current time, system info, and other context that the builtin workers receive.Both paths should build temporal/status context and set the OpenCode worker's system prompt before execution.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel_dispatch.rs` around lines 617 - 645, The OpenCode worker branch (OpenCodeWorker::new and OpenCodeWorker::new_interactive) never builds or injects the temporal/status system prompt, so system_prompt remains None; fix by replicating the builtin flow: compute worker_status_text from temporal context and system info, call render_worker_prompt(worker_status_text) to produce the system prompt, and call with_system_prompt(...) on the OpenCode worker before returning (apply this for both new_interactive and new paths, after optional with_secrets_store and before with_sqlite_pool or final return). Ensure you reference OpenCodeWorker::new_interactive / OpenCodeWorker::new, render_worker_prompt, with_system_prompt, with_secrets_store, and with_sqlite_pool when implementing the change.
🧹 Nitpick comments (2)
src/agent/channel_dispatch.rs (1)
418-423: Extract worker status-text assembly into one helper.This
SystemInfo+TemporalContextblock is now duplicated in multiple worker spawn/resume paths. A small shared helper would make future status-format changes much harder to miss.Also applies to: 990-997
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel_dispatch.rs` around lines 418 - 423, Extract the duplicated worker status assembly into a single helper function: create a helper (e.g., build_worker_status_text or WorkerStatus::assemble_for_worker) that takes the runtime config reference and sandbox (matching the inputs used by SystemInfo::from_runtime_config and TemporalContext::from_runtime) and returns the Option<String> currently assigned to worker_status_text by calling SystemInfo::from_runtime_config(...).render_for_worker(&TemporalContext::from_runtime(...).current_time_line()). Replace the duplicated blocks (including the occurrences around SystemInfo::from_runtime_config, TemporalContext::from_runtime, current_time_line, and render_for_worker) with calls to this helper to ensure a single source of truth for status formatting.src/agent/channel.rs (1)
2072-2073: Avoidrcin new code.Please spell this out as
runtime_configorconfig; the new helper is otherwise harder to scan and it misses the repo naming rule for Rust locals.As per coding guidelines "Don't abbreviate variable names. Use
queuenotq,messagenotmsg,channelnotch. Common abbreviations likeconfigare fine"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel.rs` around lines 2072 - 2073, Rename the short local variable rc to a descriptive name like runtime_config (or config) where it is assigned from self.deps.runtime_config and then passed into SystemInfo::from_runtime_config; update the binding line (currently let rc = &self.deps.runtime_config;) and any subsequent uses to use runtime_config so the code follows the repo naming rule and improves readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/channels.rs`:
- Around line 408-410: The code is silently swallowing failures from
skills.render_channel_prompt (and similar render/store/serialize calls noted
around the other blocks) by converting Errors into empty strings/None/Null;
change these call sites (e.g., skills.render_channel_prompt, the
storage/serialization helpers at the other noted blocks) to propagate the Result
upwards or return an explicit partial/error status in the API response instead
of unwrap_or_default/None/Null: replace the implicit defaulting with proper
error handling (use ? to return Err from the handler or map errors into a clear
partial/invalid payload field and log the error), ensuring the inspection API
reports a non-200 or a payload flag when parts failed so callers can distinguish
incomplete responses.
- Around line 427-479: The handler is manually recreating the system/next-turn
prompt (using SystemInfo::from_runtime_config, TemporalContext::from_runtime and
calling prompt_engine.render_channel_prompt_with_links with many None values)
which can drift from the live turn; replace this manual assembly by invoking the
shared "next-turn" prompt builder used by the live channel turn (the same
function used during dispatch/turn processing) and pass
channel_state/prompt_engine plus the real runtime context (identity_context,
memory_bulletin, skills_prompt, worker_capabilities and channel metadata) so
coalesce_hint, available_channels, org_context, adapter_prompt and
project_context are resolved the same way as in live dispatch; keep
SystemInfo/TemporalContext usage only if that shared builder requires them,
otherwise remove the ad-hoc render_channel_prompt_with_links call and route
through the canonical builder to produce the exact next-turn prompt.
In `@src/cron/scheduler.rs`:
- Around line 476-479: job_count() currently returns
self.jobs.read().await.len() which counts disabled entries too; change it to
iterate the jobs map and count only entries where the Job struct's enabled field
is true (e.g., filter on job.enabled) or alternatively rename the method/label
to indicate "registered" instead of "active"; locate the job_count method and
update it to read the map and return the number of jobs with enabled == true (or
adjust callers that build the "Cron: … active job(s)" system-info to use the
enabled-only count).
---
Outside diff comments:
In `@src/agent/channel_dispatch.rs`:
- Around line 617-645: The OpenCode worker branch (OpenCodeWorker::new and
OpenCodeWorker::new_interactive) never builds or injects the temporal/status
system prompt, so system_prompt remains None; fix by replicating the builtin
flow: compute worker_status_text from temporal context and system info, call
render_worker_prompt(worker_status_text) to produce the system prompt, and call
with_system_prompt(...) on the OpenCode worker before returning (apply this for
both new_interactive and new paths, after optional with_secrets_store and before
with_sqlite_pool or final return). Ensure you reference
OpenCodeWorker::new_interactive / OpenCodeWorker::new, render_worker_prompt,
with_system_prompt, with_secrets_store, and with_sqlite_pool when implementing
the change.
In `@tests/context_dump.rs`:
- Around line 365-375: The test is calling render_worker_prompt with None for
the status_text argument, causing dumps to miss the worker prompt; replace the
None with Some(system_info.render_for_worker(...)) so the test uses the same
status_text production as production callers. Locate the two test calls to
render_worker_prompt (the blocks around lines 365-375 and 541-551) and pass the
result of system_info.render_for_worker(...) (matching the arguments used in src
production callers) as the status_text parameter instead of None so the dumped
prompt matches what gets sent to the LLM.
---
Nitpick comments:
In `@src/agent/channel_dispatch.rs`:
- Around line 418-423: Extract the duplicated worker status assembly into a
single helper function: create a helper (e.g., build_worker_status_text or
WorkerStatus::assemble_for_worker) that takes the runtime config reference and
sandbox (matching the inputs used by SystemInfo::from_runtime_config and
TemporalContext::from_runtime) and returns the Option<String> currently assigned
to worker_status_text by calling
SystemInfo::from_runtime_config(...).render_for_worker(&TemporalContext::from_runtime(...).current_time_line()).
Replace the duplicated blocks (including the occurrences around
SystemInfo::from_runtime_config, TemporalContext::from_runtime,
current_time_line, and render_for_worker) with calls to this helper to ensure a
single source of truth for status formatting.
In `@src/agent/channel.rs`:
- Around line 2072-2073: Rename the short local variable rc to a descriptive
name like runtime_config (or config) where it is assigned from
self.deps.runtime_config and then passed into SystemInfo::from_runtime_config;
update the binding line (currently let rc = &self.deps.runtime_config;) and any
subsequent uses to use runtime_config so the code follows the repo naming rule
and improves readability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3318238c-b1d9-4048-870e-7a57cedd6761
⛔ Files ignored due to path filters (1)
.github/workflows/release.ymlis excluded by!**/*.yml
📒 Files selected for processing (15)
prompts/en/fragments/system/worker_time_context.md.j2prompts/en/worker.md.j2src/agent/channel.rssrc/agent/channel_dispatch.rssrc/agent/channel_history.rssrc/agent/channel_prompt.rssrc/agent/cortex.rssrc/agent/status.rssrc/api/channels.rssrc/api/server.rssrc/cron/scheduler.rssrc/prompts/engine.rssrc/prompts/text.rssrc/tools/spawn_worker.rstests/context_dump.rs
💤 Files with no reviewable changes (3)
- src/agent/channel_prompt.rs
- src/prompts/text.rs
- prompts/en/fragments/system/worker_time_context.md.j2
Comment on lines
+427
to
+479
| let system_info = crate::agent::status::SystemInfo::from_runtime_config( | ||
| rc.as_ref(), | ||
| &channel_state.deps.sandbox, | ||
| ); | ||
| let temporal_context = crate::agent::channel_prompt::TemporalContext::from_runtime(rc.as_ref()); | ||
| let current_time_line = temporal_context.current_time_line(); | ||
| let status_text = { | ||
| let status = channel_state.status_block.read().await; | ||
| status.render_full(¤t_time_line, &system_info) | ||
| }; | ||
|
|
||
| // ── Conversation context (from DB channel metadata) ── | ||
| let conversation_context = match channel_state.channel_store.get(&query.channel_id).await { | ||
| Ok(Some(info)) => { | ||
| let server_name = info | ||
| .platform_meta | ||
| .as_ref() | ||
| .and_then(|meta| { | ||
| meta.get("discord_guild_name") | ||
| .or_else(|| meta.get("slack_workspace_id")) | ||
| }) | ||
| .and_then(|v| v.as_str()); | ||
| prompt_engine | ||
| .render_conversation_context( | ||
| &info.platform, | ||
| server_name, | ||
| info.display_name.as_deref(), | ||
| ) | ||
| .ok() | ||
| } | ||
| _ => None, | ||
| }; | ||
|
|
||
| let sandbox_enabled = channel_state.deps.sandbox.containment_active(); | ||
|
|
||
| let empty_to_none = |s: String| if s.is_empty() { None } else { Some(s) }; | ||
|
|
||
| // ── Render system prompt ── | ||
| let system_prompt = prompt_engine | ||
| .render_channel_prompt_with_links( | ||
| empty_to_none(identity_context), | ||
| empty_to_none(memory_bulletin.to_string()), | ||
| empty_to_none(skills_prompt), | ||
| worker_capabilities, | ||
| conversation_context, | ||
| empty_to_none(status_text), | ||
| None, // coalesce_hint — only relevant during batched dispatch | ||
| None, // available_channels — requires messaging manager context | ||
| sandbox_enabled, | ||
| None, // org_context — requires link resolution | ||
| None, // adapter_prompt — requires adapter context | ||
| None, // project_context — requires project store queries | ||
| ) |
Contributor
There was a problem hiding this comment.
Reuse the real next-turn prompt builder here.
Lines 427-479 still reconstruct the prompt by hand. SystemInfo::from_runtime_config() only gives the synchronous base snapshot, and Lines 473-479 hardcode None for coalesce_hint, available_channels, org_context, adapter_prompt, and project_context. That means /channels/inspect can drift from the actual next-turn prompt even though the handler/docstring says it is exact. Please route this through the same prompt-assembly path the live channel turn uses.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/channels.rs` around lines 427 - 479, The handler is manually
recreating the system/next-turn prompt (using SystemInfo::from_runtime_config,
TemporalContext::from_runtime and calling
prompt_engine.render_channel_prompt_with_links with many None values) which can
drift from the live turn; replace this manual assembly by invoking the shared
"next-turn" prompt builder used by the live channel turn (the same function used
during dispatch/turn processing) and pass channel_state/prompt_engine plus the
real runtime context (identity_context, memory_bulletin, skills_prompt,
worker_capabilities and channel metadata) so coalesce_hint, available_channels,
org_context, adapter_prompt and project_context are resolved the same way as in
live dispatch; keep SystemInfo/TemporalContext usage only if that shared builder
requires them, otherwise remove the ad-hoc render_channel_prompt_with_links call
and route through the canonical builder to produce the exact next-turn prompt.
Debug tool that shows exactly what the LLM sees on each turn. The inspector modal displays the full rendered system prompt and conversation history as raw monospace text — useful for debugging prompt construction, message coalescing, history backfill, and status block content. Snapshot capture is a per-channel toggle that records the exact prompt and history on every LLM turn. Snapshots are stored in a dedicated redb database (prompt_snapshots.redb) so they can be deleted independently. Backend: - PromptSnapshotStore with save/list/get/clear_channel (separate redb) - Channel::maybe_capture_snapshot() fires-and-forgets on each turn - Settings store: prompt_capture_enabled/set_prompt_capture per channel - API: GET /channels/inspect returns rendered system_prompt + history - API: POST /channels/inspect/capture toggles capture - API: GET /channels/inspect/snapshots and /snapshot for history Frontend: - PromptInspectModal with Current/History sidebar and capture toggle - Raw text rendering of system prompt and message history - Snapshot list with click-to-view historical captures
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@interface/src/components/PromptInspectModal.tsx`:
- Around line 31-43: The two useQuery calls for listPromptSnapshots and
getPromptSnapshot currently ignore errors, so API failures silently show the
empty state; update both queries to capture and surface errors by adding onError
handlers and/or reading the returned error/isError values (for the query keys
["promptSnapshots", channelId] and ["promptSnapshot", channelId,
selectedSnapshot]) and propagate them to the component-level error state or into
the same error renderer used for inspectPrompt. Concretely, modify the useQuery
for listPromptSnapshots and getPromptSnapshot to include onError: (err) =>
setError(err) (or set a local error variable), and/or read error/isError from
the hooks (e.g., snapshotListError, snapshotDetailError) and render a
descriptive error message instead of the generic "No snapshots yet" empty state
when those errors are present.
- Around line 3-8: The import list in PromptInspectModal.tsx includes an unused
symbol PromptSnapshot which triggers TS6133; remove PromptSnapshot from the
import statement that currently imports api, PromptInspectResponse,
PromptSnapshotSummary, and PromptSnapshot so only the used types (e.g.,
PromptSnapshotSummary and PromptInspectResponse) and api remain imported.
In `@interface/src/routes/ChannelDetail.tsx`:
- Around line 386-393: The Inspect button currently relies on title for
accessibility which isn't reliable; update the Button element (the one using
onClick={() => setInspectOpen(true)} with HugeiconsIcon and CodeIcon) to include
an explicit accessible name by adding aria-label="Inspect prompt" so screen
readers announce the action correctly.
In `@src/agent/channel.rs`:
- Around line 3058-3071: The code currently uses
serde_json::to_value(history).unwrap_or(Value::Null) which can produce a
snapshot with history: null while history_length > 0; change the logic around
serde_json::to_value(history) in the block that builds
crate::agent::prompt_snapshot::PromptSnapshot so that serialization errors are
not swallowed: call serde_json::to_value(history) and handle the Result
explicitly (propagate the error or log and skip creating/saving the snapshot),
e.g., if to_value returns Err then abort constructing PromptSnapshot (do not set
history to Null) and surface or log the error; update any callers of the
snapshot creation to handle the propagated error or the conditional skip
accordingly (refer to history_json, history_length, PromptSnapshot and the
serde_json::to_value call to locate the code).
In `@src/agent/prompt_snapshot.rs`:
- Around line 152-169: The loop currently swallows deserialization failures for
PromptSnapshot (using if let Ok(...)) which hides corrupt rows; change the code
to propagate or return an error when
serde_json::from_slice::<PromptSnapshot>(data) fails instead of skipping—e.g.,
replace the if let Ok(...) block with a match that on Err maps the serde error
into the existing crate::error::SettingsError (or another appropriate error) and
returns it, so callers of the function receive the deserialization failure;
update any function signature if needed to propagate the error from this loop
that constructs PromptSnapshotSummary entries.
In `@src/api/channels.rs`:
- Around line 531-542: The current lookup that sets runtime_config (using
state.runtime_configs.load() and falling back to configs.values().next()) can
return the wrong agent's config when a channel is inactive; change this to
require or resolve the owning agent before falling back: look up the channel
ownership via the per-agent ChannelStore (or require body.agent_id) using
state.channel_states and ChannelStore APIs to determine the correct agent_id,
then select the runtime config/store for that agent_id instead of arbitrarily
using the first entry; apply the same fix to the analogous lookup at the other
location that handles snapshot/store resolution (the other block that mirrors
this logic).
In `@src/main.rs`:
- Around line 2462-2472: The prompt snapshot DB initialization
(PromptSnapshotStore::new) currently returns an error that aborts
initialize_agents(); change prompt_snapshot_store to be an
Option<Arc<PromptSnapshotStore>> so a failure becomes a non-fatal warning:
attempt to create the store with
PromptSnapshotStore::new(&snapshot_path).with_context(...).map(Arc::new).map(Some).
If that returns Err, log a warning including agent_config.id and the error and
set prompt_snapshot_store = None (i.e., disable snapshotting for that agent)
instead of returning Err; update any downstream code that used
prompt_snapshot_store to handle the Option (skip snapshot operations when None).
Ensure the same fallback is applied to the other occurrence noted (lines around
2538-2540).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 554957b9-a08b-4521-8f13-ad2868b31868
📒 Files selected for processing (13)
interface/src/api/client.tsinterface/src/components/PromptInspectModal.tsxinterface/src/routes/ChannelDetail.tsxsrc/agent.rssrc/agent/channel.rssrc/agent/prompt_snapshot.rssrc/api/channels.rssrc/api/server.rssrc/config/runtime.rssrc/cron/scheduler.rssrc/main.rssrc/settings/store.rstests/context_dump.rs
✅ Files skipped from review due to trivial changes (1)
- src/agent.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/cron/scheduler.rs
- tests/context_dump.rs
Comment on lines
+31
to
+43
| const { data: snapshotList } = useQuery({ | ||
| queryKey: ["promptSnapshots", channelId], | ||
| queryFn: () => api.listPromptSnapshots(channelId), | ||
| enabled: open && view === "history", | ||
| staleTime: 0, | ||
| }); | ||
|
|
||
| const { data: snapshotDetail, isLoading: snapshotLoading } = useQuery({ | ||
| queryKey: ["promptSnapshot", channelId, selectedSnapshot], | ||
| queryFn: () => api.getPromptSnapshot(channelId, selectedSnapshot!), | ||
| enabled: open && selectedSnapshot !== null, | ||
| staleTime: 0, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd PromptInspectModal.tsxRepository: spacedriveapp/spacebot
Length of output: 113
🏁 Script executed:
wc -l interface/src/components/PromptInspectModal.tsxRepository: spacedriveapp/spacebot
Length of output: 117
🏁 Script executed:
cat -n interface/src/components/PromptInspectModal.tsxRepository: spacedriveapp/spacebot
Length of output: 13590
Add error handling for snapshot list and detail queries.
The error variable only covers inspectPrompt (line 144). The listPromptSnapshots and getPromptSnapshot queries lack error handling; if either fails, the error silently fails and renders as an empty state ("No snapshots yet"), making API failures indistinguishable from the absence of snapshots.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interface/src/components/PromptInspectModal.tsx` around lines 31 - 43, The
two useQuery calls for listPromptSnapshots and getPromptSnapshot currently
ignore errors, so API failures silently show the empty state; update both
queries to capture and surface errors by adding onError handlers and/or reading
the returned error/isError values (for the query keys ["promptSnapshots",
channelId] and ["promptSnapshot", channelId, selectedSnapshot]) and propagate
them to the component-level error state or into the same error renderer used for
inspectPrompt. Concretely, modify the useQuery for listPromptSnapshots and
getPromptSnapshot to include onError: (err) => setError(err) (or set a local
error variable), and/or read error/isError from the hooks (e.g.,
snapshotListError, snapshotDetailError) and render a descriptive error message
instead of the generic "No snapshots yet" empty state when those errors are
present.
Comment on lines
+152
to
+169
| for entry in range { | ||
| let entry = entry.map_err(|error| crate::error::SettingsError::ReadFailed { | ||
| key: prefix.clone(), | ||
| details: error.to_string(), | ||
| })?; | ||
| let key = entry.0.value(); | ||
| if !key.starts_with(&prefix) { | ||
| break; // Past our channel's entries. | ||
| } | ||
| let data = entry.1.value(); | ||
| if let Ok(snapshot) = serde_json::from_slice::<PromptSnapshot>(data) { | ||
| summaries.push(PromptSnapshotSummary { | ||
| timestamp_ms: snapshot.timestamp_ms, | ||
| user_message: snapshot.user_message, | ||
| system_prompt_chars: snapshot.system_prompt_chars, | ||
| history_length: snapshot.history_length, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Fail the listing when a stored snapshot is unreadable.
A corrupt row currently just disappears from the response, which makes snapshot loss look like "no data" instead of a storage problem. Propagate the deserialization error, or at least log and abort, rather than silently skipping it. As per coding guidelines, "Don't silently discard errors. No let _ = on Results. Handle them, log them, or propagate them."
🛠️ Proposed fix
let key = entry.0.value();
if !key.starts_with(&prefix) {
break; // Past our channel's entries.
}
let data = entry.1.value();
- if let Ok(snapshot) = serde_json::from_slice::<PromptSnapshot>(data) {
- summaries.push(PromptSnapshotSummary {
- timestamp_ms: snapshot.timestamp_ms,
- user_message: snapshot.user_message,
- system_prompt_chars: snapshot.system_prompt_chars,
- history_length: snapshot.history_length,
- });
- }
+ let snapshot = serde_json::from_slice::<PromptSnapshot>(data).map_err(|error| {
+ crate::error::SettingsError::ReadFailed {
+ key: key.to_string(),
+ details: format!("failed to deserialize snapshot: {error}"),
+ }
+ })?;
+ summaries.push(PromptSnapshotSummary {
+ timestamp_ms: snapshot.timestamp_ms,
+ user_message: snapshot.user_message,
+ system_prompt_chars: snapshot.system_prompt_chars,
+ history_length: snapshot.history_length,
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agent/prompt_snapshot.rs` around lines 152 - 169, The loop currently
swallows deserialization failures for PromptSnapshot (using if let Ok(...))
which hides corrupt rows; change the code to propagate or return an error when
serde_json::from_slice::<PromptSnapshot>(data) fails instead of skipping—e.g.,
replace the if let Ok(...) block with a match that on Err maps the serde error
into the existing crate::error::SettingsError (or another appropriate error) and
returns it, so callers of the function receive the deserialization failure;
update any function signature if needed to propagate the error from this loop
that constructs PromptSnapshotSummary entries.
Comment on lines
+531
to
+542
| // Find the agent's runtime config that owns this channel. | ||
| let runtime_config = { | ||
| let configs = state.runtime_configs.load(); | ||
| let channel_state = state.channel_states.read().await; | ||
| channel_state | ||
| .get(&body.channel_id) | ||
| .map(|cs| cs.deps.runtime_config.clone()) | ||
| .or_else(|| { | ||
| // Fall back to first agent config if channel not active | ||
| configs.values().next().cloned() | ||
| }) | ||
| }; |
Contributor
There was a problem hiding this comment.
Resolve inactive channels to their owning agent before using config/store fallbacks.
These paths currently fall back to the first runtime config/store in the process once the channel is not active. In a multi-agent deployment that can toggle capture on the wrong agent or list/get snapshots from the wrong database while still returning a normal response. Require agent_id here, or resolve ownership through the per-agent ChannelStore before selecting a runtime config or snapshot store.
Also applies to: 623-650
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/channels.rs` around lines 531 - 542, The current lookup that sets
runtime_config (using state.runtime_configs.load() and falling back to
configs.values().next()) can return the wrong agent's config when a channel is
inactive; change this to require or resolve the owning agent before falling
back: look up the channel ownership via the per-agent ChannelStore (or require
body.agent_id) using state.channel_states and ChannelStore APIs to determine the
correct agent_id, then select the runtime config/store for that agent_id instead
of arbitrarily using the first entry; apply the same fix to the analogous lookup
at the other location that handles snapshot/store resolution (the other block
that mirrors this logic).
Comment on lines
+223
to
+225
| pub fn prompt_capture_enabled(&self, channel_id: &str) -> bool { | ||
| let key = format!("{PROMPT_CAPTURE_PREFIX}{channel_id}"); | ||
| matches!(self.get_raw(&key), Ok(v) if v == "true") |
Contributor
There was a problem hiding this comment.
Don't hide settings-store failures as false.
If get_raw() starts failing or the stored value is malformed, this reports prompt capture as disabled instead of surfacing a storage problem. At minimum log the failure; ideally mirror the existing boolean-setting readers and return a Result.
🩹 Local fix
pub fn prompt_capture_enabled(&self, channel_id: &str) -> bool {
let key = format!("{PROMPT_CAPTURE_PREFIX}{channel_id}");
- matches!(self.get_raw(&key), Ok(v) if v == "true")
+ match self.get_raw(&key) {
+ Ok(raw) => match raw.parse::<bool>() {
+ Ok(enabled) => enabled,
+ Err(error) => {
+ tracing::warn!(%channel_id, value = %raw, %error, "invalid prompt capture setting");
+ false
+ }
+ },
+ Err(crate::error::Error::Settings(settings_error))
+ if matches!(*settings_error, SettingsError::NotFound { .. }) => false,
+ Err(error) => {
+ tracing::warn!(%channel_id, %error, "failed to read prompt capture setting");
+ false
+ }
+ }
}As per coding guidelines, "Don't silently discard errors. No let _ = on Results. Handle them, log them, or propagate them."
- Surface serde_json::to_value failure as 500 in inspect API - Use .chars().count() for system_prompt_chars (was bytes) - Add context window to render_for_worker (doc/impl mismatch) - Extract build_worker_status_text() helper to deduplicate - Inject temporal/status context into OpenCode workers - Handle snapshot history serialization failure (log + skip) - Make prompt snapshot store init non-fatal - Rename rc to runtime_config in build_system_info - Count only enabled jobs in job_count() - Remove unused PromptSnapshot import - Add aria-label to inspect button
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
## Systemsection in their status block with 17 fields: version, deployment, models, thinking effort, context window, limits, capabilities, MCP servers, sandbox, warmup, bulletin age, cron count. Smart collapsing keeps it compact (identical models shown once, thinking effort hidden when all "auto", cron hidden when zero).status_textinworker.md.j2. Deletedbuild_worker_task_with_temporal_context(),worker_task_preamble(), and the deadworker_time_context.md.j2template + its registration.GET /api/channels/inspect?channel_id=...returns the exact system prompt and conversation history the LLM would see on the next turn, as JSON.verify-versionjob torelease.ymlthat ensuresCargo.tomlversion matches the git tag before building.Changed files (16)
src/agent/status.rsSystemInfostruct, constructors, renderers, 8 new testssrc/agent/channel.rsbuild_system_info(), updated prompt builders to userender_full()src/agent/channel_prompt.rssrc/agent/channel_dispatch.rsSystemInfostatus textsrc/agent/channel_history.rsworker_system_info_render_includes_time_and_modelsrc/agent/cortex.rsSystemInfostatus textsrc/tools/spawn_worker.rsSystemInfostatus textsrc/prompts/engine.rsrender_worker_prompt()takesstatus_textparam; removed dead template registrationsrc/prompts/text.rsinclude_str!for worker_time_contextprompts/en/worker.md.j2status_textblockprompts/en/fragments/system/worker_time_context.md.j2src/cron/scheduler.rspub async fn job_count()src/api/channels.rsinspect_prompt()handlersrc/api/server.rs/channels/inspectroutetests/context_dump.rsstatus_textparam.github/workflows/release.ymlverify-versionjobTesting
just gate-prpasses: 499 tests, 0 failures. 8 new unit tests for SystemInfo rendering.Note
This PR enhances system observability with a structured SystemInfo status block (17 fields), establishes the canonical way for workers to access temporal context via system prompts (eliminating an anti-pattern), and adds operator tooling with the prompt inspect API for debugging. The release workflow gains version verification to prevent tag/code mismatches. All changes are backward compatible; the status block intelligently collapses redundant information to stay compact in channel context.
Written by Tembo for commit 38c8cd4. This will update automatically on new commits.