From 42e6e330f40412b0d10d3f89825450a16643a798 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Thu, 7 May 2026 11:32:41 +0200 Subject: [PATCH 1/8] fix(logs,storage): runtime logs overhaul (pagination, ordering, density) + storage UI polish --- .../src/handlers/log_handler.rs | 187 +++- .../src/services/search.rs | 167 +++- crates/temps-log-aggregator/src/types.rs | 2 +- web/src/components/command/CommandPalette.tsx | 5 +- .../runtime-logs/history-log-viewer.tsx | 799 ++++++++++++------ web/src/components/runtime-logs/log-line.tsx | 2 +- .../components/runtime-logs/log-viewer.tsx | 489 ++++++++--- .../storage/EditResourceLimitsDialog.tsx | 429 +++++++--- .../storage/ServiceResourcesPanel.tsx | 430 +++++----- web/src/globals.css | 25 + web/src/pages/Storage.tsx | 135 ++- 11 files changed, 1941 insertions(+), 729 deletions(-) diff --git a/crates/temps-log-aggregator/src/handlers/log_handler.rs b/crates/temps-log-aggregator/src/handlers/log_handler.rs index 5e39e2db7f..00f05ff9b7 100644 --- a/crates/temps-log-aggregator/src/handlers/log_handler.rs +++ b/crates/temps-log-aggregator/src/handlers/log_handler.rs @@ -1286,11 +1286,14 @@ mod tests { #[tokio::test] #[serial_test::serial] async fn test_search_logs_with_pagination() { + // Regression test: pagination via next_cursor must walk into older + // logs without overlap. Previously the server emitted next_cursor + // but never honored filter.cursor on the way back in, so every + // "next page" click returned page 1 again. let ctx = create_test_context().await; let project_id = next_test_project_id(); let now = Utc::now(); - // Seed many log lines let mut lines = Vec::new(); for i in 0..15 { lines.push(make_log_line( @@ -1307,8 +1310,7 @@ mod tests { let server = build_test_server(ctx.app_state.clone()); - // Request a small page - let response = server + let page1 = server .post("/logs/search") .json(&serde_json::json!({ "project_id": project_id, @@ -1317,15 +1319,182 @@ mod tests { "page_size": 5, })) .await; + assert_eq!(page1.status_code(), StatusCode::OK); + let page1_body: serde_json::Value = page1.json(); + let page1_lines = page1_body["lines"].as_array().unwrap(); + assert_eq!(page1_lines.len(), 5); + let page1_cursor = page1_body["next_cursor"] + .as_str() + .expect("page 1 must emit a cursor when more matches exist (15 total, 5 per page)"); + + let page2 = server + .post("/logs/search") + .json(&serde_json::json!({ + "project_id": project_id, + "start_time": (now - Duration::hours(1)).to_rfc3339(), + "end_time": (now + Duration::hours(1)).to_rfc3339(), + "page_size": 5, + "cursor": page1_cursor, + })) + .await; + assert_eq!(page2.status_code(), StatusCode::OK); + let page2_body: serde_json::Value = page2.json(); + let page2_lines = page2_body["lines"].as_array().unwrap(); + assert_eq!(page2_lines.len(), 5, "page 2 should have 5 more lines"); + + // No overlap: every page-2 message is distinct from every page-1 message. + let p1_msgs: std::collections::HashSet<&str> = page1_lines + .iter() + .map(|l| l["message"].as_str().unwrap()) + .collect(); + for l in page2_lines { + let m = l["message"].as_str().unwrap(); + assert!( + !p1_msgs.contains(m), + "page 2 leaked a page-1 line: {} (cursor was ignored)", + m, + ); + } - assert_eq!(response.status_code(), StatusCode::OK); + // Page 2 lines are strictly older than every page-1 line. + let p1_oldest = page1_lines + .iter() + .map(|l| l["timestamp"].as_str().unwrap()) + .min() + .unwrap(); + for l in page2_lines { + let ts = l["timestamp"].as_str().unwrap(); + assert!( + ts < p1_oldest, + "page 2 line {} is not older than page-1 oldest {}", + ts, + p1_oldest, + ); + } + } + + #[tokio::test] + #[serial_test::serial] + async fn test_search_logs_returns_chronological_order() { + // The page is bounded server-side to the *newest* page_size matches in + // the time window (heap-kept), but the wire format is ASC so the UI + // can render terminal-style with newest at the bottom. "Chronological" + // here means oldest first inside the page; "load older" pages pre- + // pend at the top of the rendered rope. + let ctx = create_test_context().await; + let project_id = next_test_project_id(); + let now = Utc::now(); + + let mut lines = Vec::new(); + for i in 0..10 { + lines.push(make_log_line( + project_id, + "api", + "prod", + LogLevel::Info, + &format!("event {}", i), + now - Duration::seconds(10 - i), + "container-order", + )); + } + seed_logs(&ctx, lines).await; + + let server = build_test_server(ctx.app_state.clone()); + let response = server + .post("/logs/search") + .json(&serde_json::json!({ + "project_id": project_id, + "start_time": (now - Duration::hours(1)).to_rfc3339(), + "end_time": (now + Duration::hours(1)).to_rfc3339(), + "page_size": 100, + })) + .await; let body: serde_json::Value = response.json(); - let lines_arr = body["lines"].as_array().unwrap(); - assert!( - lines_arr.len() <= 5, - "Expected at most 5 lines with page_size=5, got {}", - lines_arr.len() + let arr = body["lines"].as_array().unwrap(); + assert_eq!(arr.len(), 10); + + // Each subsequent line must be strictly newer (ASC ordering). + for w in arr.windows(2) { + let a = w[0]["timestamp"].as_str().unwrap(); + let b = w[1]["timestamp"].as_str().unwrap(); + assert!( + a <= b, + "results not in ASC order: {} appeared before {}", + a, + b, + ); + } + } + + #[tokio::test] + #[serial_test::serial] + async fn test_search_logs_wider_window_returns_more_recent_logs() { + // Regression test: the early-exit `break` after page_size matches + // ran chunks oldest-first, so a "Last 6 hours" query and a "Last 1 + // hour" query returned essentially the same lines from the start + // of the window. The bigger window expansion was wasted because + // the scanner stopped reading once it had enough, and "enough" + // came from the oldest chunks. + let ctx = create_test_context().await; + let project_id = next_test_project_id(); + let now = Utc::now(); + + // Seed 10 lines spread across 4 hours: half at -3h30m, half at -30m. + let mut lines = Vec::new(); + for i in 0..10 { + let offset = if i < 5 { + Duration::minutes(-210) + } else { + Duration::minutes(-30) + }; + lines.push(make_log_line( + project_id, + "api", + "prod", + LogLevel::Info, + &format!("evt-{}", i), + now + offset + Duration::seconds(i), + "container-window", + )); + } + seed_logs(&ctx, lines).await; + + let server = build_test_server(ctx.app_state.clone()); + + // Last 1 hour: only the -30m batch is in window (5 lines). + let r1h = server + .post("/logs/search") + .json(&serde_json::json!({ + "project_id": project_id, + "start_time": (now - Duration::hours(1)).to_rfc3339(), + "end_time": (now + Duration::minutes(1)).to_rfc3339(), + "page_size": 100, + })) + .await; + let body_1h: serde_json::Value = r1h.json(); + let len_1h = body_1h["lines"].as_array().unwrap().len(); + assert_eq!( + len_1h, 5, + "Last-1h window should hold exactly the -30m batch" + ); + + // Last 4 hours: both batches are in window (10 lines). If the early-exit + // bug were still present, this would also return ~5. + let r4h = server + .post("/logs/search") + .json(&serde_json::json!({ + "project_id": project_id, + "start_time": (now - Duration::hours(4)).to_rfc3339(), + "end_time": (now + Duration::minutes(1)).to_rfc3339(), + "page_size": 100, + })) + .await; + let body_4h: serde_json::Value = r4h.json(); + let len_4h = body_4h["lines"].as_array().unwrap().len(); + assert_eq!( + len_4h, 10, + "Last-4h window should expand to include both batches", ); } diff --git a/crates/temps-log-aggregator/src/services/search.rs b/crates/temps-log-aggregator/src/services/search.rs index e8176ae8ae..5d77506ad3 100644 --- a/crates/temps-log-aggregator/src/services/search.rs +++ b/crates/temps-log-aggregator/src/services/search.rs @@ -3,9 +3,11 @@ //! All searches read directly from compressed chunk files on disk/S3. //! The log_chunks table provides the index of which files to read. -use std::collections::HashSet; +use std::cmp::Reverse; +use std::collections::{BinaryHeap, HashSet}; use std::sync::Arc; +use chrono::{DateTime, Utc}; use tracing::{debug, warn}; use uuid::Uuid; @@ -14,6 +16,34 @@ use crate::services::LogMetadataService; use crate::storage::LogStorage; use crate::types::*; +/// Heap entry that orders LogSearchLine by timestamp only. +/// +/// We need this because `LogSearchLine` doesn't implement Ord (it carries +/// JSON values that aren't comparable). The heap's min/max behavior depends +/// solely on the timestamp; ties keep insertion order, which is fine — the +/// dedup pass upstream already collapsed identical lines. +struct HeapEntry { + timestamp: DateTime, + line: LogSearchLine, +} + +impl PartialEq for HeapEntry { + fn eq(&self, other: &Self) -> bool { + self.timestamp == other.timestamp + } +} +impl Eq for HeapEntry {} +impl PartialOrd for HeapEntry { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} +impl Ord for HeapEntry { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.timestamp.cmp(&other.timestamp) + } +} + /// Maximum time range for full text search (hours) const MAX_FULLTEXT_HOURS: u32 = 24; @@ -42,12 +72,26 @@ impl LogSearchService { // Validate required params self.validate_filter(filter)?; - let page_size = std::cmp::min(filter.page_size, 500) as u64; + // 2000 is the upper bound — keeps memory bounded for any single + // request while letting the UI fetch enough rope to feel useful. + // Frontend defaults to 500. + let page_size = std::cmp::min(filter.page_size, 2000) as u64; // Always search from chunk files — no log_events table needed self.archive_search(filter, page_size).await } + /// Parse a pagination cursor of the form `:`. + /// + /// Cursor semantics: "give me lines strictly older than this timestamp". + /// The chunk UUID is purely informational and not currently used for + /// disambiguation — the timestamp is the only ordering key. + fn parse_cursor(cursor: &str) -> Option> { + let (ts_str, _) = cursor.split_once(':')?; + let ts_millis: i64 = ts_str.parse().ok()?; + chrono::DateTime::from_timestamp_millis(ts_millis) + } + /// Get context lines around a specific log line in a chunk. pub async fn get_context( &self, @@ -135,15 +179,22 @@ impl LogSearchService { filter: &LogSearchFilter, page_size: u64, ) -> Result { + // When a cursor is set, narrow the effective end_time to the cursor's + // timestamp so we strictly paginate into older logs. The cursor was + // emitted as the oldest line of the previous page; "older than that" + // means strictly less, so we use < not <= when filtering lines below. + let cursor_ts = filter.cursor.as_deref().and_then(Self::parse_cursor); + let effective_end = cursor_ts.unwrap_or(filter.end_time).min(filter.end_time); + // Find relevant chunks via metadata let service_filter = filter.services.first().map(|s| s.as_str()); - let chunks = self + let mut chunks = self .metadata_service .find_chunks( filter.project_id, service_filter, filter.start_time, - filter.end_time, + effective_end, ) .await?; @@ -156,13 +207,31 @@ impl LogSearchService { }); } + // Process chunks newest-first so the bounded heap converges quickly: + // the first chunks we read already hold the most recent lines, and + // the heap rejects older lines once full. + chunks.sort_by(|a, b| b.ended_at.cmp(&a.ended_at)); + debug!( project_id = %filter.project_id, chunk_count = chunks.len(), + page_size, + has_cursor = cursor_ts.is_some(), "Starting archive search" ); - let mut all_matches = Vec::new(); + // Bounded min-heap of size `page_size + 1`. Top of heap is the + // oldest line in the heap; once the heap is full we drop any new + // line older than the top, and replace the top when a newer line + // arrives. We keep one extra slot so that on early termination we + // still have a "what's just past the page" line to confirm has_more + // and emit the cursor. + // + // Memory: O(page_size) regardless of total matches in window. + // Time: O(N log K) where N=total matches, K=page_size. + let heap_cap = page_size as usize + 1; + let mut heap: BinaryHeap> = BinaryHeap::with_capacity(heap_cap); + let mut total_scanned = 0u64; // Dedup key: (ts_nanos, container_id, stream, msg). Lines identical on all four // are considered the same event and collapsed. This defends against: @@ -201,21 +270,36 @@ impl LogSearchService { Ok((chunk_id, data)) => { let content = String::from_utf8_lossy(&data); for (line_idx, raw_line) in content.lines().enumerate() { - total_scanned += 1; - if let Ok(parsed) = serde_json::from_str::(raw_line) { - if !self.line_matches_filter(&parsed, filter) { - continue; - } - let dedup_key = ( - parsed.ts.timestamp_nanos_opt().unwrap_or(0), - parsed.container_id.clone(), - parsed.stream, - parsed.msg.clone(), - ); - if !seen.insert(dedup_key) { + let Ok(parsed) = serde_json::from_str::(raw_line) else { + continue; + }; + if !self.line_matches_filter(&parsed, filter) { + continue; + } + // Cursor is exclusive: only return lines strictly + // older than the cursor timestamp so the previous + // page's oldest line isn't returned again. + if let Some(c) = cursor_ts { + if parsed.ts >= c { continue; } - all_matches.push(LogSearchLine { + } + let dedup_key = ( + parsed.ts.timestamp_nanos_opt().unwrap_or(0), + parsed.container_id.clone(), + parsed.stream, + parsed.msg.clone(), + ); + if !seen.insert(dedup_key) { + continue; + } + // total_scanned counts matches that survived all + // filters (time + level + service + text). Users + // care about "matches in window", not raw bytes. + total_scanned += 1; + let entry = HeapEntry { + timestamp: parsed.ts, + line: LogSearchLine { timestamp: parsed.ts, level: parsed.level, service: parsed.service, @@ -224,7 +308,16 @@ impl LogSearchService { chunk_id, line_offset: line_idx as i32, deploy_id: parsed.deploy_id, - }); + }, + }; + + if heap.len() < heap_cap { + heap.push(Reverse(entry)); + } else if let Some(Reverse(oldest)) = heap.peek() { + if entry.timestamp > oldest.timestamp { + heap.pop(); + heap.push(Reverse(entry)); + } } } } @@ -233,27 +326,39 @@ impl LogSearchService { } } } - - // Early exit if we have enough results - if all_matches.len() > page_size as usize { - break; - } } - // Sort by timestamp ascending (oldest first, like a terminal) - all_matches.sort_by_key(|a| a.timestamp); - - let has_more = all_matches.len() > page_size as usize; - all_matches.truncate(page_size as usize); - + // The bounded heap kept the page_size + 1 *newest* matches in window + // regardless of how many total matches exist (memory bound). Now we + // need to (a) decide if there's more (heap overshot by one) and + // (b) drop the extra entry so the response carries exactly page_size + // lines, then (c) return them in CHRONOLOGICAL order (ASC) so the UI + // can render terminal-style with newest at the bottom. + let mut newest_first: Vec = + heap.into_iter().map(|Reverse(e)| e.line).collect(); + newest_first.sort_by(|a, b| b.timestamp.cmp(&a.timestamp)); + + let has_more = newest_first.len() > page_size as usize; + newest_first.truncate(page_size as usize); + + // next_cursor points at the OLDEST line in this page. The next call + // narrows end_time to this timestamp and only returns lines strictly + // older — i.e. "load older". Read it off the newest-first vec while + // the oldest is still trivially `last()`, before we flip orientation. let next_cursor = if has_more { - all_matches + newest_first .last() .map(|l| format!("{}:{}", l.timestamp.timestamp_millis(), l.chunk_id)) } else { None }; + // Flip to ASC for the wire format: oldest first, newest last. Matches + // terminal/journalctl convention so the UI's auto-load-older pattern + // can prepend to the top while new content appears at the bottom. + let mut all_matches = newest_first; + all_matches.reverse(); + Ok(LogSearchResult { lines: all_matches, next_cursor, diff --git a/crates/temps-log-aggregator/src/types.rs b/crates/temps-log-aggregator/src/types.rs index 8a57bff9b6..1f14ccc781 100644 --- a/crates/temps-log-aggregator/src/types.rs +++ b/crates/temps-log-aggregator/src/types.rs @@ -144,7 +144,7 @@ pub struct LogSearchFilter { /// Cursor for pagination #[serde(default, skip_serializing_if = "Option::is_none")] pub cursor: Option, - /// Page size (default: 100, max: 500) + /// Page size (default: 100, max: 2000) #[serde(default = "default_page_size")] pub page_size: u32, } diff --git a/web/src/components/command/CommandPalette.tsx b/web/src/components/command/CommandPalette.tsx index 5b8c5ef9ed..d4a4c38adc 100644 --- a/web/src/components/command/CommandPalette.tsx +++ b/web/src/components/command/CommandPalette.tsx @@ -37,6 +37,7 @@ import { GitBranch, Globe, HardDrive, + History, Home, Key, Mail, @@ -382,7 +383,7 @@ const projectNavItems: NavigationItem[] = [ { title: 'Log History', url: 'runtime?tab=history', - icon: ScrollText, + icon: History, keywords: ['logs', 'history', 'search', 'archive', 'past'], }, { @@ -538,7 +539,7 @@ const projectNavItems: NavigationItem[] = [ { title: 'Request Logs', url: 'request-logs', - icon: ScrollText, + icon: Network, keywords: ['logs', 'requests', 'http', 'traffic'], }, ] diff --git a/web/src/components/runtime-logs/history-log-viewer.tsx b/web/src/components/runtime-logs/history-log-viewer.tsx index d902048d4c..222c93b395 100644 --- a/web/src/components/runtime-logs/history-log-viewer.tsx +++ b/web/src/components/runtime-logs/history-log-viewer.tsx @@ -13,6 +13,14 @@ import { SelectTrigger, SelectValue, } from '@/components/ui/select' +import { + Tooltip, + TooltipContent, + TooltipProvider, + TooltipTrigger, +} from '@/components/ui/tooltip' +import { DateRangePicker } from '@/components/ui/date-range-picker' +import type { DateRange } from 'react-day-picker' import { cn } from '@/lib/utils' import { LogLevel, @@ -24,13 +32,18 @@ import { useVirtualizer } from '@tanstack/react-virtual' import AnsiToHtml from 'ansi-to-html' import { AlertCircle, - ChevronLeft, - ChevronRight, + ArrowUp, Clock, Loader2, Search, } from 'lucide-react' -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { + useCallback, + useEffect, + useMemo, + useRef, + useState, +} from 'react' const ansiConverter = new AnsiToHtml({ fg: 'var(--foreground)', @@ -49,42 +62,33 @@ const LEVEL_COLORS: Record = { TRACE: 'bg-zinc-400/15 text-zinc-500 dark:text-zinc-500 border-zinc-400/20', } -function TimeRangeSelect({ - value, - onChange, -}: { - value: string - onChange: (value: string) => void -}) { - return ( - - ) -} +// Server enforces a 24h cap on full-text search (MAX_FULLTEXT_HOURS in +// search.rs). The 7d/30d presets and any custom range over 24h are fine for +// level/service filters but the search box must be disabled to avoid a 400 +// from the backend. +const FULLTEXT_MAX_HOURS = 24 + +const TIME_RANGES: Array<{ value: string; label: string; hours: number }> = [ + { value: '15m', label: 'Last 15 min', hours: 0.25 }, + { value: '1h', label: 'Last 1 hour', hours: 1 }, + { value: '6h', label: 'Last 6 hours', hours: 6 }, + { value: '24h', label: 'Last 24 hours', hours: 24 }, + { value: '7d', label: 'Last 7 days', hours: 24 * 7 }, + { value: '30d', label: 'Last 30 days', hours: 24 * 30 }, +] + +const CUSTOM_RANGE_VALUE = 'custom' -function getTimeRange(range: string): { start: string; end: string } { +function presetTimeRange(range: string): { start: string; end: string } { const now = new Date() const end = now.toISOString() - const ms: Record = { - '15m': 15 * 60 * 1000, - '1h': 60 * 60 * 1000, - '6h': 6 * 60 * 60 * 1000, - '24h': 24 * 60 * 60 * 1000, - } - const start = new Date(now.getTime() - (ms[range] ?? ms['1h'])).toISOString() + const found = TIME_RANGES.find((r) => r.value === range) + const hours = found?.hours ?? 1 + const start = new Date(now.getTime() - hours * 60 * 60 * 1000).toISOString() return { start, end } } + function HistoryLogLine({ line }: { line: LogSearchLine }) { const d = new Date(line.timestamp) const base = d.toLocaleTimeString('en-US', { @@ -116,7 +120,6 @@ function HistoryLogLine({ line }: { line: LogSearchLine }) { className="whitespace-pre-wrap break-all min-w-0 flex-1" dangerouslySetInnerHTML={{ __html: ansiConverter.toHtml(line.message) }} /> - ) } @@ -132,11 +135,28 @@ export default function HistoryLogViewer({ const [searchText, setSearchText] = useState('') const [debouncedText, setDebouncedText] = useState('') const [timeRange, setTimeRange] = useState('1h') - const [cursor, setCursor] = useState() - const [cursorStack, setCursorStack] = useState([]) + // Custom range, only meaningful when timeRange === CUSTOM_RANGE_VALUE. + // Stored as DateRange (from react-day-picker) so the existing + // DateRangePicker component can both read and write it without translation. + const [customRange, setCustomRange] = useState() + // Older pages loaded on demand. The first page comes from useLogHistory. + // When the user clicks "Load older", we fetch the next page using the + // current oldest line's cursor and append the result here. Filter/range + // changes clear this back to []; see the filterKey effect below. + const [olderPages, setOlderPages] = useState([]) + const [olderCursor, setOlderCursor] = useState() + const [loadingOlder, setLoadingOlder] = useState(false) + const [olderError, setOlderError] = useState() + const [olderExhausted, setOlderExhausted] = useState(false) + const parentRef = useRef(null) - // Track filter key to reset pagination on filter changes - const filterKey = `${selectedEnv}-${selectedService}-${selectedLevels.join(',')}-${debouncedText}-${timeRange}` + // Include the custom range in the filterKey so picking a different window + // resets the older-pages rope. ms keys keep the string compact. + const customRangeKey = + timeRange === CUSTOM_RANGE_VALUE && customRange?.from && customRange?.to + ? `${customRange.from.getTime()}-${customRange.to.getTime()}` + : '' + const filterKey = `${selectedEnv}-${selectedService}-${selectedLevels.join(',')}-${debouncedText}-${timeRange}-${customRangeKey}` const prevFilterKeyRef = useRef(filterKey) // Debounce search text @@ -145,34 +165,71 @@ export default function HistoryLogViewer({ return () => clearTimeout(timer) }, [searchText]) - // Reset pagination when filters change + // Reset the older-pages rope whenever any filter changes — otherwise we'd + // splice unrelated logs from a stale level/text/range into the current view. if (filterKey !== prevFilterKeyRef.current) { prevFilterKeyRef.current = filterKey - if (cursor !== undefined || cursorStack.length > 0) { - setCursor(undefined) - setCursorStack([]) - } + setOlderPages([]) + setOlderCursor(undefined) + setOlderExhausted(false) + setOlderError(undefined) } - // Environments const { data: environments } = useQuery({ ...getEnvironmentsOptions({ path: { project_id: project.id }, }), }) - // Auto-select first environment (store ID as string) useEffect(() => { if (environments?.length && !selectedEnv) { setSelectedEnv(String(environments[0].id)) } }, [environments, selectedEnv]) - // Memoize time range so it only recalculates when timeRange selection changes, - // not on every render (getTimeRange calls new Date() which would create new - // strings each render, causing infinite query invalidation). - // eslint-disable-next-line react-hooks/exhaustive-deps - const { start, end } = useMemo(() => getTimeRange(timeRange), [timeRange]) + // Resolve {start, end} from either the active preset or a custom DateRange. + // For presets we recompute on every timeRange change (presetTimeRange calls + // new Date()), so the memo's job is to avoid creating new strings on every + // render — only when the inputs actually change. When custom is active but + // not yet fully picked (start or end missing) we fall back to a 1h window + // so the query stays well-formed instead of erroring. + const { start, end } = useMemo(() => { + if (timeRange === CUSTOM_RANGE_VALUE) { + if (customRange?.from && customRange?.to) { + return { + start: customRange.from.toISOString(), + end: customRange.to.toISOString(), + } + } + return presetTimeRange('1h') + } + return presetTimeRange(timeRange) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [timeRange, customRangeKey]) + + // Effective range duration for the fulltext-disabled gate. Custom ranges + // get their actual span; presets fall back to the table; unknown defaults + // to 1h so we never lock the user out unexpectedly. + const currentRangeHours = useMemo(() => { + if (timeRange === CUSTOM_RANGE_VALUE) { + if (customRange?.from && customRange?.to) { + const ms = customRange.to.getTime() - customRange.from.getTime() + return Math.max(0, ms / (60 * 60 * 1000)) + } + return 1 + } + return TIME_RANGES.find((r) => r.value === timeRange)?.hours ?? 1 + }, [timeRange, customRange]) + const fulltextDisabled = currentRangeHours > FULLTEXT_MAX_HOURS + + // Hard-clear the search box when a long range is picked so the disabled + // input doesn't quietly send a stale text filter that the server rejects. + useEffect(() => { + if (fulltextDisabled && searchText.length > 0) { + setSearchText('') + } + }, [fulltextDisabled, searchText]) + const { data, isLoading, isFetching, error } = useLogHistory( { projectId: project.id, @@ -181,14 +238,39 @@ export default function HistoryLogViewer({ levels: selectedLevels.length > 0 ? selectedLevels : undefined, envs: selectedEnv ? [selectedEnv] : undefined, services: selectedService ? [selectedService] : undefined, - text: debouncedText || undefined, - cursor, - pageSize: 200, + text: !fulltextDisabled && debouncedText ? debouncedText : undefined, + // Frontend asks for 500 per page; server caps at 2000. + pageSize: 500, }, - !!project.id + !!project.id, ) - const lines = data?.lines ?? [] + // Build the rendered rope in ASC order (oldest at index 0, newest at end) + // so the UI renders terminal-style with newest at the bottom. + // + // Wire shape from the server: each page is already ASC. The first page + // contains the *newest* slab in the time window. Older pages, fetched as + // the user scrolls up, contain progressively earlier slabs. + // + // olderPages is in fetch order: [page2, page3, ...]. page2 covers logs + // immediately older than page1; page3 covers logs immediately older than + // page2; etc. To get a single ASC vec we walk olderPages from the END + // backward (oldest fetched first) then append the first page at the end. + const lines = useMemo(() => { + const firstPage = data?.lines ?? [] + if (olderPages.length === 0) return firstPage + const olderFlat: LogSearchLine[] = [] + for (let i = olderPages.length - 1; i >= 0; i--) { + olderFlat.push(...olderPages[i]) + } + return olderFlat.concat(firstPage) + }, [data?.lines, olderPages]) + + // The cursor we use for "load older" — once we've loaded extra pages, + // the server's last-emitted cursor lives in olderCursor; before that, + // it's data.next_cursor from the first-page response. + const effectiveNextCursor = + olderCursor !== undefined ? olderCursor : data?.next_cursor ?? null const virtualizer = useVirtualizer({ count: lines.length, @@ -197,218 +279,447 @@ export default function HistoryLogViewer({ overscan: 20, }) - // Scroll to bottom when new data loads (logs read oldest→newest, bottom is most recent) - useEffect(() => { - if (lines.length > 0) { - virtualizer.scrollToIndex(lines.length - 1, { align: 'end' }) - } - }, [lines.length, virtualizer]) - const toggleLevel = useCallback((level: LogLevel) => { setSelectedLevels((prev) => - prev.includes(level) ? prev.filter((l) => l !== level) : [...prev, level] + prev.includes(level) ? prev.filter((l) => l !== level) : [...prev, level], ) }, []) - const handleNextPage = useCallback(() => { - if (data?.next_cursor) { - if (cursor) { - setCursorStack((prev) => [...prev, cursor]) + const handleLoadOlder = useCallback(async () => { + if (!effectiveNextCursor || loadingOlder) return + setLoadingOlder(true) + setOlderError(undefined) + try { + const body: Record = { + project_id: project.id, + start_time: start, + end_time: end, + cursor: effectiveNextCursor, + page_size: 500, + } + if (selectedLevels.length > 0) body.levels = selectedLevels + if (selectedEnv) body.envs = [selectedEnv] + if (selectedService) body.services = [selectedService] + if (!fulltextDisabled && debouncedText) body.text = debouncedText + + const res = await fetch('/api/logs/search', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + credentials: 'include', + body: JSON.stringify(body), + }) + if (!res.ok) { + throw new Error(`Server returned ${res.status}`) + } + const json = (await res.json()) as { + lines: LogSearchLine[] + next_cursor: string | null + } + if (json.lines.length === 0) { + setOlderExhausted(true) + } else { + setOlderPages((prev) => [...prev, json.lines]) + } + setOlderCursor(json.next_cursor ?? undefined) + if (!json.next_cursor) { + setOlderExhausted(true) } - setCursor(data.next_cursor) + } catch (e) { + setOlderError(e instanceof Error ? e.message : 'Failed to load older logs') + } finally { + setLoadingOlder(false) } - }, [data?.next_cursor, cursor]) - - const handlePrevPage = useCallback(() => { - setCursorStack((prev) => { - const newStack = [...prev] - const prevCursor = newStack.pop() - setCursor(prevCursor) - return newStack + }, [ + effectiveNextCursor, + loadingOlder, + project.id, + start, + end, + selectedLevels, + selectedEnv, + selectedService, + fulltextDisabled, + debouncedText, + ]) + + const totalMatched = data?.total_scanned ?? 0 + const showingCount = lines.length + const canLoadOlder = !!effectiveNextCursor && !olderExhausted + + // Auto-load older logs when the sentinel enters the scroll viewport. + // Lines are ASC (newest at the bottom) so "older" lives at the TOP of + // the log pane — the sentinel sits above all rendered lines and fires + // when the user has scrolled close to the top of the rope. Stops when: + // - we're already fetching (loadingOlder) + // - the cursor is exhausted (canLoadOlder = false) + // - the previous attempt errored (olderError) — user must click to retry + // The manual button stays mounted so error-retry and visual progress are + // both addressable; it just rarely needs to be clicked in practice. + const sentinelRef = useRef(null) + useEffect(() => { + const sentinel = sentinelRef.current + const root = parentRef.current + if (!sentinel || !root) return + if (!canLoadOlder || loadingOlder || olderError) return + + const observer = new IntersectionObserver( + (entries) => { + for (const entry of entries) { + if (entry.isIntersecting) { + void handleLoadOlder() + } + } + }, + { + root, + // Pre-fire 200px before the sentinel reaches the top of the + // viewport so the next page is already arriving by the time the + // user scrolls to the top of the rope. + rootMargin: '200px 0px 0px 0px', + threshold: 0, + }, + ) + observer.observe(sentinel) + return () => observer.disconnect() + }, [canLoadOlder, loadingOlder, olderError, handleLoadOlder]) + + // Pin scroll to the bottom on the very first page render. After that, the + // user owns the scroll position — we don't auto-snap on subsequent + // updates (especially not on prepended older pages, where the goal is + // exactly the opposite: keep the user's reading position fixed even as + // new rows shift in above). + const initialScrolledRef = useRef(false) + useEffect(() => { + if (initialScrolledRef.current) return + const root = parentRef.current + if (!root) return + if ((data?.lines?.length ?? 0) === 0) return + // Defer to next frame so the virtualizer has measured first-page rows. + const id = requestAnimationFrame(() => { + root.scrollTop = root.scrollHeight + initialScrolledRef.current = true }) - }, []) + return () => cancelAnimationFrame(id) + }, [data?.lines]) + + // Reset the "first scroll" flag when filters change, so a freshly-fetched + // first page anchors to the bottom again. + useEffect(() => { + initialScrolledRef.current = false + }, [filterKey]) + + // Preserve scroll position when older pages prepend at the top. Without + // this, scrollTop stays at its absolute pixel value while N new rows are + // inserted above it — which yanks the user's eye downward to whatever + // line is now at that pixel offset. Capture scrollHeight before the + // append, then bump scrollTop by the delta after the DOM settles. + const prevScrollHeightRef = useRef(null) + const olderPagesCountRef = useRef(0) + useEffect(() => { + const root = parentRef.current + if (!root) return + const prev = olderPagesCountRef.current + const next = olderPages.length + olderPagesCountRef.current = next + + // A fetch began (loadingOlder true) — snapshot scrollHeight. + if (loadingOlder && prevScrollHeightRef.current == null) { + prevScrollHeightRef.current = root.scrollHeight + return + } + // A new older page just landed — adjust scrollTop by the height delta. + if (next > prev && prevScrollHeightRef.current != null) { + const before = prevScrollHeightRef.current + prevScrollHeightRef.current = null + // Wait one frame for the virtualizer to grow the spacer for the + // newly-prepended rows, then offset scrollTop by the delta so the + // user's read position is visually stable. + requestAnimationFrame(() => { + const delta = root.scrollHeight - before + if (delta > 0) { + root.scrollTop = root.scrollTop + delta + } + }) + } + }, [olderPages, loadingOlder]) return ( -
- {/* Filters */} -
- - - + + + + + {environments?.map((env) => ( + + {env.name} ))} - - - - - -
- - setSearchText(e.target.value)} - className="pl-9 w-full" - /> -
-
+ + - {/* Level filter chips */} -
- {LOG_LEVEL_OPTIONS.map((level) => ( - - ))} - {selectedLevels.length > 0 && ( - - + + {/* Level filter chips */} +
+ {LOG_LEVEL_OPTIONS.map((level) => ( + + ))} + {selectedLevels.length > 0 && ( + + )}
-
- {/* Log lines */} -
- {isLoading && !data ? ( -
- + {/* Error state */} + {error && ( + + + + Failed to load logs: {error.message} + + + )} + + {/* Results header */} +
+
+ {isFetching && } + + Showing {showingCount.toLocaleString()} + {totalMatched > showingCount + ? ` of ${totalMatched.toLocaleString()}+` + : ''}{' '} + {showingCount === 1 ? 'log' : 'logs'} +
- ) : lines.length === 0 ? ( -
-
- -

No logs found for the selected filters

-

- Try adjusting the time range or clearing filters -

+
+ + {/* Log lines */} +
+ {isLoading && !data ? ( +
+
-
- ) : ( -
+ ) : lines.length === 0 ? ( +
+
+ +

No logs found for the selected filters

+

+ Try adjusting the time range or clearing filters +

+
+
+ ) : (
- {virtualizer.getVirtualItems().map((virtualRow) => ( -
- -
- ))} + {/* Sentinel ABOVE the virtualizer spacer. ASC ordering means + older logs live at the top, so the auto-load-older trigger + belongs at the top of the rope. IntersectionObserver fires + handleLoadOlder when the sentinel scrolls into view. The + button-shaped content is the manual fallback (error retry, + accessibility). Must be inside parentRef so the observer's + root can detect intersection during scroll. */} +
+ {olderError ? ( + + ) : olderExhausted || !canLoadOlder ? ( + + Beginning of logs in this time range + + ) : loadingOlder ? ( + + + Loading older logs... + + ) : ( + + )} +
+ + {/* Virtualizer spacer: absolute-positioned rows live inside, + laid out top→bottom in ASC timestamp order so the newest + line ends up at the bottom of the scroll viewport. */} +
+ {virtualizer.getVirtualItems().map((virtualRow) => ( +
+ +
+ ))} +
-
- )} + )} +
-
+ ) } diff --git a/web/src/components/runtime-logs/log-line.tsx b/web/src/components/runtime-logs/log-line.tsx index 64c91c0179..89c3bd6a81 100644 --- a/web/src/components/runtime-logs/log-line.tsx +++ b/web/src/components/runtime-logs/log-line.tsx @@ -30,7 +30,7 @@ export const LogLine = memo(function LogLine({ return (
diff --git a/web/src/components/runtime-logs/log-viewer.tsx b/web/src/components/runtime-logs/log-viewer.tsx index f43d1d81a8..57d27ffe9e 100644 --- a/web/src/components/runtime-logs/log-viewer.tsx +++ b/web/src/components/runtime-logs/log-viewer.tsx @@ -6,6 +6,7 @@ import { listContainersOptions, } from '@/api/client/@tanstack/react-query.gen' import { Alert, AlertDescription } from '@/components/ui/alert' +import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' import { Checkbox } from '@/components/ui/checkbox' import { @@ -71,6 +72,20 @@ const MAX_VISIBLE_LOGS = 5000 // in JS memory waiting to render. Trim to the most recent 1000 and surface // the dropped count so the user knows they're sampling, not recording. const MAX_PENDING_BUFFER = 1000 +// Minimum gap between Live-mode flushes. The previous behavior scheduled +// a rAF on every WS frame; on a 1000+ lps firehose the eye perceives a +// blurred wall of replacing text. 33ms (~30Hz) gives the user a perceivable +// batch cadence — fast enough to feel live, slow enough that any one batch +// is visually distinct from the next. +const LIVE_FLUSH_MIN_GAP_MS = 33 +// Window over which we compute the live lines-per-second readout. 5s is +// short enough to react to bursts but long enough that brief lulls don't +// flap the displayed number. +const LPS_WINDOW_MS = 5_000 +// Threshold above which we surface a `sampled` chip — once incoming exceeds +// this, the visible buffer cap will roll lines off faster than the user can +// read, so they should know they're not seeing every line. +const LPS_SAMPLED_THRESHOLD = 60 const INTERVAL_OPTIONS_MS = [5_000, 30_000, 60_000] as const type IntervalMs = (typeof INTERVAL_OPTIONS_MS)[number] @@ -123,16 +138,18 @@ function formatIntervalLabel(ms: IntervalMs): string { function estimateLineHeight(content: string, containerWidth: number) { // Assuming average character width of 8px in monospace font const averageCharWidth = 9 - const lineHeight = 20 // Base height for a single line - const minHeight = 24 // Minimum height to prevent overlap + // text-xs (12px) × leading-snug (1.375) ≈ 16.5px per text row. + const lineHeight = 17 + const minHeight = 18 if (!content || !containerWidth) return minHeight - // Account for padding (py-1 = 8px vertical padding) - const paddingHeight = 8 + // Container is now px-3 (12px each side) and rows are py-0 — no extra + // vertical padding from the row wrapper, so paddingHeight stays 0. + const paddingHeight = 0 // Calculate how many lines this content might wrap into - const effectiveWidth = containerWidth - 32 // Account for container padding (p-4 = 16px each side) + const effectiveWidth = containerWidth - 24 // px-3 = 12px each side const charactersPerLine = Math.max( 1, Math.floor(effectiveWidth / averageCharWidth) @@ -195,15 +212,34 @@ export default function LogViewer({ project }: { project: ProjectResponse }) { // mode (rAF for live, setInterval for interval, never for pause). const pendingLogsRef = useRef([]) const rafHandleRef = useRef(null) + const flushTimeoutRef = useRef | null>(null) + const lastFlushTsRef = useRef(0) const intervalHandleRef = useRef | null>(null) + // Sliding window of WS message arrival timestamps used to compute live + // lines-per-second. Pushed on every enqueueLog; trimmed on demand. Stays + // a ref (not state) because we re-read the count once per second on the + // existing setNow tick — no need to render on every push. + const lpsSamplesRef = useRef([]) + const [lps, setLps] = useState(0) + // Index of the first row in the most recently flushed batch, captured at + // flush time. Rendered rows whose virtual index is >= this get a fade-in + // animation so the user perceives the new batch arriving as motion rather + // than a wall expanding silently. + const lastBatchStartRef = useRef(0) + const [lastBatchStart, setLastBatchStart] = useState(0) + // Set by the Interval polling effect to its current poll function. The + // "Refresh now" button calls this to fire an immediate poll instead of + // waiting for the next tick. Null when not in Interval mode. + const pollNowRef = useRef<(() => void) | null>(null) // Live-mirror of mode/lastInterval so the WS callbacks (created once) can // read the latest value without being re-bound on every change. const modeRef = useRef(mode) - // Track whether the previous WS effect run was paused. When it was, the - // *next* run is "resuming from pause" and should keep the existing visible - // logs instead of wiping them. Genuine source changes (container, env, - // dates, tail, timestamps) still wipe. - const wasPausedRef = useRef(mode.kind === 'pause') + // Signature of the WS effect's *source* params (everything except mode). + // If the new run has the same signature, we're toggling modes and must + // NOT wipe the visible logs — otherwise switching Live → Interval blanks + // the pane until the next tick (5s of empty screen). Source-param + // changes (env, container, dates, tail, timestamps) still wipe. + const sourceSigRef = useRef('') const virtualizer = useVirtualizer({ count: logs.length, getScrollElement: () => parentRef.current, @@ -324,21 +360,32 @@ export default function LogViewer({ project }: { project: ProjectResponse }) { setDroppedSinceFlush(0) droppedSinceFlushRef.current = 0 if (incoming.length === 0) return + lastFlushTsRef.current = Date.now() setLogs((prev) => { const merged = [...prev, ...incoming] - return merged.length > MAX_VISIBLE_LOGS - ? merged.slice(-MAX_VISIBLE_LOGS) - : merged + const trimmed = + merged.length > MAX_VISIBLE_LOGS + ? merged.slice(-MAX_VISIBLE_LOGS) + : merged + // Capture where this batch starts in the trimmed list so the row + // renderer can apply the fade-in animation to just the new rows. + // When we trim, the start is `length - incoming.length`; when we + // don't, it's `prev.length`. + const batchStart = Math.max(0, trimmed.length - incoming.length) + lastBatchStartRef.current = batchStart + setLastBatchStart(batchStart) + return trimmed }) }, []) - // Enqueue a single line. In Live mode we schedule a rAF flush immediately - // so the existing per-frame behavior is preserved. In Pause/Interval modes - // the line just sits in the buffer until the corresponding mechanism drains - // it — *but* the buffer is capped at MAX_PENDING_BUFFER so a high-volume - // stream during a 30s tick can't accumulate megabytes in JS memory. When - // the cap is hit we drop the oldest pending line and bump the dropped - // counter so the user notices they're sampling. + // Enqueue a single line. In Live mode flushes are throttled to at most one + // every LIVE_FLUSH_MIN_GAP_MS so the visible cadence stays eye-paceable + // even on a firehose stream. In Pause/Interval modes the line just sits in + // the buffer until the corresponding mechanism drains it. The buffer is + // capped at MAX_PENDING_BUFFER so a high-volume stream during a 30s tick + // can't accumulate megabytes in JS memory. When the cap is hit we drop + // the oldest pending line and bump the dropped counter so the user + // notices they're sampling. const enqueueLog = useCallback( (line: string) => { const buf = pendingLogsRef.current @@ -348,12 +395,36 @@ export default function LogViewer({ project }: { project: ProjectResponse }) { buf.splice(0, overflow) droppedSinceFlushRef.current += overflow } + // Track WS arrival time for the lps readout. Trimmed lazily on the + // 1Hz tick effect — pushing here is O(1). + lpsSamplesRef.current.push(Date.now()) + if (modeRef.current.kind === 'live') { - if (rafHandleRef.current != null) return - rafHandleRef.current = requestAnimationFrame(() => { - rafHandleRef.current = null - flushPending() - }) + // Already a flush scheduled — let it pick up this line. + if (rafHandleRef.current != null || flushTimeoutRef.current != null) + return + + const now = Date.now() + const sinceLast = now - lastFlushTsRef.current + if (sinceLast >= LIVE_FLUSH_MIN_GAP_MS) { + // Past the gap — flush on the next frame as before. + rafHandleRef.current = requestAnimationFrame(() => { + rafHandleRef.current = null + flushPending() + }) + } else { + // Within the gap — defer the flush until the gap elapses, then + // run on the following animation frame so the DOM mutation + // aligns with paint timing. + const wait = LIVE_FLUSH_MIN_GAP_MS - sinceLast + flushTimeoutRef.current = setTimeout(() => { + flushTimeoutRef.current = null + rafHandleRef.current = requestAnimationFrame(() => { + rafHandleRef.current = null + flushPending() + }) + }, wait) + } } else { // For Pause/Interval, keep counters fresh on a slow cadence — the // dedicated effect below ticks `now` once a second. @@ -366,50 +437,137 @@ export default function LogViewer({ project }: { project: ProjectResponse }) { [flushPending], ) - // Drive the Interval mode flush + countdown. Also drives the 1Hz "now" tick - // so the "Next refresh in 0:03" / buffered counter UIs stay live. + // Drive the Interval mode poll + countdown. Also drives the 1Hz "now" tick + // so the "Next refresh in 0:03" / lps UIs stay live. + // + // Interval mode does NOT use the WebSocket. Instead it polls the + // /api/logs/search endpoint every N seconds and replaces the visible + // buffer with the most recent 500 lines. Snapshot semantics — what you + // see is "the last 500 lines as of N seconds ago," well-defined and + // free of duplicate bursts that an interval-flushed-WS produced. useEffect(() => { - // Always tick "now" at 1Hz so countdowns + buffered counts update. - const nowTimer = setInterval(() => setNow(Date.now()), 1000) + const nowTimer = setInterval(() => { + const t = Date.now() + setNow(t) + const cutoff = t - LPS_WINDOW_MS + const samples = lpsSamplesRef.current + let firstFresh = 0 + while (firstFresh < samples.length && samples[firstFresh] < cutoff) { + firstFresh++ + } + if (firstFresh > 0) samples.splice(0, firstFresh) + const windowSec = LPS_WINDOW_MS / 1000 + setLps(samples.length / windowSec) + }, 1000) if (mode.kind !== 'interval') { setNextTickAt(null) return () => clearInterval(nowTimer) } + if (!selectedTarget) { + return () => clearInterval(nowTimer) + } const ms = mode.ms + // Abort signal so any in-flight fetch when mode/source changes mid-poll + // is cancelled — prevents a slow response from clobbering the buffer + // after the user has switched away. + const abort = new AbortController() + const poll = async () => { + try { + const now = new Date() + const start = new Date(now.getTime() - 60 * 60 * 1000) // last 1h + const body: Record = { + project_id: project.id, + start_time: start.toISOString(), + end_time: now.toISOString(), + envs: [String(selectedTarget)], + page_size: 500, + } + const res = await fetch('/api/logs/search', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + credentials: 'include', + body: JSON.stringify(body), + signal: abort.signal, + }) + if (!res.ok) return + const json = (await res.json()) as { + lines: Array<{ + timestamp: string + level: string + service: string + message: string + }> + } + // Lines arrive ASC (oldest first → newest last), matching the + // viewer's existing "newest at the bottom" assumption. We render + // a single-line representation so the new format slots into the + // existing string-based logs[] buffer without changes. + const formatted = json.lines.map((l) => { + if (showTimestamps) { + return `${l.timestamp} [${l.level}] ${l.service}: ${l.message}` + } + return `[${l.level}] ${l.service}: ${l.message}` + }) + if (!abort.signal.aborted) { + setLogs(formatted) + // Captured-batch animation: treat the whole snapshot as a fresh + // batch so newly-rendered rows fade in. + lastBatchStartRef.current = 0 + setLastBatchStart(0) + } + } catch { + // Ignore network errors; the next tick will retry. + } + } + // Expose the poll function so "Refresh now" can fire an immediate poll + // and reset the countdown without waiting for setInterval's next tick. + pollNowRef.current = () => { + void poll() + setNextTickAt(Date.now() + ms) + } + // Fire immediately so the user sees logs on entry instead of staring + // at a blank pane until the first tick elapses. + void poll() setNextTickAt(Date.now() + ms) intervalHandleRef.current = setInterval(() => { - flushPending() + void poll() setNextTickAt(Date.now() + ms) }, ms) return () => { clearInterval(nowTimer) + abort.abort() + pollNowRef.current = null if (intervalHandleRef.current != null) { clearInterval(intervalHandleRef.current) intervalHandleRef.current = null } } - }, [mode, flushPending]) + }, [mode, project.id, selectedTarget, showTimestamps]) - // When entering Pause from Live, cancel any pending rAF so the in-flight - // batch doesn't surprise-flush after the user just paused. The buffered - // counter immediately reflects whatever was already pending. + // When entering Pause from Live, cancel any pending rAF AND any deferred + // throttle timeout so neither fires after the user just paused. The + // buffered counter immediately reflects whatever was already pending. useEffect(() => { if (mode.kind === 'live') return if (rafHandleRef.current != null) { cancelAnimationFrame(rafHandleRef.current) rafHandleRef.current = null } + if (flushTimeoutRef.current != null) { + clearTimeout(flushTimeoutRef.current) + flushTimeoutRef.current = null + } setBufferedCount(pendingLogsRef.current.length) }, [mode]) - // Manual "Resume" / "Refresh now" — flush whatever is buffered into the - // visible list immediately. Used by both the Pause-mode resume button and - // the Interval-mode "refresh now" button. + // Manual "Refresh now" — Interval mode triggers an immediate poll via the + // ref the Interval effect populated. Other modes drain the rAF buffer. const flushNow = useCallback(() => { - flushPending() - if (modeRef.current.kind === 'interval') { - setNextTickAt(Date.now() + modeRef.current.ms) + if (modeRef.current.kind === 'interval' && pollNowRef.current) { + pollNowRef.current() + return } + flushPending() }, [flushPending]) // WebSocket connection effect @@ -419,24 +577,44 @@ export default function LogViewer({ project }: { project: ProjectResponse }) { // Wait for container to be selected - don't connect without a specific container if (!selectedContainer) return + // Build a source-only signature (everything that affects what stream we + // connect to, excluding mode). If only mode changed since the last run, + // we're toggling Live/Pause/Interval and must keep the visible logs. + const sourceSig = JSON.stringify({ + p: project.id, + e: selectedTarget, + c: selectedContainer, + sd: startDate?.getTime() ?? null, + ed: endDate?.getTime() ?? null, + t: tail, + ts: showTimestamps, + }) + const sourceUnchanged = sourceSigRef.current === sourceSig + sourceSigRef.current = sourceSig + // Pause mode: don't open the WS at all. The cleanup from the previous // effect run (if any) has already closed any existing socket. We keep the // visible logs intact so the user can read what's on screen. if (mode.kind === 'pause') { - wasPausedRef.current = true return } - const resumingFromPause = wasPausedRef.current - wasPausedRef.current = false + // Interval mode: don't open the WS either. A separate polling effect + // calls /api/logs/search every N seconds and replaces the visible buffer + // with a fresh snapshot. Streaming + interval-replace is a worse UX + // (duplicate bursts on every interval reconnect) and wastes a connection. + if (mode.kind === 'interval') { + return + } // Capture the container this effect-instance is tailing. Used by the // socket handlers below to reject any late frames from a previous // socket whose handlers may still fire while React is unwinding. const targetContainer = selectedContainer - // Only wipe state when it's a genuine source change (container/env/dates/ - // tail/timestamps changed). Resuming from Pause keeps what's on screen. - if (!resumingFromPause) { + // Only wipe state when it's a genuine source change. Mode toggles + // (Live → Interval, Pause → Live, etc.) keep the visible buffer so the + // user doesn't stare at a blank pane while waiting for the next tick. + if (!sourceUnchanged) { setLogs([]) pendingLogsRef.current = [] setBufferedCount(0) @@ -447,6 +625,10 @@ export default function LogViewer({ project }: { project: ProjectResponse }) { cancelAnimationFrame(rafHandleRef.current) rafHandleRef.current = null } + if (flushTimeoutRef.current != null) { + clearTimeout(flushTimeoutRef.current) + flushTimeoutRef.current = null + } setRetryCount(0) setErrorMessage('') isConnectingRef.current = false @@ -473,10 +655,11 @@ export default function LogViewer({ project }: { project: ProjectResponse }) { Math.floor(endDate.getTime() / 1000).toString() ) } - // On resume-from-Pause, ask for a small backlog so the user sees a - // smooth catch-up of recent activity rather than a wall of history or - // a confusing dead pause. On a genuine source change, use the full tail. - const effectiveTail = resumingFromPause ? Math.min(tail, 200) : tail + // When the source params didn't change (we're just toggling modes), + // ask for a small backlog so the user sees a smooth catch-up rather + // than a wall of history dumped on top of what's already on screen. + // On a genuine source change use the full tail. + const effectiveTail = sourceUnchanged ? Math.min(tail, 200) : tail if (effectiveTail) { params.append('tail', effectiveTail.toString()) } @@ -609,6 +792,10 @@ export default function LogViewer({ project }: { project: ProjectResponse }) { cancelAnimationFrame(rafHandleRef.current) rafHandleRef.current = null } + if (flushTimeoutRef.current != null) { + clearTimeout(flushTimeoutRef.current) + flushTimeoutRef.current = null + } pendingLogsRef.current = [] // Clear any pending retry timeout @@ -767,16 +954,61 @@ export default function LogViewer({ project }: { project: ProjectResponse }) { setCurrentMatchIndex(-1) }, [searchTerm]) + // Auto-scroll-to-bottom when in follow mode. Two subtleties beyond a naive + // `scrollTop = scrollHeight`: + // + // 1. tanstack-virtual measures rows lazily as they scroll into view; a + // multi-line JSON log can grow by 60+ pixels post-measurement. If we + // only set scrollTop once on the `logs` change, the virtualizer's + // follow-up reflow pushes us above the true bottom and the next + // handleScroll incorrectly disengages follow mode. Schedule the snap + // on rAF so it runs after the virtualizer commit. + // + // 2. Don't pin if we're already at the bottom — avoids cancelling + // momentum-scroll on macOS when a flick happens to land at-bottom. useEffect(() => { - if (autoScroll && parentRef.current) { - parentRef.current.scrollTop = parentRef.current.scrollHeight - } + if (!autoScroll) return + const root = parentRef.current + if (!root) return + const id = requestAnimationFrame(() => { + // Re-check inside the rAF: the user may have scrolled away in the + // intervening tick, in which case we must not yank them back. + if (!autoScroll) return + const target = root.scrollHeight - root.clientHeight + if (root.scrollTop < target) { + root.scrollTop = target + } + }) + return () => cancelAnimationFrame(id) }, [logs, autoScroll]) + // Track whether the most recent scroll event was user-driven. handleScroll + // alone can't tell — content reflow inside the virtualizer dispatches the + // same event shape. We mark `userScrolledRef` on wheel / keydown / touch + // and only let those events disengage follow mode. Re-engagement (from + // false → true) still works on any scroll-to-bottom because that's the + // explicit signal "I want to follow again". + const userScrolledRef = useRef(false) + const markUserIntent = useCallback(() => { + userScrolledRef.current = true + }, []) + const handleScroll = (event: React.UIEvent) => { const { scrollTop, scrollHeight, clientHeight } = event.currentTarget - const isAtBottom = scrollHeight - scrollTop - clientHeight < 1 - setAutoScroll(isAtBottom) + // Tolerance of 8px absorbs sub-pixel rounding plus the ~1–4px gap that + // the virtualizer's post-measurement reflow can leave behind. + const isAtBottom = scrollHeight - scrollTop - clientHeight < 8 + if (isAtBottom) { + // User (or our own snap) is at the bottom — engage follow. + if (!autoScroll) setAutoScroll(true) + userScrolledRef.current = false + return + } + // Not at the bottom. Only disengage if the user actually moved us here — + // otherwise this is a reflow-induced scroll event and we ignore it. + if (userScrolledRef.current && autoScroll) { + setAutoScroll(false) + } } const handleSearch = useCallback((value: string) => { @@ -804,15 +1036,17 @@ export default function LogViewer({ project }: { project: ProjectResponse }) { return (
- {/* Add connection status alerts */} - {connectionStatus === 'connecting' && ( + {/* Connection status alerts — only meaningful in Live mode where we + actually hold a WebSocket. Pause has no socket; Interval polls + HTTP and surfaces errors silently (the next tick retries). */} + {mode.kind === 'live' && connectionStatus === 'connecting' && ( Connecting to log stream... )} - {connectionStatus === 'error' && ( + {mode.kind === 'live' && connectionStatus === 'error' && ( @@ -822,7 +1056,7 @@ export default function LogViewer({ project }: { project: ProjectResponse }) { )} - {connectionStatus === 'permanent_error' && ( + {mode.kind === 'live' && connectionStatus === 'permanent_error' && ( @@ -840,8 +1074,8 @@ export default function LogViewer({ project }: { project: ProjectResponse }) { )} {/* Main Filters */} -
-
+
+
setMemoryMb(e.target.value)} - /> - MiB -
-
+ {/* Swap sub-row — only meaningful when memory is on, so it's + nested as part of the memory section. */} +
+

- Off (default): swap disabled. On: lets the container - page out up to swap-MiB above the memory cap. + Page out N MiB beyond the memory cap.

- {swapEnabled ? ( -
- + setSwapMb(e.target.value)} + onChange={setSwapMb} + presets={SWAP_PRESETS} + unit="MiB" + min={64} + step={64} /> - - MiB total - + {memoryTotal != null && ( +

+ Total memory + swap{' '} + + {memoryTotal} MiB + +

+ )}
- ) : null} + )}
- ) : null} -
+
+ {/* CPU ------------------------------------------------------- */} -
-
-
- -

- Cap CPU at N cores (fractional allowed, e.g. 0.5). -

-
- -
- {cpuEnabled ? ( -
- setCpuCores(e.target.value)} - /> - cores -
- ) : null} -
+ + + {validation ? ( - {validation} + + {validation} + ) : null}
- + +
+ {currentSwapExtra > 0 && !swapEnabled && memoryEnabled && ( + +{currentSwapExtra} MiB swap currently allowed + )} +
@@ -403,3 +427,130 @@ export function EditResourceLimitsDialog({ ) } + +// --------------------------------------------------------------------------- +// Helpers and small components +// --------------------------------------------------------------------------- + +const MEMORY_PRESETS = [256, 512, 1024, 2048, 4096] +const SWAP_PRESETS = [128, 256, 512, 1024] +const CPU_PRESETS = [0.25, 0.5, 1, 2, 4] + +function formatCores(n: number): string { + return n % 1 === 0 ? n.toString() : n.toFixed(2).replace(/\.?0+$/, '') +} + +/** + * One row per cap (Memory, CPU). Replaces the previous bordered card with + * a flat label/toggle pair plus a "Currently: X" chip so the operator can + * see the applied value without enabling the switch. + */ +function LimitSection({ + label, + description, + currentLabel, + enabled, + onToggle, + toggleId, + children, +}: { + label: string + description: string + currentLabel: string + enabled: boolean + onToggle: (v: boolean) => void + toggleId: string + children: React.ReactNode +}) { + return ( +
+
+
+
+ + + now {currentLabel} + +
+

+ {description} +

+
+ +
+ {enabled &&
{children}
} +
+ ) +} + +/** + * Numeric input with a row of small "preset" buttons below. Clicking a + * preset fills the input. Selected preset gets a subtle highlight so the + * operator knows which value they're at. + */ +function PresetInput({ + id, + value, + onChange, + presets, + unit, + min, + step, + decimal = false, +}: { + id: string + value: string + onChange: (v: string) => void + presets: number[] + unit: string + min: number + step: number + decimal?: boolean +}) { + const numeric = Number(value) + return ( +
+
+ onChange(e.target.value)} + /> + {unit} +
+
+ {presets.map((preset) => { + const isActive = Number.isFinite(numeric) && numeric === preset + return ( + + ) + })} +
+
+ ) +} diff --git a/web/src/components/storage/ServiceResourcesPanel.tsx b/web/src/components/storage/ServiceResourcesPanel.tsx index 6796c8ac5d..d17afa15f5 100644 --- a/web/src/components/storage/ServiceResourcesPanel.tsx +++ b/web/src/components/storage/ServiceResourcesPanel.tsx @@ -9,17 +9,10 @@ import type { import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert' import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' -import { - Card, - CardContent, - CardDescription, - CardHeader, - CardTitle, -} from '@/components/ui/card' -import { Progress } from '@/components/ui/progress' +import { Card, CardContent } from '@/components/ui/card' import { Skeleton } from '@/components/ui/skeleton' import { TimeAgo } from '@/components/utils/TimeAgo' -import { formatBytes } from '@/lib/utils' +import { formatBytes, cn } from '@/lib/utils' import { useQuery } from '@tanstack/react-query' import { AlertTriangle, Cpu, MemoryStick, Settings2 } from 'lucide-react' import { useState } from 'react' @@ -31,16 +24,12 @@ interface ServiceResourcesPanelProps { } /** - * Combined runtime + live-stats panel for an external service. - * - * Surfaces three things the operator needs when a database is misbehaving: - * - Container restart count and OOM-killed flag (the smoking gun for - * silent crashes — kernel OOM never reaches the app's logs). - * - Live CPU / memory usage with limit context, polled every 5s. - * - The currently-applied resource caps + a button to change them. + * Compact runtime + live-stats panel for an external service. * - * For cluster topologies, every member is rendered as its own row so a - * promoted-then-demoted replica can be diagnosed in isolation. + * Per-member layout is a single horizontal row with inline CPU/memory meters + * so a standalone service occupies one line and a 3-node cluster occupies + * three. The OOM alert is the only thing that breaks vertical rhythm — it + * earns the room because it's the smoking gun for silent crashes. */ export function ServiceResourcesPanel({ serviceId, @@ -56,60 +45,38 @@ export function ServiceResourcesPanel({ const statsQuery = useQuery({ ...getServiceStatsOptions({ path: { id: serviceId } }), - // 5-second poll is the sweet spot between "feels live" and "doesn't - // hammer the docker socket". The bollard one-shot stats call is - // cheap (single snapshot) so this scales fine for clusters. refetchInterval: 5_000, refetchIntervalInBackground: false, staleTime: 4_000, }) - // The first member's currentLimits is what the dialog seeds from. For - // cluster services we trust the monitor + replicas to share caps — - // every member is created with the same limits in init_cluster. const firstMember = runtimeQuery.data?.members?.[0] const currentLimits = firstMember?.resource_limits + const isLive = statsQuery.isSuccess && !statsQuery.isStale return ( <> - - -
- Resources - - Container runtime, live CPU/memory, and applied limits. - -
- - - - + + {runtimeQuery.isPending ? ( ) : runtimeQuery.isError ? ( - Failed to load runtime info. Showing the most recent - snapshot if available — otherwise try reloading. + Failed to load runtime info. Try reloading. ) : runtimeQuery.data?.members.length === 0 ? ( -

- No containers found for this service yet. Once it starts, - runtime details will appear here. -

+
+

+ No containers yet — runtime details appear once it starts. +

+ setDialogOpen(true)} /> +
) : ( -
- {runtimeQuery.data?.members.map((member) => { +
    + {runtimeQuery.data?.members.map((member, idx) => { const stats = statsQuery.data?.members.find( (m) => m.container_name === member.container_name, ) @@ -118,10 +85,14 @@ export function ServiceResourcesPanel({ key={member.container_name} member={member} stats={stats} + isLive={isLive && idx === 0} + onEditLimits={ + idx === 0 ? () => setDialogOpen(true) : undefined + } /> ) })} -
+ )} @@ -138,61 +109,69 @@ export function ServiceResourcesPanel({ } // --------------------------------------------------------------------------- -// Member row: one container's runtime + stats. A standalone service has a -// single row labeled "standalone"; a cluster has one row per member. +// Member row: identity left, inline meters right, edit-limits trailing on +// the first row. Live pulse rides on the first row's status dot to avoid a +// separate header. OOM alert is full-width because the operator must see it. // --------------------------------------------------------------------------- function MemberRow({ member, stats, + isLive, + onEditLimits, }: { member: ContainerRuntimeInfo stats: ContainerStatsSample | undefined + isLive: boolean + onEditLimits?: () => void }) { const restartCount = member.restart_count ?? 0 const oomKilled = member.oom_killed === true const status = member.status ?? null - - // We only flag a crash loop when restart_count > 0 — most healthy - // containers stay at 0 forever. Any positive number deserves the - // operator's attention even if not currently OOM-killed. const showRestartWarning = restartCount > 0 + return ( -
- {/* Identity row -------------------------------------------------- */} -
-
- {member.container_name} - {member.role} - {status ? ( - - {status} - - ) : ( - missing - )} +
  • +
    + {/* Identity: name • role • status • limits hint */} +
    + + {member.container_name} + + + {member.role} + + + +
    + + {/* Live meters — sit on a single line above lg, stack tight below */} +
    + +
    -
    + + {onEditLimits && ( + + )} +
    + + {/* Started + restart counter — third-tier metadata, only shown when + there's a signal worth surfacing (started_at or non-zero restarts). */} + {(member.started_at || showRestartWarning) && ( +
    {member.started_at ? ( Started ) : null} - - {restartCount} restart{restartCount === 1 ? '' : 's'} - + {showRestartWarning && ( + + {restartCount} restart{restartCount === 1 ? '' : 's'} + + )}
    -
    + )} - {/* OOM banner — high-value diagnostic, always shown when set. */} {oomKilled ? ( @@ -207,29 +186,73 @@ function MemberRow({ ) : null} - . Either raise the memory cap below, or investigate the - workload that pushed past it. + . Either raise the memory cap, or investigate the workload that + pushed past it. ) : null} +
    • + ) +} - {/* Live stats ---------------------------------------------------- */} -
    - - -
    +// Tiny status pill — saves the horizontal space a full Badge eats. When +// `live` is set on a running container the dot pulses, replacing the +// separate "live" indicator the old card header used to show. +function StatusDot({ + status, + live = false, +}: { + status: string | null + live?: boolean +}) { + if (!status) { + return ( + + + missing + + ) + } + const tone = + status === 'running' + ? 'bg-emerald-500' + : status === 'exited' || status === 'dead' + ? 'bg-red-500' + : 'bg-amber-500' + return ( + + + {live && status === 'running' && ( + + )} + + + {status} + + ) +} - {/* Applied limits summary --------------------------------------- */} - -
    +// Compact icon-only "Edit limits" trigger — title attribute carries the +// label since the panel header that used to spell it out is gone. +function EditLimitsButton({ onClick }: { onClick: () => void }) { + return ( + ) } // --------------------------------------------------------------------------- -// Per-metric cells: CPU and memory each get their own progress bar with -// percent-of-limit and absolute usage labels. Falls back to a "—" line -// when stats are unavailable (container not running yet, docker socket -// blip, etc.) so the panel layout stays stable. +// CPU meter: inline value + thin bar. Bar capacity is rebased to the user's +// CPU cap when set, so a 4-of-16-core cap reads "100%" at saturation rather +// than "25%" (which feels like the bar is broken). // --------------------------------------------------------------------------- function CpuMeter({ sample, @@ -238,12 +261,6 @@ function CpuMeter({ sample: ContainerStatsSample | undefined limits: ContainerRuntimeInfo['resource_limits'] }) { - // Docker's `cpu_percent` is computed as - // (cpu_delta / system_delta) * online_cpus * 100 - // so 100% == "saturating every host core". When the operator has set a - // CPU cap (`nano_cpus`), we want the user's mental model to be "% of my - // cap", not "% of the host" — otherwise a container pinned to 4 of 16 - // cores can never read above 25% and the bar feels broken. const hostCpuPercent = sample?.cpu_percent ?? null const onlineCpus = sample?.online_cpus ?? null const capCores = @@ -251,9 +268,6 @@ function CpuMeter({ ? limits.nano_cpus / 1_000_000_000 : null - // Re-base the percent against the cap when one is set. Multiply by - // (host / cap) — if you're capped at 4 of 16 cores, you reach 100% of - // your cap when host_cpu_percent hits 25%. let displayedPercent = hostCpuPercent if ( displayedPercent != null && @@ -264,42 +278,23 @@ function CpuMeter({ displayedPercent = displayedPercent * (onlineCpus / capCores) } - // Cap the bar at 100% even if the raw computation overshoots — Docker's - // formula can read >100% during the first sample window of a freshly - // started container, and the rebase math can also briefly overshoot. const barValue = displayedPercent != null ? Math.min(100, Math.max(0, displayedPercent)) : 0 - // Caption: prefer the cap when set, fall back to host cores. Always - // include the host context when capped so the operator can verify the - // cap is the bound (not coincidence). - const caption = - capCores != null - ? `of ${capCores.toFixed(capCores % 1 === 0 ? 0 : 2)} core${capCores === 1 ? '' : 's'} capped${onlineCpus != null ? ` (host ${onlineCpus})` : ''}` - : onlineCpus != null - ? `of ${onlineCpus} core${onlineCpus === 1 ? '' : 's'}` - : null - return ( -
    -
    -
    - - CPU -
    - - {displayedPercent != null ? `${displayedPercent.toFixed(1)}%` : '—'} - {caption != null ? ( - - {caption} - - ) : null} - -
    - -
    + } + label="CPU" + value={ + displayedPercent != null + ? `${displayedPercent.toFixed(1)}%` + : '—' + } + barPercent={barValue} + tone={barValue >= 90 ? 'danger' : barValue >= 70 ? 'warn' : 'normal'} + /> ) } @@ -316,37 +311,89 @@ function MemoryMeter({ const hasUserLimit = (limits?.memory_mb ?? null) != null const barValue = percent != null ? Math.min(100, Math.max(0, percent)) : 0 + const tone = !hasUserLimit + ? 'normal' + : barValue >= 90 + ? 'danger' + : barValue >= 70 + ? 'warn' + : 'normal' return ( -
    -
    -
    - - Memory -
    - - {usage != null ? formatBytes(usage) : '—'} - {limit != null ? ( - - / {formatBytes(limit)} + } + label="Memory" + value={ + usage != null + ? hasUserLimit && limit != null + ? `${formatBytes(usage)} / ${formatBytes(limit)}` + : formatBytes(usage) + : '—' + } + barPercent={barValue} + tone={tone} + hint={!hasUserLimit && limit != null ? 'unlimited' : undefined} + /> + ) +} + +// Shared inline meter primitive — small icon + label on top, value on right, +// thin bar below. Tone shifts the bar color past 70% / 90%. +function Meter({ + icon, + label, + value, + barPercent, + tone = 'normal', + hint, +}: { + icon: React.ReactNode + label: string + value: string + barPercent: number + tone?: 'normal' | 'warn' | 'danger' + hint?: string +}) { + const barClass = + tone === 'danger' + ? 'bg-red-500' + : tone === 'warn' + ? 'bg-amber-500' + : 'bg-foreground/70' + return ( +
    +
    +
    + {icon} + {label} + {hint && ( + + {hint} - ) : null} + )} +
    + + {value}
    - - {!hasUserLimit && limit != null ? ( - // No user-set cap → Docker reports host RAM as the "limit" and the - // percentage reads "5% of host". Make that explicit so operators - // don't think this is a real bound. -

    - Unlimited — percentage shown is of host RAM, not an applied cap. -

    - ) : null} +
    +
    +
    ) } -function LimitsSummary({ +// Inline applied-limits chip — three small pills max (mem / cpu / swap). +function LimitsInline({ limits, }: { limits: ContainerRuntimeInfo['resource_limits'] @@ -358,55 +405,48 @@ function LimitsSummary({ if (memory == null && nano == null) { return ( -

    - No limits applied — this container runs unconstrained. -

    + + Unlimited + ) } return ( -
    - {memory != null ? ( + + {cpuCores != null && ( - {memory} MiB - {' memory'} - {swap != null && swap > memory ? ` (+${swap - memory} MiB swap)` : ''} + + {cpuCores.toFixed(cpuCores % 1 === 0 ? 0 : 2)} + {' '} + cpu - ) : null} - {cpuCores != null ? ( + )} + {memory != null && ( - - {cpuCores.toFixed(2)} - - {' CPU cores'} + {memory} MiB + {swap != null && swap > memory ? ` (+${swap - memory} swap)` : ''} - ) : null} -
    + )} + ) } -// --------------------------------------------------------------------------- -// Skeleton matching the live row's layout so the page doesn't visibly -// collapse and re-expand when data arrives. Per project guidelines: -// "Skeletons over spinners for content loading". -// --------------------------------------------------------------------------- +// Skeleton matches the new compact row layout. function ResourcesSkeleton() { return ( -
    -
    - - -
    -
    -
    +
    +
    + + + +
    - -
    -
    -
    - +
    + + +
    ) } diff --git a/web/src/globals.css b/web/src/globals.css index 49c035eaaf..1259b5e490 100644 --- a/web/src/globals.css +++ b/web/src/globals.css @@ -411,3 +411,28 @@ body { @apply bg-background text-foreground; } } + +/* Fade-in for freshly-arrived log lines in the live viewer. + Applied only to rows in the most recently flushed batch — older rows + already in the DOM render with no animation cost. 120ms is short enough + to feel responsive but long enough that the eye registers the new row + appearing rather than the wall just getting taller. Respects reduced- + motion preferences. */ +@keyframes log-fade-in { + from { + opacity: 0; + } + to { + opacity: 1; + } +} + +.log-fresh-line { + animation: log-fade-in 120ms ease-out both; +} + +@media (prefers-reduced-motion: reduce) { + .log-fresh-line { + animation: none; + } +} diff --git a/web/src/pages/Storage.tsx b/web/src/pages/Storage.tsx index a13e7ae56a..4b65a7ef3e 100644 --- a/web/src/pages/Storage.tsx +++ b/web/src/pages/Storage.tsx @@ -1,9 +1,14 @@ -import { listServicesOptions } from '@/api/client/@tanstack/react-query.gen' +import { + getServiceRuntimeOptions, + getServiceStatsOptions, + listServicesOptions, +} from '@/api/client/@tanstack/react-query.gen' import { ExternalServiceInfo } from '@/api/client/types.gen' import { listServiceHealthStatuses, type HealthStatus, } from '@/lib/service-health' +import { formatBytes } from '@/lib/utils' import { CreateServiceButton } from '@/components/storage/CreateServiceButton' import { DeleteServiceButton } from '@/components/storage/DeleteServiceButton' import { EditServiceDialog } from '@/components/storage/EditServiceDialog' @@ -20,8 +25,10 @@ import { usePageTitle } from '@/hooks/usePageTitle' import { useQuery } from '@tanstack/react-query' import { ChevronRight, + Cpu, Database, HardDrive, + MemoryStick, Pencil, RefreshCcw, } from 'lucide-react' @@ -68,7 +75,7 @@ export function Storage() { }) useEffect(() => { - setBreadcrumbs([{ label: 'Storage', href: '/storage' }]) + setBreadcrumbs([{ label: 'Databases', href: '/storage' }]) }, [setBreadcrumbs]) // Keyboard shortcut: N to open the create service dropdown @@ -77,7 +84,7 @@ export function Storage() { callback: () => setIsCreateDropdownOpen(true), }) - usePageTitle('Storage') + usePageTitle('Databases') // Render external services content based on loading/error/empty state const renderExternalServicesContent = () => { @@ -162,7 +169,7 @@ export function Storage() {
    -

    Storage

    +

    Databases

    @@ -364,6 +371,9 @@ function ServicesDividerList({
    + {service.status === 'running' && ( + + )} ) } + +/** + * Compact CPU + memory readout (with applied caps) for one running service. + * + * Aggregates across all cluster members — sum of CPU% (rebased against the + * member's CPU cap so a "1 of 4 cores" cap reads 100% at saturation, not + * 25%) and sum of memory bytes / sum of memory caps. + * + * Two queries: stats polls every 10s (live), runtime polls every 60s + * (limits change rarely). The detail page polls faster — 10s is fine for + * "is this database loaded?" from a list view. + */ +function ServiceMetricsCell({ serviceId }: { serviceId: number }) { + const stats = useQuery({ + ...getServiceStatsOptions({ path: { id: serviceId } }), + refetchInterval: 10_000, + refetchIntervalInBackground: false, + staleTime: 8_000, + }) + const runtime = useQuery({ + ...getServiceRuntimeOptions({ path: { id: serviceId } }), + refetchInterval: 60_000, + staleTime: 50_000, + }) + + if ( + stats.isPending || + stats.isError || + !stats.data?.members?.length + ) { + return null + } + + // Rebase each member's CPU% against its cap so the headline number is + // "% of allocated", which is the only mental model that makes sense + // when caps are heterogeneous across cluster members. + const cpuPercent = stats.data.members.reduce((sum, m) => { + const hostPct = m.cpu_percent ?? 0 + const onlineCpus = m.online_cpus ?? null + const memberRuntime = runtime.data?.members.find( + (r) => r.container_name === m.container_name, + ) + const nano = memberRuntime?.resource_limits?.nano_cpus ?? null + const capCores = nano != null && nano > 0 ? nano / 1_000_000_000 : null + if (capCores != null && onlineCpus != null && capCores < onlineCpus) { + return sum + hostPct * (onlineCpus / capCores) + } + return sum + hostPct + }, 0) + + const memBytes = stats.data.members.reduce( + (sum, m) => sum + (m.memory_usage_bytes ?? 0), + 0, + ) + const hasCpu = stats.data.members.some((m) => m.cpu_percent != null) + const hasMem = stats.data.members.some((m) => m.memory_usage_bytes != null) + + // Sum applied caps across members. memory_mb is in MiB; convert to bytes + // so it composes with formatBytes. CPU cap in cores (sum across members). + const memLimitMib = runtime.data?.members.reduce( + (sum, r) => sum + (r.resource_limits?.memory_mb ?? 0), + 0, + ) + const memLimitBytes = + memLimitMib != null && memLimitMib > 0 + ? memLimitMib * 1024 * 1024 + : null + const cpuLimitCores = runtime.data?.members.reduce((sum, r) => { + const nano = r.resource_limits?.nano_cpus ?? 0 + return sum + (nano > 0 ? nano / 1_000_000_000 : 0) + }, 0) + const hasCpuCap = cpuLimitCores != null && cpuLimitCores > 0 + const hasMemCap = memLimitBytes != null && memLimitBytes > 0 + + if (!hasCpu && !hasMem) return null + + return ( +
    + {hasCpu && ( + + + {cpuPercent.toFixed(1)}% + {hasCpuCap && ( + + / {cpuLimitCores!.toFixed(cpuLimitCores! % 1 === 0 ? 0 : 2)}c + + )} + + )} + {hasMem && ( + + + {formatBytes(memBytes, 1)} + {hasMemCap && ( + + / {formatBytes(memLimitBytes!, 0)} + + )} + + )} +
    + ) +} From cf79463f99b96adeb7d46e3ba446193bd807ddf2 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Thu, 7 May 2026 11:56:32 +0200 Subject: [PATCH 2/8] docs(changelog): note runtime logs overhaul + storage UI polish --- CHANGELOG.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53a5edb7be..9f0642c83f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **Custom date range in Log History**: the time-range select gains a `Custom range…` option that opens the shared `DateRangePicker` with HH:MM precision. Picking it seeds the calendar from the previously-active preset for a smooth handoff (no empty starting state). Custom ranges feed `start_time` / `end_time` directly to `/api/logs/search`; the `filterKey` includes the picked window so the auto-loading rope resets when the range changes. The full-text search box auto-disables (with a tooltip) when the active range exceeds the server's `MAX_FULLTEXT_HOURS = 24` cap — covers both the new 7d/30d presets and any custom range over a day, and prevents silent 400s +- **`Last 7 days` and `Last 30 days` time-range presets in Log History**: complement the existing 15m/1h/6h/24h presets for forensic search. Server-side full-text search still capped at 24h via `MAX_FULLTEXT_HOURS`; the UI surfaces this as a disabled search box with tooltip rather than letting the request 400 +- **Live-rate counter and "sampled" chip in the Live log viewer**: status row now reads `Live · X lines · ~Y lps`, computed from a 5-second sliding window of WebSocket frame arrivals sampled on the existing 1Hz tick. When `lps >= 60` an amber `sampled` chip appears, signaling that the visible buffer is rolling faster than a focused user can read — at that rate even the most attentive reader sees less than 10% of arrivals before they roll off the 5,000-line cap. Tuning lives in `LPS_WINDOW_MS` and `LPS_SAMPLED_THRESHOLD` +- **Per-line fade-in animation on freshly arrived log batches**: rows in the most recently flushed batch get a 120ms `opacity 0 → 1` keyframe (`log-fade-in` in `globals.css`) so the eye perceives the new batch arriving as motion rather than a wall expanding silently. Applied via index comparison against `lastBatchStart` — rows already in the DOM render with no animation cost. Respects `prefers-reduced-motion` +- **Auto-loading log history**: the History viewer's chevron pager (which never worked anyway thanks to the cursor bug below) is replaced with an `IntersectionObserver`-driven rope. A sentinel at the top of the scroll container fires `handleLoadOlder()` when the user scrolls within 200px of the top; older pages prepend silently and scroll position is preserved across the prepend. The "Load older" button stays mounted as a fallback for error retry and accessibility but is rarely needed in practice. Pages chain — small first pages auto-fill the viewport - **Resource limits + runtime/stats panel for external services**: managed databases (postgres, postgres_cluster, redis, mongodb, rustfs, s3) can now be capped per-container with optional memory, swap, and CPU limits. Three new endpoints: `GET /external-services/{id}/runtime` (status, restart count, OOM-killed flag, applied caps), `GET .../stats` (one-shot CPU/memory snapshot), and `PATCH .../resources` (persist + live-apply via `docker.update_container`). The frontend gets a Resources card on the service detail page that polls runtime every 30s and stats every 5s, plus an Edit-limits dialog with independent Memory + CPU toggles, an explicit OOM warning when memory caps are enabled, and a per-member apply summary toast (`applied`/`stopped`/`missing`/`failed`/`requires_recreate`). The CPU meter rebases its percent against the cap when one is set ("0.2% of 4 cores capped (host 16)") so the bar is meaningful instead of pinned at host_cores/cap_cores. All `None` fields = unlimited; legacy services with no `resources` block continue to run unconstrained - **Release channels for `temps upgrade`**: new `--channel {stable,beta}` flag picks which release stream the upgrader subscribes to. `stable` (default) only considers non-prerelease tags; `beta` includes both stable and beta releases so a beta host always receives the freshest available version. **CLI-only by design** — there is no env-var fallback, so a user must pass `--channel beta` explicitly to opt into prereleases. `temps upgrade` (no flags) always lands on stable. The `install.sh` curl-pipe installer accepts the same `--channel` flag with identical semantics. Pinning a specific `--version` ignores the channel. The old boolean `--stable` flag is kept as a hidden alias for backward compat with existing scripts - **Runtime-toggleable ClickHouse analytics backend**: operators can now point Temps at a ClickHouse cluster for analytics reads instead of TimescaleDB by setting `TEMPS_CLICKHOUSE_URL` / `TEMPS_CLICKHOUSE_DATABASE` / `TEMPS_CLICKHOUSE_USER` / `TEMPS_CLICKHOUSE_PASSWORD` on `temps serve` — same binary, no rebuild, no cargo feature flag. When all four are set the analytics events plugin (1) applies the embedded CH schema migrations (events / events_5m_mv / sessions, tracked in `_temps_ch_migrations`), (2) spawns the `ChFanoutWorker` on the tokio runtime, and (3) swaps the read-side `Arc` to `ClickHouseEventsBackend`. Postgres remains the system of record; `record_event` writes to PG synchronously and enqueues into a new `events_ch_outbox` table that the worker drains asynchronously via `FOR UPDATE SKIP LOCKED` + `clickhouse::Client::insert()`. CH outage never blocks ingestion; retries are safe via `ReplacingMergeTree(_version)` dedupe keyed on `event_id`. Default behavior unchanged when env vars are unset (single-binary PG-only). All 12 `query_*` methods translate the `*Spec` value-types into CH dialect (count() / uniq() per aggregation level, toStartOfInterval + WITH FILL for gap-fill, FINAL on every read for ReplacingMergeTree correctness); property breakdowns and timelines group on plain CH columns (geo fields denormalized at fan-out time so country/region/city work without a cross-database join); the multi-project dashboard runs three CH queries with in-memory sparkline densification mirroring the Postgres impl. Worker also runs an hourly retention sweep (delivered outbox rows older than 7 days are deleted) and a 5-minute dead-letter scan (warns when rows exceed `max_attempts` so operators can investigate). Live testcontainer integration test against `clickhouse/clickhouse-server:24.8` validates migrations, row mapping, and every implemented query against a real CH server; skips gracefully without Docker (per CLAUDE.md). See [ADR-012](docs/adr/012-clickhouse-analytics-backend.md) for the bring-your-own design rationale and the operator runbook in [docs/howto/enable-clickhouse-analytics](docs/howto/enable-clickhouse-analytics/page.mdx) for the full enable/verify/rollback flow including documented divergences (HLL approximation, self-referral filter on referrer_hostname is PG-only, the `Analytics` trait stays on PG since visitor/session queries are mutable and not events-shaped) @@ -16,12 +21,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Centralized sandbox identifiers**: new `crates/temps-agents/src/sandbox/user.rs` module defines `SANDBOX_USER`, `SANDBOX_GROUP`, `SANDBOX_CHOWN`, `SANDBOX_HOME`, and `SANDBOX_WORK_DIR` constants. Every load-bearing `/home/temps`, `temps:temps`, and `/workspace` literal in the Dockerfile generator, mount logic, chown calls, and Claude projects-dir derivation now flows through these constants — single source of truth for the sandbox identity model ### Changed +- **Log History flipped to chronological (ASC) ordering**: results now arrive oldest-first / newest-last in both the API response and the UI render. The first page anchors to the bottom on initial load (most recent visible by default) and "load older" prepends at the top, matching `journalctl` / `tail -f` orientation rather than the previous "newest at top" snapshot view. This is a deliberate symmetry pass — both Live and History tabs now read downward as time progresses, eliminating the cross-tab cognitive flip +- **Live viewer "Every Ns" mode is now an HTTP poll, not a throttled WebSocket**: `Pause | Live | Every 5s/30s/60s` had three increasingly buggy interactions with the streaming WS (blank pane on entry, duplicate bursts on toggle, stale `Connecting…` banner). Interval mode now skips the WebSocket entirely and polls `/api/logs/search` with `page_size=500`, replacing the visible buffer with a fresh snapshot. Fires immediately on entry (no more 5s of empty screen) and reuses the existing "Refresh now" button to trigger an off-cycle poll. Live and Pause modes unchanged — Live still uses the WebSocket with its rAF flush pump, Pause still freezes the buffer with the socket closed +- **Live viewer flush rate throttled to ~30Hz**: `LIVE_FLUSH_MIN_GAP_MS = 33` gates how often a buffered batch is committed to React state. Previously every WebSocket frame scheduled a `requestAnimationFrame` flush; on a 1500 lps firehose the eye perceived a blurred wall of replacing text. The throttle keeps the cadence eye-paceable on any incoming rate while remaining imperceptibly delayed on slow streams. Cleanup paths cover the new deferred-flush timeout in Pause-transition / source-change / unmount +- **Log viewer mode toggles preserve the visible buffer**: switching Live ↔ Pause ↔ Interval no longer wipes the on-screen logs. Source-only signature (env, container, dates, tail, timestamps — everything except mode) is captured in a ref and compared on every WS effect run; only genuine source changes wipe. Eliminates the "blank pane for 5s after picking Every 5s" UX regression that the previous "wipe on any non-resume-from-Pause run" gate produced +- **Log viewer auto-follow tolerance bumped from `< 1px` to `< 8px`**, plus user-intent gating: scrolling away from the bottom no longer disengages follow on virtualizer-reflow-induced scroll events (multi-line JSON logs grow ~40px after the virtualizer measures them, which used to flip `autoScroll` off uninvited). Wheel / touch / keyboard events mark intent; reflow events don't. Auto-scroll now runs in `requestAnimationFrame` so the snap catches up to the post-measurement scrollHeight in the same paint cycle +- **Log viewer density tightened end-to-end**: log row line-height `leading-relaxed → leading-snug` with `py-0` (was `py-0.5`); toolbar wrapper `p-4 space-y-4 → px-3 py-2 space-y-2`; log pane padding `p-4 → px-3 py-2`; pane height switched from a fixed `h-[600px]` to `h-[calc(100vh-280px)] min-h-[300px]` so the rope fills available viewport height. `estimateLineHeight` updated to match the new measurements. On a 1080px viewport the visible log count roughly doubles +- **"Storage" surface renamed to "Databases"** everywhere user-facing: page title, breadcrumb, navigation label. URL stays `/storage` for back-compat with bookmarks and deep-links from docs. The page now also shows a compact CPU/memory readout per running service in the list, polled every 10s, so operators can spot a misbehaving database from the index page without drilling in. Per-member layout in `ServiceResourcesPanel` collapsed to a single horizontal row with inline meters and a live-indicator dot when stats are fresh +- **`EditResourceLimitsDialog` swap input semantics fixed**: the field labeled "Swap" now means "extra swap above memory" (in MiB), not Docker's raw `memory_swap` total. The UI translates to `memory + swap` at submit time, so the API contract is unchanged but the input matches its label. Existing services with `swap == memory` (the previous "swap disabled" sentinel) round-trip cleanly to "extra swap = 0" in the form +- **Command palette icons disambiguated for log entries**: `Logs` (live runtime), `Log History` (search), and `Request Logs` (HTTP traffic) all rendered with the same `ScrollText` glyph, which made search-by-icon useless. Now: `ScrollText` for live runtime, `History` for the History deep-link, `Network` for HTTP request logs (different feature entirely) - **Integration-tests CI timeout raised from 45m → 90m**: gives the postgres-upgrades matrix group headroom on cold runners where pg17/pg18 image pulls land alongside several serial orchestrator tests. Common-path runs are unaffected; the timeout only kicks in for true outliers - **Workspace mount path moves from `/workspace` to `/home/temps/workspace`**: keeps the working directory under the sandbox user's home, eliminating the cross-tree `chown` and aligning with how AI CLIs (Claude, Codex) resolve project roots. Claude's `claude_projects_dir` now derives from `SANDBOX_WORK_DIR.replace('/', '-')` so resume keys stay consistent with the new path - **`AuthFlavor::seed_path` is now `seed_path_rel` + `seed_path()` helper**: paths are stored relative to `SANDBOX_HOME` and joined at use-time, so renaming the home directory is a one-constant change instead of a grep-and-edit across every flavor - **`get_temps_api_url` is async**: now reads platform settings to surface the configured `external_url`. `WorkspacePlugin` plumbs `Arc` into `MessageExecutor` so the resolver is reachable from the session-write path ### Fixed +- **Log History pagination was emitted but never honored**: `archive_search` returned a `next_cursor` in the response but the search service ignored `filter.cursor` on the way back in, so clicking the next-page chevron returned the same first page. Cursor is now parsed (`:`), narrows the effective `end_time`, and a strict `< cursor.ts` check inside the chunk scan prevents the boundary line from being returned twice. Pagination tests previously asserted only `lines.length <= page_size`, which couldn't have caught this — replaced with a real test that fetches page 1, sends its cursor as page 2's request, and verifies both no-overlap and strictly-older timestamps +- **Log History silently truncated wide time windows to the OLDEST 200 lines**: `archive_search` ran chunks in `find_chunks` order (oldest-first by insertion), accumulated matches into an unbounded `Vec`, then `break`'d the chunk loop the moment `all_matches.len() > page_size`. Result: a "Last 6 hours" query returned roughly the same lines as "Last 1 hour" because the scanner bailed before reading any chunk past the start of the window — and the lines it did return were the oldest in the window, not the newest. Replaced with a bounded `BinaryHeap>` of size `page_size + 1`. Memory stays at O(K) regardless of total matches; time is O(N log K). All chunks are now scanned. Regression test seeds two batches at -3h30m and -30m, asserts that "Last 1h" returns 5 lines and "Last 4h" returns all 10 — would have failed under the old early-exit +- **`total_scanned` was a misleading raw byte count, not a match count**: the field was incremented before the time/level/service/text filter ran, so a query showing "200 logs (344 scanned)" implied 144 lines were filtered out by the user's level/text predicates. In reality most of those 144 were just chunk bleed-through outside the requested time window — chunk files store many minutes of logs and the scanner reads the whole file then filters. Counter now increments only after every filter passes, so the displayed number actually reflects "matches in window" the way users expect +- **Log search `page_size` cap raised from 500 → 2000**: the previous undocumented hardcoded `min(page_size, 500)` silently clamped any frontend request above 500 lines, which the History viewer hit on its very first page once the default frontend page size became 500. Cap moved to 2000 with a comment explaining the bound (memory-safe given the `MAX_FULLTEXT_HOURS` ceiling). Frontend default is 500; "Load older" appends 500 more +- **Log viewer follow mode silently disengaged on every multi-line JSON log**: `handleScroll` checked `scrollHeight - scrollTop - clientHeight < 1`, but tanstack-virtual measures rows lazily as they scroll into view; a multi-line JSON log can grow by 60+ pixels post-measurement. Each measurement-induced reflow dispatched a scroll event with `isAtBottom === false`, which flipped `autoScroll` off uninvited. Tolerance bumped to 8px and a `userScrolledRef` gate distinguishes user-initiated scrolls (wheel, touch, keyboard) from reflow events — only the former can disengage follow. The auto-scroll-to-bottom effect now runs in `requestAnimationFrame` so it catches up to the virtualizer's post-measurement `scrollHeight` in the same paint cycle +- **Live mode showed "Connecting to log stream…" banner in non-Live modes**: `connectionStatus` lingered at `'connecting'` after the WS effect early-returned in Pause / Interval mode. Banner gating updated to `mode.kind === 'live' && connectionStatus === 'connecting'` (and same for the error / permanent-error variants). Same fix applied to the `opacity-50` class on the log pane - **Stale skill content tests**: updated to match current `temps-cli/SKILL.md` headings so the suite stops failing on docs drift - **MFA verification left users stranded on the login page**: `verify_mfa_challenge` built a `HeaderMap` containing the new `session=…` cookie via `create_session_cookie`, then called `response_headers.insert(SET_COOKIE, mfa_clear_cookie)` to expire the temporary `mfa_session` cookie. `HeaderMap::insert` replaces all values for the key, so the response went out with only `mfa_session=; Max-Age=0` and no real session — the immediate `/user/me` refetch returned 401, and `ProtectedLayout` bounced the user back to `` even though the toast had already announced "MFA verified successfully". Fixed by switching to `append`, plus a regression test that mirrors the handler's exact header merge and asserts both `Set-Cookie` entries survive From 1dab7c0151c1f0bf23c5415eaa349a0ee677e2b9 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Thu, 7 May 2026 12:04:59 +0200 Subject: [PATCH 3/8] fix(logs): satisfy clippy::unnecessary_sort_by on chunk + result reorder Both reverse-comparator sort_by calls in archive_search rewritten as sort_by_key with std::cmp::Reverse. Behavior identical; clippy is happy under -D warnings (which CI uses). --- crates/temps-log-aggregator/src/services/search.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/temps-log-aggregator/src/services/search.rs b/crates/temps-log-aggregator/src/services/search.rs index 5d77506ad3..334950ca90 100644 --- a/crates/temps-log-aggregator/src/services/search.rs +++ b/crates/temps-log-aggregator/src/services/search.rs @@ -210,7 +210,7 @@ impl LogSearchService { // Process chunks newest-first so the bounded heap converges quickly: // the first chunks we read already hold the most recent lines, and // the heap rejects older lines once full. - chunks.sort_by(|a, b| b.ended_at.cmp(&a.ended_at)); + chunks.sort_by_key(|c| Reverse(c.ended_at)); debug!( project_id = %filter.project_id, @@ -336,7 +336,7 @@ impl LogSearchService { // can render terminal-style with newest at the bottom. let mut newest_first: Vec = heap.into_iter().map(|Reverse(e)| e.line).collect(); - newest_first.sort_by(|a, b| b.timestamp.cmp(&a.timestamp)); + newest_first.sort_by_key(|l| Reverse(l.timestamp)); let has_more = newest_first.len() > page_size as usize; newest_first.truncate(page_size as usize); From 60705de8e0524f64d09ed16b154c4b15c83de6b1 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Thu, 7 May 2026 12:16:20 +0200 Subject: [PATCH 4/8] feat(workspace,deployer): preview password encryption + reboot-safe Docker secrets path --- crates/temps-deployer/src/docker.rs | 186 ++++++++++++++++-- .../temps-entities/src/workspace_sessions.rs | 19 +- ...dd_workspace_preview_password_encrypted.rs | 55 ++++++ crates/temps-migrations/src/migration/mod.rs | 2 + crates/temps-proxy/src/preview_auth.rs | 1 + .../temps-workspace/src/handlers/sessions.rs | 61 +++++- crates/temps-workspace/src/plugin.rs | 10 +- .../src/services/workspace_service.rs | 87 +++++++- web/src/api/client/types.gen.ts | 10 +- .../workspace/SessionPreviewCard.tsx | 161 +++++++-------- .../workspace/SessionPreviewPopover.tsx | 174 ++++++++++++++++ .../components/workspace/SessionTerminal.tsx | 26 ++- .../components/workspace/WorkspacePage.tsx | 6 + 13 files changed, 687 insertions(+), 111 deletions(-) create mode 100644 crates/temps-migrations/src/migration/m20260507_000001_add_workspace_preview_password_encrypted.rs create mode 100644 web/src/components/workspace/SessionPreviewPopover.tsx diff --git a/crates/temps-deployer/src/docker.rs b/crates/temps-deployer/src/docker.rs index 8923cd48a3..97d54d2e52 100644 --- a/crates/temps-deployer/src/docker.rs +++ b/crates/temps-deployer/src/docker.rs @@ -57,6 +57,14 @@ pub struct DockerRuntime { /// Wrapped in `Option` so non-overlay deployers (e.g. tests) can /// skip the wiring entirely. overlay_peers: Option>>>, + /// Root directory on the host where per-container secret files are + /// materialized for bind-mounting into `/run/secrets`. Defaults to + /// `$TEMPS_DATA_DIR/secrets` (falling back to `$HOME/.temps/secrets` + /// and finally `./.temps/secrets`) so secret mounts SURVIVE A HOST + /// REBOOT — historically this lived under `std::env::temp_dir()`, + /// which is tmpfs on most Linux distros and got wiped on every + /// reboot, forcing a redeploy. Override via [`Self::with_secrets_root`]. + secrets_root: PathBuf, } /// Map a POSIX exit code (>= 128 means "killed by signal N - 128") to a human @@ -124,13 +132,13 @@ impl DockerRuntime { /// into `/run/secrets`. Lives outside `TempDir` because Docker keeps the /// directory open for the container lifetime; cleanup happens in /// `remove_container`. + /// + /// Lives under `secrets_root` (defaults to `$TEMPS_DATA_DIR/secrets`) + /// rather than `/tmp` so the bind mount survives a host reboot — + /// `/tmp` is tmpfs on most Linux distros and gets wiped, leaving + /// containers with empty `/run/secrets` until the next redeploy. fn secrets_host_dir(&self, container_name: &str) -> PathBuf { - // Use std::env::temp_dir() so the path matches what the Docker daemon - // can see — both daemon and us run on the same host in the local - // (non-remote) deployer path. - std::env::temp_dir() - .join("temps-secrets") - .join(container_name) + self.secrets_root.join(container_name) } /// Resolves the numeric (uid, gid) that the container will run as, @@ -179,6 +187,25 @@ impl DockerRuntime { } pub fn new(docker: Arc, use_buildkit: bool, network_name: String) -> Self { + let secrets_root = default_secrets_root(); + // Best-effort: ensure the root exists with restrictive perms so the + // first deploy after a fresh install doesn't race with the per-container + // mkdir. Failures are logged and not fatal — the per-deploy code path + // creates the leaf directory regardless. + if let Err(e) = std::fs::create_dir_all(&secrets_root) { + warn!( + "Failed to pre-create secrets root {}: {} (will retry per-deploy)", + secrets_root.display(), + e + ); + } else { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = + std::fs::set_permissions(&secrets_root, std::fs::Permissions::from_mode(0o700)); + } + } Self { docker, use_buildkit, @@ -188,9 +215,17 @@ impl DockerRuntime { dns_servers: Vec::new(), overlay_dns_slot: None, overlay_peers: None, + secrets_root, } } + /// Override the secrets host root. Tests use this to scope writes to + /// a temp dir; the agent/serve paths leave it at the default. + pub fn with_secrets_root(mut self, root: impl Into) -> Self { + self.secrets_root = root.into(); + self + } + /// Read the per-node Hickory resolver IP from a shared slot /// populated by the agent's `network_sync` loop after the overlay /// bridge is up. Required for app containers to resolve @@ -1286,9 +1321,14 @@ impl ContainerDeployer for DockerRuntime { // and files are visible from container start (no race with start). // // Trade-off vs tmpfs: plaintext lives on the host filesystem under - // SecretsHostDir until the container is removed. The directory is - // mode 0700 root-owned; individual files are mode 0400. Cleanup is - // handled in `remove_container`. + // `secrets_root` ($TEMPS_DATA_DIR/secrets by default) until the + // container is removed. We deliberately use a persistent path + // rather than `std::env::temp_dir()` so the bind mount survives a + // host reboot — `/tmp` is tmpfs on most Linux distros, and the + // mount would otherwise point at an empty directory after a + // restart, forcing a redeploy. The directory is mode 0700 + // root-owned; individual files are mode 0400. Cleanup is handled + // in `remove_container`. let secrets_bind = if request.secrets.is_empty() { None } else { @@ -1889,6 +1929,31 @@ impl ContainerRuntime for DockerRuntime { } } +/// Resolve the persistent root directory for materialized secret files. +/// +/// Order of preference, mirroring the rest of the codebase +/// (`temps-sandbox`, agent session storage): +/// 1. `$TEMPS_DATA_DIR/secrets` +/// 2. `$HOME/.temps/secrets` +/// 3. `./.temps/secrets` (last-resort fallback for headless container +/// runtimes where neither env var is set) +/// +/// Crucially, NONE of these resolve to `std::env::temp_dir()`. On most +/// Linux distros `/tmp` is mounted as tmpfs and wiped at boot, which +/// caused container `/run/secrets` mounts to point at empty/missing +/// directories after a host reboot and forced a redeploy. +fn default_secrets_root() -> PathBuf { + let base = std::env::var("TEMPS_DATA_DIR") + .map(PathBuf::from) + .unwrap_or_else(|_| { + std::env::var("HOME") + .map(PathBuf::from) + .unwrap_or_else(|_| PathBuf::from(".")) + .join(".temps") + }); + base.join("secrets") +} + /// Writes secrets as files into a per-container host directory for Docker /// to bind-mount into `/run/secrets`. The directory is created (or recreated) /// fresh on each call so stale entries from a previous deployment of the same @@ -1910,10 +1975,71 @@ fn write_secrets_to_host_dir( use std::fs; use std::io::Write; + // Ensure the parent (secrets root) exists. The runtime's `new()` already + // best-effort creates it, but a hostile chmod or fresh install between + // restarts could have left it missing — recreate so we don't fail with + // ENOENT. We deliberately do NOT chmod the parent here: that's the + // runtime's job, and on shared hosts the parent may be intentionally + // owned by a different user. + if let Some(parent) = dir.parent() { + fs::create_dir_all(parent)?; + } + // Recreate fresh — wipes any stale files from a previous container with // the same name (rolling deploys, retries after failure, etc.). + // + // After a host reboot we may inherit a leaf directory whose contents + // were chowned to the previous container's USER (e.g. nonroot uid + // 65532) while the parent is owned by the temps user. `remove_dir_all` + // on such a tree fails with EACCES because we can't unlink files we + // don't own inside a directory we *do* own (sticky-bit semantics on + // some filesystems). Reset the leaf's perms to 0700 owned-by-us first + // so the recursive remove can succeed. if dir.exists() { - fs::remove_dir_all(dir)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + // Best-effort: if this chmod fails (we don't own the dir), the + // remove below will still surface the real error. + let _ = fs::set_permissions(dir, fs::Permissions::from_mode(0o700)); + } + if let Err(e) = fs::remove_dir_all(dir) { + // If the leaf itself is owned by another uid (e.g. previous + // container's USER) and we can't traverse into it, fall back + // to renaming it aside so the deploy can proceed. The orphan + // is logged for operator cleanup; it's not in /tmp anymore so + // it won't auto-vanish on reboot, but it's also not blocking + // the deploy. + if e.kind() == std::io::ErrorKind::PermissionDenied { + let orphan = dir.with_extension(format!( + "orphan-{}", + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0) + )); + fs::rename(dir, &orphan).map_err(|rename_err| { + std::io::Error::new( + rename_err.kind(), + format!( + "failed to clear stale secrets dir {} \ + (rename to {} also failed: {}; original error: {})", + dir.display(), + orphan.display(), + rename_err, + e + ), + ) + })?; + warn!( + "Renamed unowned stale secrets dir to {} so deploy could proceed; \ + please remove manually", + orphan.display() + ); + } else { + return Err(e); + } + } } fs::create_dir_all(dir)?; #[cfg(unix)] @@ -1987,8 +2113,8 @@ fn write_secrets_to_host_dir( ); // World-readable fallback. Parent dir already restricts // access on the host side (only the Temps user can traverse - // into /tmp/temps-secrets); these bits are what the - // container sees. + // into the secrets root under $TEMPS_DATA_DIR/secrets); + // these bits are what the container sees. fs::set_permissions(dir, fs::Permissions::from_mode(0o755))?; for key in secrets.keys() { fs::set_permissions(dir.join(key), fs::Permissions::from_mode(0o444))?; @@ -2108,6 +2234,42 @@ mod docker_tests { assert!(dir.join("NEW").exists()); } + #[test] + fn test_default_secrets_root_uses_temps_data_dir() { + // Smoke test the env precedence. We can't unset HOME without + // breaking other tests, so verify TEMPS_DATA_DIR wins when set. + let tmp = TempDir::new().unwrap(); + let prev = std::env::var("TEMPS_DATA_DIR").ok(); + std::env::set_var("TEMPS_DATA_DIR", tmp.path()); + let root = default_secrets_root(); + assert_eq!(root, tmp.path().join("secrets")); + // Critically, this must NOT live under /tmp (unless TEMPS_DATA_DIR + // points there explicitly) — that was the bug we just fixed. + match prev { + Some(v) => std::env::set_var("TEMPS_DATA_DIR", v), + None => std::env::remove_var("TEMPS_DATA_DIR"), + } + } + + #[test] + fn test_default_secrets_root_is_persistent_path() { + // Without TEMPS_DATA_DIR, the resolved root must live under + // $HOME/.temps/secrets — i.e. NOT under std::env::temp_dir(), + // which is tmpfs on most Linux distros and was the source of + // the post-reboot redeploy regression. + let prev_data = std::env::var("TEMPS_DATA_DIR").ok(); + std::env::remove_var("TEMPS_DATA_DIR"); + let root = default_secrets_root(); + assert!( + !root.starts_with(std::env::temp_dir()), + "secrets root unexpectedly resolved to a tmpfs path: {}", + root.display() + ); + if let Some(v) = prev_data { + std::env::set_var("TEMPS_DATA_DIR", v); + } + } + #[test] fn test_write_secrets_to_host_dir_rejects_path_traversal() { let tmp = TempDir::new().unwrap(); diff --git a/crates/temps-entities/src/workspace_sessions.rs b/crates/temps-entities/src/workspace_sessions.rs index 830746e08d..bb3df387cb 100644 --- a/crates/temps-entities/src/workspace_sessions.rs +++ b/crates/temps-entities/src/workspace_sessions.rs @@ -58,13 +58,22 @@ pub struct Model { #[sea_orm(column_type = "JsonBinary")] pub mcp_servers_config: Option, /// Argon2 hash of the per-session preview password. Enforced by the - /// host-side Pingora before forwarding to the preview gateway. The - /// plaintext is only returned to the caller ONCE at session creation - /// (or on regenerate) and is never stored. + /// host-side Pingora before forwarding to the preview gateway. Kept + /// around for backward compatibility with sessions whose + /// `preview_password_encrypted` was never populated (those predate + /// reversible storage); for new writes we populate both columns so + /// the verify path keeps using argon2 while the UI can decrypt the + /// plaintext for display. pub preview_password_hash: Option, + /// AES-256-GCM ciphertext of the plaintext preview password, encoded + /// as base64 by `EncryptionService`. Populated on session create and + /// on regenerate. When `Some`, the API returns the decrypted plaintext + /// in `preview_password` on every read so users no longer need to + /// regenerate to see their own password. + pub preview_password_encrypted: Option, /// Last 4 chars of the plaintext password, surfaced in the UI so users - /// can tell which password they're looking at after the show-once - /// reveal is gone. Not sensitive on its own. + /// can tell which password they're looking at without expanding the + /// reveal. Not sensitive on its own. pub preview_password_hint: Option, /// Per-session idle timeout in minutes. When null, the server-wide /// default (60min) applies. Sessions idle longer than this are marked diff --git a/crates/temps-migrations/src/migration/m20260507_000001_add_workspace_preview_password_encrypted.rs b/crates/temps-migrations/src/migration/m20260507_000001_add_workspace_preview_password_encrypted.rs new file mode 100644 index 0000000000..e7469c95e3 --- /dev/null +++ b/crates/temps-migrations/src/migration/m20260507_000001_add_workspace_preview_password_encrypted.rs @@ -0,0 +1,55 @@ +//! Add `preview_password_encrypted` to `workspace_sessions`. +//! +//! Previously, the per-session preview password was only stored as an +//! argon2 PHC hash plus a 4-char hint. The plaintext was returned exactly +//! once at create/regenerate and was unrecoverable afterwards. +//! +//! This column adds an AES-256-GCM ciphertext of the plaintext (using the +//! platform `EncryptionService`) so the password can be returned by +//! subsequent `GET /sessions` reads without forcing the user to regenerate. +//! The argon2 hash is kept around so existing sessions whose plaintext was +//! never persisted continue to validate at the preview gateway until the +//! user regenerates them — at which point the new code populates both +//! columns. + +use sea_orm_migration::prelude::*; + +#[derive(DeriveMigrationName)] +pub struct Migration; + +#[async_trait::async_trait] +impl MigrationTrait for Migration { + async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { + manager + .alter_table( + Table::alter() + .table(WorkspaceSessions::Table) + .add_column( + ColumnDef::new(WorkspaceSessions::PreviewPasswordEncrypted) + .text() + .null(), + ) + .to_owned(), + ) + .await?; + Ok(()) + } + + async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { + manager + .alter_table( + Table::alter() + .table(WorkspaceSessions::Table) + .drop_column(WorkspaceSessions::PreviewPasswordEncrypted) + .to_owned(), + ) + .await?; + Ok(()) + } +} + +#[derive(DeriveIden)] +enum WorkspaceSessions { + Table, + PreviewPasswordEncrypted, +} diff --git a/crates/temps-migrations/src/migration/mod.rs b/crates/temps-migrations/src/migration/mod.rs index 67f7df3c5c..06a68b11d7 100644 --- a/crates/temps-migrations/src/migration/mod.rs +++ b/crates/temps-migrations/src/migration/mod.rs @@ -84,6 +84,7 @@ mod m20260501_000001_add_gitlab_webhook_to_projects; mod m20260502_000001_add_observe_correlation; mod m20260504_000001_widen_backup_size_and_heartbeat; mod m20260505_000001_create_events_ch_outbox; +mod m20260507_000001_add_workspace_preview_password_encrypted; pub struct Migrator; @@ -171,6 +172,7 @@ impl MigratorTrait for Migrator { Box::new(m20260502_000001_add_observe_correlation::Migration), Box::new(m20260504_000001_widen_backup_size_and_heartbeat::Migration), Box::new(m20260505_000001_create_events_ch_outbox::Migration), + Box::new(m20260507_000001_add_workspace_preview_password_encrypted::Migration), ] } } diff --git a/crates/temps-proxy/src/preview_auth.rs b/crates/temps-proxy/src/preview_auth.rs index 2da2049e99..c1254fccba 100644 --- a/crates/temps-proxy/src/preview_auth.rs +++ b/crates/temps-proxy/src/preview_auth.rs @@ -874,6 +874,7 @@ mod tests { files_changed: 0, metadata: None, preview_password_hash: None, + preview_password_encrypted: None, preview_password_hint: None, idle_timeout_minutes: None, cpu_milli: None, diff --git a/crates/temps-workspace/src/handlers/sessions.rs b/crates/temps-workspace/src/handlers/sessions.rs index 0be4686079..a46c7d35b2 100644 --- a/crates/temps-workspace/src/handlers/sessions.rs +++ b/crates/temps-workspace/src/handlers/sessions.rs @@ -274,9 +274,13 @@ pub struct WorkspaceSessionResponse { /// Last 4 chars of the current preview password (for UI disambiguation). /// Never contains the full password. pub preview_password_hint: Option, - /// Plaintext preview password — populated ONLY in the response to - /// `POST /sessions` (creation) and `POST /sessions/:id/preview-password/regenerate`. - /// Always `None` on list / get / update responses. + /// Plaintext preview password. Populated by the server whenever the + /// session row has a recoverable copy stored in + /// `preview_password_encrypted`; this includes new sessions and any + /// session whose password has been regenerated since the encrypted + /// column was added. Legacy sessions that only have the argon2 hash + /// return `None` here — the UI prompts for a regenerate to surface + /// the plaintext. pub preview_password: Option, /// Pre-built URLs for common dev-server ports. pub preview_urls: Vec, @@ -301,7 +305,25 @@ pub struct WorkspaceSessionResponse { } impl WorkspaceSessionResponse { + /// Build a response without the preview password populated. Used when + /// the caller doesn't have access to the encryption service or has + /// already decided not to expose the plaintext (e.g. in tests). Most + /// production handlers should use [`from_model_with_password`] instead. pub fn from_model(s: workspace_sessions::Model, parts: &PreviewUrlParts) -> Self { + Self::from_model_with_password(s, parts, None) + } + + /// Build a response with an explicit decrypted preview password. The + /// caller passes `Some(plaintext)` after asking the workspace service + /// to decrypt the row's `preview_password_encrypted`. Sessions whose + /// encrypted column is not set (legacy rows from before reversible + /// storage was added) get `None` here and the UI tells the user to + /// regenerate to view it. + pub fn from_model_with_password( + s: workspace_sessions::Model, + parts: &PreviewUrlParts, + preview_password: Option, + ) -> Self { let id = s.id; let preview_urls = build_preview_urls(&s.public_id, parts); let preview_url_template = build_preview_url_template(&s.public_id, parts); @@ -326,7 +348,7 @@ impl WorkspaceSessionResponse { .map(|t| t.format("%Y-%m-%dT%H:%M:%SZ").to_string()), sandbox_container_id: s.sandbox_container_id, preview_password_hint: s.preview_password_hint, - preview_password: None, + preview_password, preview_urls, preview_url_template, idle_timeout_minutes: s.idle_timeout_minutes, @@ -2670,7 +2692,8 @@ pub async fn workspace_regenerate_preview_password( .await?; let session = app_state.workspace_service.get_session(session_id).await?; let preview_parts = preview_url_parts(&app_state).await; - let mut resp = SessionResponse::from_model(session, &preview_parts); + let mut resp = + SessionResponse::from_model_with_password(session, &preview_parts, Some(plaintext.clone())); resp.preview_password = Some(plaintext); tracing::info!( "Workspace session {} preview password regenerated by user {}", @@ -2715,7 +2738,10 @@ pub async fn workspace_list_sessions( Ok(Json(SessionListResponse { sessions: sessions .into_iter() - .map(|s| SessionResponse::from_model(s, &preview_parts)) + .map(|s| { + let pwd = app_state.workspace_service.decrypt_preview_password(&s); + SessionResponse::from_model_with_password(s, &preview_parts, pwd) + }) .collect(), total, page, @@ -2751,8 +2777,11 @@ pub async fn workspace_get_session( .await?; let preview_parts = preview_url_parts(&app_state).await; + let pwd = app_state + .workspace_service + .decrypt_preview_password(&swm.session); Ok(Json(SessionWithMessagesResponse { - session: SessionResponse::from_model(swm.session, &preview_parts), + session: SessionResponse::from_model_with_password(swm.session, &preview_parts, pwd), messages: swm .messages .into_iter() @@ -3160,7 +3189,14 @@ pub async fn workspace_update_session( project_id, auth.user_id() ); - Ok(Json(SessionResponse::from_model(updated, &preview_parts))) + let pwd = app_state + .workspace_service + .decrypt_preview_password(&updated); + Ok(Json(SessionResponse::from_model_with_password( + updated, + &preview_parts, + pwd, + ))) } #[utoipa::path( @@ -3332,9 +3368,16 @@ pub async fn workspace_reopen_session( ); let preview_parts = preview_url_parts(&app_state).await; + let pwd = app_state + .workspace_service + .decrypt_preview_password(&session); Ok(( StatusCode::OK, - Json(SessionResponse::from_model(session, &preview_parts)), + Json(SessionResponse::from_model_with_password( + session, + &preview_parts, + pwd, + )), )) } diff --git a/crates/temps-workspace/src/plugin.rs b/crates/temps-workspace/src/plugin.rs index 36c885c6a6..ec61c13f9e 100644 --- a/crates/temps-workspace/src/plugin.rs +++ b/crates/temps-workspace/src/plugin.rs @@ -60,8 +60,14 @@ impl TempsPlugin for WorkspacePlugin { let db = context.require_service::(); let encryption_service = context.require_service::(); - // Create workspace service - let workspace_service = Arc::new(WorkspaceService::new(db.clone())); + // Create workspace service. Threading the encryption service in + // here lets `WorkspaceService` populate `preview_password_encrypted` + // on create/regenerate and decrypt it for read-time API responses, + // so users don't have to regenerate to view their own password. + let workspace_service = Arc::new(WorkspaceService::with_encryption( + db.clone(), + encryption_service.clone(), + )); context.register_service(workspace_service.clone()); // Create git credential service. Optional registration: only diff --git a/crates/temps-workspace/src/services/workspace_service.rs b/crates/temps-workspace/src/services/workspace_service.rs index 7a9a6921d6..788eb065bf 100644 --- a/crates/temps-workspace/src/services/workspace_service.rs +++ b/crates/temps-workspace/src/services/workspace_service.rs @@ -4,7 +4,9 @@ use sea_orm::{ EntityTrait, Order, PaginatorTrait, QueryFilter, QueryOrder, }; use std::sync::Arc; +use tracing::warn; +use temps_core::EncryptionService; use temps_entities::{workspace_messages, workspace_sessions}; use crate::error::WorkspaceError; @@ -103,11 +105,57 @@ pub struct CreatedSession { /// Workspace service handles CRUD for sessions and messages. pub struct WorkspaceService { db: Arc, + /// Used to encrypt the plaintext preview password at rest so the API + /// can return it on every read instead of only once at create time. + /// Optional for tests that don't exercise the preview-password paths; + /// production wiring always supplies it via the workspace plugin. + encryption: Option>, } impl WorkspaceService { + /// Construct without an encryption service. Tests use this; production + /// code goes through `with_encryption` so reads can return plaintext. pub fn new(db: Arc) -> Self { - Self { db } + Self { + db, + encryption: None, + } + } + + /// Construct with an encryption service. Required in production so + /// `preview_password_encrypted` is populated on writes and decryptable + /// on reads. + pub fn with_encryption( + db: Arc, + encryption: Arc, + ) -> Self { + Self { + db, + encryption: Some(encryption), + } + } + + /// Decrypt a session's stored preview password, if one is stored + /// reversibly. Returns `None` for legacy sessions that only have the + /// argon2 hash, for sessions with no password set, or when the + /// encryption service is absent (test wiring). + /// + /// A decryption failure is logged at WARN and treated as `None` so a + /// single corrupt row doesn't fail the whole list endpoint. + pub fn decrypt_preview_password(&self, model: &workspace_sessions::Model) -> Option { + let enc = self.encryption.as_ref()?; + let encrypted = model.preview_password_encrypted.as_deref()?; + match enc.decrypt_string(encrypted) { + Ok(plaintext) => Some(plaintext), + Err(e) => { + warn!( + session_id = model.id, + error = %e, + "preview password decryption failed; treating as unavailable" + ); + None + } + } } /// Create a new workspace session. Also generates the session's @@ -187,6 +235,14 @@ impl WorkspaceService { reason, })?; + // Encrypt the plaintext at rest so subsequent reads can return it + // without forcing a regenerate. The argon2 hash is still written + // alongside it so the preview gateway's verify path is unchanged. + // If encryption fails (only possible on a misconfigured master key) + // we fail the create — saving a half-populated row would silently + // break the "view password later" contract this column exists for. + let encrypted = self.encrypt_preview_password(&gp.plaintext, 0)?; + let now = Utc::now(); let session = workspace_sessions::ActiveModel { public_id: Set(crate::services::public_id::generate()), @@ -214,6 +270,7 @@ impl WorkspaceService { estimated_cost_cents: Set(0), files_changed: Set(0), preview_password_hash: Set(Some(gp.hash)), + preview_password_encrypted: Set(encrypted), preview_password_hint: Set(Some(gp.hint)), ..Default::default() }; @@ -234,13 +291,40 @@ impl WorkspaceService { let session = self.get_session(session_id).await?; let gp = preview_password::generate() .map_err(|reason| WorkspaceError::PasswordHashFailed { session_id, reason })?; + let encrypted = self.encrypt_preview_password(&gp.plaintext, session_id)?; let mut active: workspace_sessions::ActiveModel = session.into(); active.preview_password_hash = Set(Some(gp.hash)); + active.preview_password_encrypted = Set(encrypted); active.preview_password_hint = Set(Some(gp.hint)); active.update(self.db.as_ref()).await?; Ok(gp.plaintext) } + /// Encrypt a plaintext preview password for at-rest storage. + /// + /// Returns `Ok(Some(ciphertext))` when the encryption service is wired + /// up, `Ok(None)` when it isn't (test wiring — the row will fall back + /// to argon2-only behavior, indistinguishable from a legacy session). + /// Returns `Err` only when the encryption service is present but + /// failed — a misconfigured master key — which we propagate as + /// `PasswordHashFailed` so the caller refuses to create the session + /// rather than silently dropping the encrypted column. + fn encrypt_preview_password( + &self, + plaintext: &str, + session_id: i32, + ) -> Result, WorkspaceError> { + let Some(enc) = self.encryption.as_ref() else { + return Ok(None); + }; + enc.encrypt_string(plaintext) + .map(Some) + .map_err(|e| WorkspaceError::PasswordHashFailed { + session_id, + reason: format!("encrypt preview password: {}", e), + }) + } + /// Get a session by ID. pub async fn get_session( &self, @@ -677,6 +761,7 @@ mod tests { files_changed: 0, metadata: None, preview_password_hash: None, + preview_password_encrypted: None, preview_password_hint: None, idle_timeout_minutes: None, cpu_milli: None, diff --git a/web/src/api/client/types.gen.ts b/web/src/api/client/types.gen.ts index 046adc092e..398b905559 100644 --- a/web/src/api/client/types.gen.ts +++ b/web/src/api/client/types.gen.ts @@ -13753,9 +13753,13 @@ export type WorkspaceSessionResponse = { */ pids_limit?: number | null; /** - * Plaintext preview password — populated ONLY in the response to - * `POST /sessions` (creation) and `POST /sessions/:id/preview-password/regenerate`. - * Always `None` on list / get / update responses. + * Plaintext preview password. Populated by the server whenever the + * session row has a recoverable copy stored in + * `preview_password_encrypted`; this includes new sessions and any + * session whose password has been regenerated since the encrypted + * column was added. Legacy sessions that only have the argon2 hash + * return `None` here — the UI prompts for a regenerate to surface + * the plaintext. */ preview_password?: string | null; /** diff --git a/web/src/components/workspace/SessionPreviewCard.tsx b/web/src/components/workspace/SessionPreviewCard.tsx index 9184074ca4..ec64ff2bb7 100644 --- a/web/src/components/workspace/SessionPreviewCard.tsx +++ b/web/src/components/workspace/SessionPreviewCard.tsx @@ -58,27 +58,30 @@ export function SessionPreviewCard({ }: SessionPreviewCardProps) { const queryClient = useQueryClient() - // Card stays collapsed by default — opens automatically the first time we - // see a fresh plaintext password so the user doesn't miss the show-once reveal. + // Card stays collapsed by default — opens automatically the first time + // we see a fresh plaintext password (from create/regenerate) so the user + // doesn't miss the change. const [expanded, setExpanded] = useState( defaultExpanded || !!session.preview_password, ) - // The plaintext password lives in component state so the show-once reveal - // survives a refetch (which would null it out on the server side). - const [revealedPassword, setRevealedPassword] = useState( - session.preview_password ?? null, + // Reveal toggle. Passwords are now stored encrypted-at-rest server-side, + // so the API returns the plaintext on every read. We default to hidden + // so the password isn't on screen by default; the user clicks Show to + // reveal it. After a regenerate we auto-reveal so the new value is + // immediately visible. + const [passwordVisible, setPasswordVisible] = useState( + !!session.preview_password, ) - - // When a new session arrives with a fresh password, surface it and pop - // the card open so the user can copy it before it disappears. useEffect(() => { if (session.preview_password) { - setRevealedPassword(session.preview_password) + setPasswordVisible(true) setExpanded(true) } }, [session.preview_password]) + const plaintext = session.preview_password ?? null + const invalidate = () => queryClient.invalidateQueries({ queryKey: ['workspace', projectId, 'session', session.id], @@ -87,10 +90,10 @@ export function SessionPreviewCard({ const regenerate = useMutation({ ...workspaceRegeneratePreviewPasswordMutation(), - onSuccess: (updated) => { - if (updated.preview_password) { - setRevealedPassword(updated.preview_password) - } + onSuccess: () => { + // Reveal the new value automatically — the response repopulates + // session.preview_password through the query invalidation below. + setPasswordVisible(true) toast.success('Preview password regenerated') invalidate() }, @@ -239,7 +242,6 @@ export function SessionPreviewCard({ const lifecycleBusy = stop.isPending || start.isPending || restart.isPending || refresh.isPending - const passwordRevealed = revealedPassword !== null // Free-form port input — lets users open arbitrary ports beyond the // common ones we surface as chips. @@ -350,10 +352,28 @@ export function SessionPreviewCard({
    - {/* Preview URLs */} + {/* Preview URLs + password — grouped because anyone visiting a port + will be challenged by the preview gateway for this password. */}
    -
    - Preview URLs +
    +
    + Preview URLs +
    +
    {session.preview_urls.map((p) => ( @@ -389,6 +409,52 @@ export function SessionPreviewCard({ )}
    + + {/* Password row — sits directly under the port chips since they're + related: the gateway prompts for this on every preview visit. + The plaintext is now retrievable from the server (encrypted at + rest), so we can show it on demand with a hide/show toggle. */} + {plaintext ? ( +
    +
    + + Preview password +
    +
    + + {passwordVisible ? plaintext : '•'.repeat(plaintext.length)} + + + +
    +
    + ) : ( +
    + + {session.preview_password_hint ? ( + + Preview password ends in{' '} + + …{session.preview_password_hint} + + . Click Regenerate to + view it. + + ) : ( + + No preview password set — click{' '} + Regenerate to issue one. + + )} +
    + )}
    {/* Idle timeout (per-session) */} @@ -578,65 +644,6 @@ export function SessionPreviewCard({

    - {/* Show-once preview password */} -
    -
    -
    - - Preview password -
    - -
    - - {passwordRevealed ? ( -
    -
    - - {revealedPassword} - - - -
    -

    - Save this password — it will not be shown again. Enter it on the - login page shown when you first open a preview URL. -

    -
    - ) : ( -
    - {session.preview_password_hint ? ( - <> - Active password ends in{' '} - - …{session.preview_password_hint} - - . Click Regenerate to - issue a new one. - - ) : ( - <>No preview password set yet. Click Regenerate to issue one. - )} -
    - )} -
    )} diff --git a/web/src/components/workspace/SessionPreviewPopover.tsx b/web/src/components/workspace/SessionPreviewPopover.tsx new file mode 100644 index 0000000000..6893aa8f5f --- /dev/null +++ b/web/src/components/workspace/SessionPreviewPopover.tsx @@ -0,0 +1,174 @@ +// Globe icon + popover for the workspace session header. Surfaces the +// sandbox's preview ports without making the user expand the chat-side +// SessionPreviewCard. Lets them hop to a known port chip or punch in an +// arbitrary port via the same `preview_url_template` that the card uses. + +import { useMemo, useState } from 'react' +import { Eye, EyeOff, ExternalLink, Globe, KeyRound } from 'lucide-react' + +import { Button } from '@/components/ui/button' +import { CopyButton } from '@/components/ui/copy-button' +import { Input } from '@/components/ui/input' +import { + Popover, + PopoverContent, + PopoverTrigger, +} from '@/components/ui/popover' +import type { WorkspaceSessionResponse as WorkspaceSession } from '@/api/client/types.gen' + +interface SessionPreviewPopoverProps { + session: WorkspaceSession +} + +export function SessionPreviewPopover({ session }: SessionPreviewPopoverProps) { + const [customPort, setCustomPort] = useState('') + // Preview password is hidden by default so it isn't on screen the moment + // the popover opens; one click on Show reveals it. Sessions whose + // password predates reversible storage return null here — fall back to + // the 4-char hint and prompt the user to regenerate. + const [passwordVisible, setPasswordVisible] = useState(false) + const previewPassword = session.preview_password ?? null + + const customPortValid = useMemo(() => { + if (!/^\d+$/.test(customPort)) return false + const n = Number(customPort) + return n >= 1 && n <= 65535 + }, [customPort]) + + const customUrl = + customPortValid && session.preview_url_template + ? session.preview_url_template.replace('{port}', customPort) + : null + + const openCustom = () => { + if (!customUrl) return + window.open(customUrl, '_blank', 'noopener,noreferrer') + } + + return ( + + + + + +
    + + Preview ports +
    + + {session.preview_urls.length > 0 ? ( +
    + {session.preview_urls.map((p) => ( + + + {p.port} + + ))} +
    + ) : ( +

    + No detected ports yet — start a server in the sandbox or open one + below. +

    + )} + +
    +
    + Open custom port +
    +
    + setCustomPort(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Enter' && customPortValid) openCustom() + }} + placeholder="3000" + inputMode="numeric" + className="h-8 text-xs" + /> + +
    + {customUrl && ( +
    + + {customUrl} + + +
    + )} +
    + + {previewPassword ? ( +
    +
    + + Preview password +
    +
    + + {passwordVisible + ? previewPassword + : '•'.repeat(previewPassword.length)} + + + +
    +
    + ) : ( + session.preview_password_hint && ( +
    + + + Preview password ends in{' '} + + {session.preview_password_hint} + + . Regenerate from the chat panel to view it. + +
    + ) + )} +     +     + ) +} diff --git a/web/src/components/workspace/SessionTerminal.tsx b/web/src/components/workspace/SessionTerminal.tsx index a538c216d7..219886e757 100644 --- a/web/src/components/workspace/SessionTerminal.tsx +++ b/web/src/components/workspace/SessionTerminal.tsx @@ -266,9 +266,25 @@ export const SessionTerminal = forwardRef< // Debounced fit on container resize. Skips degenerate (hidden) // measurements so we never ship a tiny size to the PTY. + // + // Short-circuit when the container's pixel dimensions haven't changed + // since the last fit. fit.fit() walks the canvas to remeasure cell + // metrics and reallocates the WASM grid — running it on every + // ResizeObserver callback during a drag pegs a CPU core. Since the + // container size is the only input that affects cols/rows, comparing + // it lets us skip the entire fit/sendResize chain when nothing has + // moved (which is most callbacks: the parent layout reflows by a + // few pixels constantly without changing our box). + let lastFitW = -1 + let lastFitH = -1 const refit = () => { if (!fit || !term) return - if (container.clientWidth < 20 || container.clientHeight < 20) return + const w = container.clientWidth + const h = container.clientHeight + if (w < 20 || h < 20) return + if (w === lastFitW && h === lastFitH) return + lastFitW = w + lastFitH = h try { fit.fit() sendResize() @@ -276,9 +292,15 @@ export const SessionTerminal = forwardRef< /* fit can throw if container is detached mid-resize */ } } + // Trailing-edge debounce at 150ms. Drag events fire at ~16ms (60fps); + // 60ms collapses only a few per burst, so fit/SIGWINCH fired 15+ + // times per second of drag — each one a full tmux pane repaint that + // ghostty-web has to VT-parse and canvas-paint. 150ms means we fit + // once per drag (when the user pauses), eliminating the per-frame + // CPU spike and the corruption from tmux racing its own repaints. ro = new ResizeObserver(() => { if (resizeTimer) clearTimeout(resizeTimer) - resizeTimer = setTimeout(refit, 60) + resizeTimer = setTimeout(refit, 150) }) ro.observe(container) diff --git a/web/src/components/workspace/WorkspacePage.tsx b/web/src/components/workspace/WorkspacePage.tsx index 0f696ede5b..987797f9f6 100644 --- a/web/src/components/workspace/WorkspacePage.tsx +++ b/web/src/components/workspace/WorkspacePage.tsx @@ -61,6 +61,7 @@ import type { } from '@/api/client/types.gen' import { sessionStreamUrl } from './helpers' import { SessionPreviewCard } from './SessionPreviewCard' +import { SessionPreviewPopover } from './SessionPreviewPopover' import { TerminalTabs, type TerminalTabsHandle } from './TerminalTabs' import { SandboxStatsBadge } from './SandboxStatsBadge' import { TerminalKeysMenu } from './TerminalKeysMenu' @@ -594,6 +595,11 @@ export function WorkspacePage({ project }: WorkspacePageProps) {
    + {activeSessionQuery.data?.session && ( + + )} {viewMode === 'terminal' && ( +
    + + + + + Two-Factor Authentication From 58abb44a7c4739f57a6cf4a16a27a2b8c7b78012 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Thu, 7 May 2026 15:10:21 +0200 Subject: [PATCH 6/8] feat: misc improvements across git credential, sandbox, workspace, and log viewers --- crates/temps-agents/src/sandbox/docker.rs | 10 +- crates/temps-git-credential/src/bin/daemon.rs | 102 +++++- crates/temps-git-credential/src/bin/helper.rs | 81 ++++- crates/temps-git-credential/src/lib.rs | 2 +- .../temps-git-credential/tests/end_to_end.rs | 337 +++++++++++++++++- .../src/services/git_provider_manager.rs | 63 +++- .../src/services/message_executor.rs | 33 +- .../containers/ContainerLogsViewer.tsx | 90 ++--- .../runtime-logs/history-log-viewer.tsx | 133 ++++++- 9 files changed, 762 insertions(+), 89 deletions(-) diff --git a/crates/temps-agents/src/sandbox/docker.rs b/crates/temps-agents/src/sandbox/docker.rs index ce5b66ddc0..3f74920749 100644 --- a/crates/temps-agents/src/sandbox/docker.rs +++ b/crates/temps-agents/src/sandbox/docker.rs @@ -314,13 +314,21 @@ COPY --from=git-credential-builder /build/target/release/temps-git-credential-da RUN chmod 0755 /usr/local/bin/temps-git-credential-helper /usr/local/bin/temps-git-credential-daemon \ && mkdir -p /run/temps-git \ && chown temps-git:git-users /run/temps-git \ - && chmod 0750 /run/temps-git \ + && chmod 2750 /run/temps-git \ && mkdir -p /etc/temps \ && chown temps-git:git-users /etc/temps \ && chmod 0710 /etc/temps \ && touch /etc/temps/credential-daemon.env \ && chown temps-git:temps-git /etc/temps/credential-daemon.env \ && chmod 0600 /etc/temps/credential-daemon.env +# Mode 2750 on /run/temps-git/ sets the setgid bit on the directory. +# Without it, files (including the IPC socket) created inside inherit +# the *creating process's egid* — `temps-git`'s primary group, which +# is also `temps-git`. With setgid, new files inherit the parent dir's +# group (`git-users`) instead, which is what the user shell needs for +# `connect()` to succeed under mode 0660. Belt-and-braces: the daemon +# also explicitly chgrp's the socket on bind, so existing images +# without the setgid bit still recover after a daemon restart. # System-wide git config: route every HTTPS git auth request through # the credential helper. `useHttpPath=true` is mandatory — without it # git omits the `path=` field, and the daemon can't tell what repo is diff --git a/crates/temps-git-credential/src/bin/daemon.rs b/crates/temps-git-credential/src/bin/daemon.rs index ed5d7846f9..ae10f57221 100644 --- a/crates/temps-git-credential/src/bin/daemon.rs +++ b/crates/temps-git-credential/src/bin/daemon.rs @@ -18,7 +18,8 @@ use std::time::Duration; use serde::{Deserialize, Serialize}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::{UnixListener, UnixStream}; -use tokio::sync::Mutex; +use tokio::signal::unix::{signal, SignalKind}; +use tokio::sync::{Mutex, RwLock}; use tracing::{debug, error, info, warn}; use temps_git_credential::ipc::{IpcRequest, IpcResponse}; @@ -105,13 +106,22 @@ async fn main() { let config_path = std::env::var("TEMPS_GIT_CREDENTIAL_DAEMON_ENV") .unwrap_or_else(|_| DEFAULT_DAEMON_ENV_PATH.to_string()); let config = match DaemonConfig::load_from(Path::new(&config_path)) { - Ok(c) => Arc::new(c), + Ok(c) => Arc::new(RwLock::new(c)), Err(e) => { error!("Failed to load daemon config from {}: {}", config_path, e); std::process::exit(1); } }; + // Spawn a SIGHUP listener that reloads the env file in place. The + // host-side message_executor sends SIGHUP after rewriting the env + // file (e.g. when a session's deployment token rotates) — without + // this handler, the default disposition for SIGHUP is *terminate*, + // which would kill the daemon and force a relaunch on next IPC. + // Reloading in place keeps cached credentials valid across token + // rotations and avoids the brief unavailability window. + spawn_sighup_reloader(config.clone(), config_path.clone()); + let socket_path = PathBuf::from( std::env::var("TEMPS_GIT_CREDENTIAL_SOCKET") .unwrap_or_else(|_| DEFAULT_SOCKET_PATH.to_string()), @@ -140,8 +150,12 @@ async fn main() { }; // Set permissive group-read perms (0660). The Dockerfile chowns - // the parent dir to `temps-git:git-users` so this picks up the - // right owner; mode change is the part we control here. + // the parent dir to `temps-git:git-users` and sets setgid (mode + // 2750) so the socket inherits `git-users` as its group. We still + // explicitly chgrp here as a safety net for images built before + // the setgid fix (existing containers can recover by killing the + // daemon and letting message_executor relaunch it, without + // rebuilding the image). if let Err(e) = std::fs::set_permissions( &socket_path, std::os::unix::fs::PermissionsExt::from_mode(0o660), @@ -152,6 +166,23 @@ async fn main() { e ); } + match nix::unistd::Group::from_name("git-users") { + Ok(Some(group)) => { + if let Err(e) = std::os::unix::fs::chown(&socket_path, None, Some(group.gid.as_raw())) { + warn!( + "Failed to chgrp credential socket {} to git-users: {}", + socket_path.display(), + e + ); + } + } + Ok(None) => warn!( + "Group 'git-users' not found; user shell will not be able to \ + connect to the credential socket. Check the sandbox image's \ + groupadd step." + ), + Err(e) => warn!("Failed to look up 'git-users' group: {}", e), + } info!( "temps-git-credential-daemon listening on {}", @@ -191,9 +222,48 @@ fn init_tracing() { tracing_subscriber::fmt().with_env_filter(filter).init(); } +/// Spawn a background task that watches for SIGHUP and reloads the env +/// file on each delivery. A failed reload is logged but does NOT +/// terminate the daemon — the existing in-memory config keeps serving +/// requests. This is the safer default: a malformed file should never +/// take down the credential pipeline mid-session. +fn spawn_sighup_reloader(config: Arc>, config_path: String) { + tokio::spawn(async move { + let mut hup = match signal(SignalKind::hangup()) { + Ok(s) => s, + Err(e) => { + error!( + "Failed to register SIGHUP handler: {}. Token reloads will require restart.", + e + ); + return; + } + }; + loop { + if hup.recv().await.is_none() { + // Stream closed — Tokio is shutting down. Exit quietly. + return; + } + match DaemonConfig::load_from(Path::new(&config_path)) { + Ok(new_cfg) => { + let mut guard = config.write().await; + *guard = new_cfg; + info!("Reloaded daemon config from {} on SIGHUP", config_path); + } + Err(e) => { + warn!( + "SIGHUP received but reloading {} failed: {}. Continuing with existing config.", + config_path, e + ); + } + } + } + }); +} + async fn handle_connection( mut stream: UnixStream, - config: Arc, + config: Arc>, http: reqwest::Client, cache: Cache, ) -> std::io::Result<()> { @@ -253,7 +323,7 @@ async fn handle_get( owner: &str, repo: &str, operation: Operation, - config: &DaemonConfig, + config: &RwLock, http: &reqwest::Client, cache: &Cache, ) -> IpcResponse { @@ -282,9 +352,23 @@ async fn handle_get( } } + // Snapshot the config under a short-lived read lock. Holding the + // lock across the HTTP call would block the SIGHUP reloader for the + // full request duration; the snapshot keeps lock contention to a + // few µs even under bursty mint traffic. + let (api_url, api_token) = { + let cfg = config.read().await; + (cfg.api_url.clone(), cfg.api_token.clone()) + }; + + // Plugin routes are nested under `/api` in `temps-core/plugin.rs`'s + // `build_application`, so the actual mount point is + // `/api/workspace/git-credential`. Hitting the unprefixed path falls + // through to the SPA static-file fallback and returns HTML, which + // surfaces here as `error decoding response body`. let url = format!( - "{}/workspace/git-credential", - config.api_url.trim_end_matches('/') + "{}/api/workspace/git-credential", + api_url.trim_end_matches('/') ); let body = MintRequestBody { host: host.to_string(), @@ -298,7 +382,7 @@ async fn handle_get( let resp = match http .post(&url) - .bearer_auth(&config.api_token) + .bearer_auth(&api_token) .json(&body) .send() .await diff --git a/crates/temps-git-credential/src/bin/helper.rs b/crates/temps-git-credential/src/bin/helper.rs index c7c66abf8d..b0759590af 100644 --- a/crates/temps-git-credential/src/bin/helper.rs +++ b/crates/temps-git-credential/src/bin/helper.rs @@ -92,7 +92,7 @@ fn run_get(input: &str) { eprintln!("temps-git-credential-helper: refused by daemon: {reason}"); } Err(e) => { - eprintln!("temps-git-credential-helper: daemon IPC failed: {e}"); + eprintln!("temps-git-credential-helper: {}", explain_ipc_error(&e)); } } } @@ -102,7 +102,7 @@ fn run_store_or_erase(req: IpcRequest) { // inject tokens into our flow). Erase without a parsed body is also // a no-op against the daemon — handled here as "ack and forget". if let Err(e) = send_to_daemon(&req) { - eprintln!("temps-git-credential-helper: daemon IPC failed: {e}"); + eprintln!("temps-git-credential-helper: {}", explain_ipc_error(&e)); } } @@ -120,7 +120,7 @@ fn run_erase(input: &str) { repo: req.repo, }; if let Err(e) = send_to_daemon(&ipc_req) { - eprintln!("temps-git-credential-helper: daemon IPC failed: {e}"); + eprintln!("temps-git-credential-helper: {}", explain_ipc_error(&e)); } } @@ -131,6 +131,39 @@ fn socket_path() -> PathBuf { ) } +/// Map an IPC error into a message that points the user at the actual +/// cause. The default `daemon IPC failed: …` error was a debugging +/// black hole — every failure mode (missing daemon, wrong group +/// membership, dead daemon, malformed response) surfaced identically. +/// These messages are the user's only feedback path because git itself +/// only reports "Authentication failed" downstream. +fn explain_ipc_error(e: &io::Error) -> String { + let path = socket_path(); + let path_str = path.display(); + match e.kind() { + io::ErrorKind::NotFound => format!( + "credential daemon socket not found at {path_str}. \ + The daemon was not started for this session — try reopening \ + the workspace, or check /tmp/temps-git-credential-daemon.log \ + inside the sandbox for startup errors." + ), + io::ErrorKind::PermissionDenied => format!( + "permission denied connecting to {path_str}. \ + Your shell user is likely missing the 'git-users' group \ + membership that grants access to the daemon socket. \ + Run `id` inside the sandbox to verify; expected groups \ + include 'git-users'." + ), + io::ErrorKind::ConnectionRefused => format!( + "connection refused at {path_str}. \ + A stale socket exists but no daemon is listening — the \ + daemon may have crashed. Check \ + /tmp/temps-git-credential-daemon.log inside the sandbox." + ), + _ => format!("daemon IPC failed: {e}"), + } +} + fn send_to_daemon(req: &IpcRequest) -> io::Result { let path = socket_path(); let mut stream = UnixStream::connect(&path)?; @@ -149,3 +182,45 @@ fn send_to_daemon(req: &IpcRequest) -> io::Result { })?; Ok(response) } + +#[cfg(test)] +mod tests { + use super::*; + + /// ENOENT must steer the user toward "daemon never started" rather + /// than the generic IPC error — the original bug surfaced as opaque + /// "Permission denied" when the socket dir was actually readable but + /// the daemon never launched. + #[test] + fn explain_ipc_error_distinguishes_not_found() { + let err = io::Error::from(io::ErrorKind::NotFound); + let msg = explain_ipc_error(&err); + assert!(msg.contains("not found"), "msg: {msg}"); + assert!(msg.contains("daemon was not started"), "msg: {msg}"); + } + + /// EACCES means the socket exists but the caller's group membership + /// is wrong — a different fix path from "daemon down". The message + /// must mention `git-users` or the user has nothing to grep for. + #[test] + fn explain_ipc_error_distinguishes_permission_denied() { + let err = io::Error::from(io::ErrorKind::PermissionDenied); + let msg = explain_ipc_error(&err); + assert!(msg.contains("permission denied"), "msg: {msg}"); + assert!(msg.contains("git-users"), "msg: {msg}"); + } + + /// ECONNREFUSED only happens when a socket file exists but no + /// process is bound to it — a crashed daemon. The message must + /// point at the daemon log so the user can find the crash reason. + #[test] + fn explain_ipc_error_distinguishes_connection_refused() { + let err = io::Error::from(io::ErrorKind::ConnectionRefused); + let msg = explain_ipc_error(&err); + assert!(msg.contains("connection refused"), "msg: {msg}"); + assert!( + msg.contains("temps-git-credential-daemon.log"), + "msg: {msg}" + ); + } +} diff --git a/crates/temps-git-credential/src/lib.rs b/crates/temps-git-credential/src/lib.rs index 2d50f98f7b..ed45bd2b22 100644 --- a/crates/temps-git-credential/src/lib.rs +++ b/crates/temps-git-credential/src/lib.rs @@ -4,7 +4,7 @@ //! ## Architecture (one paragraph) //! Git, running as user `temps` (uid 1000), invokes the credential //! helper. The helper reads git's request from stdin (key=value lines), -//! connects to the daemon over a Unix socket at `/run/temps/git.sock` +//! connects to the daemon over a Unix socket at `/run/temps-git/git.sock` //! (mode `0660`, owned by `temps-git:git-users`, with `temps` in the //! `git-users` group), forwards the request as a JSON line, and writes //! the daemon's JSON response back to stdout in git's expected diff --git a/crates/temps-git-credential/tests/end_to_end.rs b/crates/temps-git-credential/tests/end_to_end.rs index 8424744837..09bd144702 100644 --- a/crates/temps-git-credential/tests/end_to_end.rs +++ b/crates/temps-git-credential/tests/end_to_end.rs @@ -2,7 +2,7 @@ //! //! Spins up: //! 1. A tiny in-process HTTP server impersonating the control-plane -//! `/workspace/git-credential` endpoint. +//! `/api/workspace/git-credential` endpoint. //! 2. The real `temps-git-credential-daemon` binary, pointed at that //! server via `TEMPS_GIT_CREDENTIAL_DAEMON_ENV` and listening on a //! temp-dir Unix socket. @@ -340,3 +340,338 @@ async fn helper_rejects_request_without_path() { "stderr should explain the missing path, got: {stderr}" ); } + +/// Crash + relaunch path. Models what `message_executor`'s launch +/// script does when it detects a dead daemon: remove the stale socket, +/// relaunch the daemon, verify the new instance serves requests. +/// +/// Why this matters: the original "git push fails inside sandbox" bug +/// happened because the launch script's idempotency check (`[ -S socket +/// ]`) would skip the relaunch if the socket file still existed — +/// which it does after a crash. This test pins the post-fix invariant: +/// after `rm -f socket && relaunch`, the daemon comes back healthy. +#[tokio::test(flavor = "multi_thread")] +async fn daemon_can_be_relaunched_after_crash() { + let (helper, daemon) = match binaries_or_skip() { + Some(p) => p, + None => return, + }; + + let dir = tempfile::tempdir().expect("tempdir"); + let socket_path = dir.path().join("git.sock"); + let env_file = dir.path().join("daemon.env"); + let mock_port = pick_free_port(); + + let mock_requests = Arc::new(AtomicU64::new(0)); + let mock_handle = tokio::spawn(mock_server( + mock_port, + 200, + json!({ + "username": "x-access-token", + "password": "ghs_RELAUNCH", + "expires_at": "2099-01-01T00:00:00Z", + }), + mock_requests.clone(), + )); + tokio::time::sleep(Duration::from_millis(50)).await; + + std::fs::write( + &env_file, + format!( + "TEMPS_API_URL=http://127.0.0.1:{}\nTEMPS_API_TOKEN=dt_relaunch\n", + mock_port + ), + ) + .expect("write env file"); + + // First daemon instance. + let mut daemon_a = Command::new(&daemon) + .env("TEMPS_GIT_CREDENTIAL_DAEMON_ENV", &env_file) + .env("TEMPS_GIT_CREDENTIAL_SOCKET", &socket_path) + .env("RUST_LOG", "warn") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn daemon A"); + + let mut waited = 0u32; + while !socket_path.exists() && waited < 50 { + tokio::time::sleep(Duration::from_millis(50)).await; + waited += 1; + } + assert!(socket_path.exists(), "daemon A never bound socket"); + + // Sanity: helper works against daemon A. + { + let mut h = Command::new(&helper) + .arg("get") + .env("TEMPS_GIT_CREDENTIAL_SOCKET", &socket_path) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn helper A"); + h.stdin + .as_mut() + .unwrap() + .write_all(b"protocol=https\nhost=github.com\npath=acme/web\n\n") + .unwrap(); + let out = h.wait_with_output().unwrap(); + assert!(out.status.success(), "pre-crash helper should succeed"); + assert!( + String::from_utf8_lossy(&out.stdout).contains("ghs_RELAUNCH"), + "pre-crash helper should return mock credential" + ); + } + + // Simulate daemon crash with SIGKILL — bypasses any cleanup the + // daemon would do on graceful shutdown, leaving the socket file on + // disk. This is exactly the state that tripped the old idempotency + // check (`[ -S socket ]` was true even though no daemon listened). + let _ = daemon_a.kill(); + let _ = daemon_a.wait(); + + // Stale socket invariant: file exists but connect() now fails + // because no process is bound to it. This is the precondition the + // launch-script fix explicitly cleans up before relaunching. + assert!( + socket_path.exists(), + "SIGKILL should leave stale socket on disk — that is the bug \ + the relaunch path has to handle" + ); + + // Mirror the launch script's recovery: remove stale socket, then + // relaunch the daemon. (The actual script in message_executor.rs + // also checks `pgrep` first; we already KNOW the process is dead so + // we go straight to the cleanup + relaunch.) + std::fs::remove_file(&socket_path).expect("rm stale socket"); + + let mut daemon_b = Command::new(&daemon) + .env("TEMPS_GIT_CREDENTIAL_DAEMON_ENV", &env_file) + .env("TEMPS_GIT_CREDENTIAL_SOCKET", &socket_path) + .env("RUST_LOG", "warn") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn daemon B"); + + let mut waited = 0u32; + while !socket_path.exists() && waited < 50 { + tokio::time::sleep(Duration::from_millis(50)).await; + waited += 1; + } + assert!( + socket_path.exists(), + "daemon B never bound socket after relaunch" + ); + + // Verify the new daemon serves a fresh credential — same mock + // server, but a separate request from helper B's perspective. + let pre_count = mock_requests.load(Ordering::SeqCst); + { + let mut h = Command::new(&helper) + .arg("get") + .env("TEMPS_GIT_CREDENTIAL_SOCKET", &socket_path) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn helper B"); + h.stdin + .as_mut() + .unwrap() + .write_all(b"protocol=https\nhost=github.com\npath=acme/web\n\n") + .unwrap(); + let out = h.wait_with_output().unwrap(); + assert!( + out.status.success(), + "post-relaunch helper should succeed, stderr={:?}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + String::from_utf8_lossy(&out.stdout).contains("ghs_RELAUNCH"), + "post-relaunch helper should return mock credential" + ); + } + // The relaunched daemon has a fresh empty cache, so it MUST hit + // the mock server — confirming we're talking to a live process and + // not somehow reading stale state. + assert!( + mock_requests.load(Ordering::SeqCst) > pre_count, + "post-relaunch helper should have triggered a new mint call" + ); + + let _ = daemon_b.kill(); + let _ = daemon_b.wait(); + mock_handle.abort(); +} + +/// SIGHUP must reload the env file in place rather than killing the +/// daemon. The previous behavior (default disposition = terminate) +/// caused a token-rotation refresh to take down the credential +/// pipeline until the next workspace open. The new behavior reads the +/// updated env file and serves credentials with the rotated token on +/// the next request. +#[tokio::test(flavor = "multi_thread")] +async fn daemon_reloads_env_file_on_sighup() { + let (helper, daemon) = match binaries_or_skip() { + Some(p) => p, + None => return, + }; + + let dir = tempfile::tempdir().expect("tempdir"); + let socket_path = dir.path().join("git.sock"); + let env_file = dir.path().join("daemon.env"); + + // Two mock servers on different ports — the env file rewrite + // points the daemon at the second one. If SIGHUP truly reloaded, + // the post-HUP helper request lands on server B. + let port_a = pick_free_port(); + let port_b = pick_free_port(); + let requests_a = Arc::new(AtomicU64::new(0)); + let requests_b = Arc::new(AtomicU64::new(0)); + + let mock_a = tokio::spawn(mock_server( + port_a, + 200, + json!({ + "username": "x-access-token", + "password": "ghs_FROM_A", + "expires_at": "2099-01-01T00:00:00Z", + }), + requests_a.clone(), + )); + let mock_b = tokio::spawn(mock_server( + port_b, + 200, + json!({ + "username": "x-access-token", + "password": "ghs_FROM_B", + "expires_at": "2099-01-01T00:00:00Z", + }), + requests_b.clone(), + )); + tokio::time::sleep(Duration::from_millis(50)).await; + + // Start pointing at server A. + std::fs::write( + &env_file, + format!( + "TEMPS_API_URL=http://127.0.0.1:{}\nTEMPS_API_TOKEN=dt_v1\n", + port_a + ), + ) + .unwrap(); + + let mut daemon_proc = Command::new(&daemon) + .env("TEMPS_GIT_CREDENTIAL_DAEMON_ENV", &env_file) + .env("TEMPS_GIT_CREDENTIAL_SOCKET", &socket_path) + .env("RUST_LOG", "warn") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn daemon"); + let daemon_pid = daemon_proc.id(); + + let mut waited = 0u32; + while !socket_path.exists() && waited < 50 { + tokio::time::sleep(Duration::from_millis(50)).await; + waited += 1; + } + assert!(socket_path.exists()); + + // First request — must hit server A. Use a unique repo so the + // daemon's per-(host,owner,repo,op) cache doesn't mask the second + // request. + { + let mut h = Command::new(&helper) + .arg("get") + .env("TEMPS_GIT_CREDENTIAL_SOCKET", &socket_path) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + h.stdin + .as_mut() + .unwrap() + .write_all(b"protocol=https\nhost=github.com\npath=acme/repo-a\n\n") + .unwrap(); + let out = h.wait_with_output().unwrap(); + assert!(out.status.success()); + assert!( + String::from_utf8_lossy(&out.stdout).contains("ghs_FROM_A"), + "pre-HUP request should hit server A" + ); + } + assert_eq!( + requests_a.load(Ordering::SeqCst), + 1, + "server A should have served exactly one request" + ); + + // Rewrite env file to point at server B, then SIGHUP. + std::fs::write( + &env_file, + format!( + "TEMPS_API_URL=http://127.0.0.1:{}\nTEMPS_API_TOKEN=dt_v2\n", + port_b + ), + ) + .unwrap(); + + // Use nix's re-exported libc — Tokio's process API doesn't expose + // signals directly, and `nix` is already a workspace dependency so + // we don't pull in a new crate. Signal 0 is the standard "is the + // process alive?" probe (no signal delivered, just permission + + // existence check). + use nix::libc; + unsafe { + libc::kill(daemon_pid as libc::pid_t, libc::SIGHUP); + } + + // Give the reloader a moment to swap the config. + tokio::time::sleep(Duration::from_millis(200)).await; + + // The daemon process must still be alive — the whole point of the + // SIGHUP handler is that it does NOT terminate. + let still_alive = unsafe { libc::kill(daemon_pid as libc::pid_t, 0) } == 0; + assert!( + still_alive, + "daemon should survive SIGHUP, but the process is gone" + ); + + // Second request, different repo to dodge the cache. Must now hit + // server B because the reloader swapped the API URL. + { + let mut h = Command::new(&helper) + .arg("get") + .env("TEMPS_GIT_CREDENTIAL_SOCKET", &socket_path) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + h.stdin + .as_mut() + .unwrap() + .write_all(b"protocol=https\nhost=github.com\npath=acme/repo-b\n\n") + .unwrap(); + let out = h.wait_with_output().unwrap(); + assert!(out.status.success()); + assert!( + String::from_utf8_lossy(&out.stdout).contains("ghs_FROM_B"), + "post-HUP request should hit server B (env reload effective)" + ); + } + assert_eq!( + requests_b.load(Ordering::SeqCst), + 1, + "server B should have served exactly one request after SIGHUP" + ); + + let _ = daemon_proc.kill(); + let _ = daemon_proc.wait(); + mock_a.abort(); + mock_b.abort(); +} diff --git a/crates/temps-git/src/services/git_provider_manager.rs b/crates/temps-git/src/services/git_provider_manager.rs index 91ba4c7165..d6b006b757 100644 --- a/crates/temps-git/src/services/git_provider_manager.rs +++ b/crates/temps-git/src/services/git_provider_manager.rs @@ -741,14 +741,65 @@ impl GitProviderManager { operation: super::git_provider::ScopedTokenOp, ) -> Result { let connection = self.get_connection(connection_id).await?; - let provider_service = self.get_provider_service(connection.provider_id).await?; - let installation_id = connection.installation_id.as_deref(); + // Fast path: GitHub App installation. Provider service mints a + // fresh per-op installation token narrowed to (repo, permission) + // — the security model the daemon was originally designed for. + if let Some(installation_id) = connection.installation_id.as_deref() { + let provider_service = self.get_provider_service(connection.provider_id).await?; + return provider_service + .mint_scoped_repo_token(Some(installation_id), owner, repo, operation) + .await + .map_err(GitProviderManagerError::from); + } - provider_service - .mint_scoped_repo_token(installation_id, owner, repo, operation) - .await - .map_err(GitProviderManagerError::from) + // Fallback: PAT or OAuth connection. There is no provider API + // to narrow these tokens to (repo, permission) at runtime, so + // we hand back the stored long-lived token. This trades the + // per-op blast-radius guarantee for the ability to use git at + // all — operators who need the strict guarantee should switch + // their project to a GitHub App connection. The trade-off is + // logged at WARN so audit reviews can see when it kicked in. + let provider = self.get_provider(connection.provider_id).await?; + let encrypted_token = connection.access_token.as_ref().ok_or_else(|| { + GitProviderManagerError::InvalidConfiguration(format!( + "Connection {} has no access_token; cannot mint credential", + connection_id + )) + })?; + let token = self.decrypt_string(encrypted_token).await?; + + // Username conventions for HTTP Basic auth on git over HTTPS: + // GitHub: `x-access-token` (works for both App tokens and PATs) + // GitLab: `oauth2` (works for both PATs and OAuth tokens) + // Anything else: best-effort `x-access-token`. If we ever add + // Bitbucket etc. this needs updating. + let username = match provider.provider_type.as_str() { + "gitlab" => "oauth2", + _ => "x-access-token", + } + .to_string(); + + tracing::warn!( + connection_id, + owner = %owner, + repo = %repo, + ?operation, + provider_type = %provider.provider_type, + "Minting non-scoped credential from PAT/OAuth connection — \ + token is NOT narrowed per-op. Switch the project to a \ + GitHub App connection if per-op scoping is required." + ); + + Ok(super::git_provider::ScopedTokenGrant { + username, + password: token, + // Stored tokens may have an expiry on the connection row, + // but the daemon doesn't depend on it (it re-mints on every + // operation anyway). Surface `None` so the helper protocol + // doesn't carry a misleading promise. + expires_at: connection.token_expires_at, + }) } /// Get decrypted webhook secret for a provider diff --git a/crates/temps-workspace/src/services/message_executor.rs b/crates/temps-workspace/src/services/message_executor.rs index f8e8f129a1..2d4489ef0c 100644 --- a/crates/temps-workspace/src/services/message_executor.rs +++ b/crates/temps-workspace/src/services/message_executor.rs @@ -1800,21 +1800,32 @@ chmod 0600 /etc/temps/credential-daemon.env" // `docker exec --user` (a Docker API call, which is *not* // subject to no-new-privileges) and detach with `setsid`. // - // Idempotency: if the daemon is already running we don't want - // to spawn a second one, so the launcher first checks for an - // existing socket. The daemon's bind() also fails-fast on - // collision, but the pre-check keeps the logs clean. + // Liveness check: a present socket file alone is not enough + // evidence the daemon is alive — a previous daemon may have + // crashed leaving a stale socket behind. We check both: socket + // exists AND a `temps-git`-owned daemon process is running. If + // either is missing, remove the stale socket and relaunch. This + // makes the call self-healing across daemon crashes without + // needing an in-container supervisor (which `no-new-privileges` + // would block anyway). let launch_script = "set -e -if [ -S /run/temps-git/git.sock ]; then - # Daemon already running — env-file refresh path. Send SIGHUP so - # it picks up the new token. The daemon doesn't currently handle - # SIGHUP — but a future revision can without changing this caller. - pkill -HUP -u temps-git temps-git-cred 2>/dev/null || true +socket=/run/temps-git/git.sock +# pgrep -u temps-git -f exits 0 when ≥1 matching process is alive. +if [ -S \"$socket\" ] && pgrep -u temps-git -f temps-git-credential-daemon >/dev/null 2>&1; then + # Daemon healthy — env-file refresh path. Send SIGHUP so it picks up + # the new token. The daemon doesn't currently handle SIGHUP — but a + # future revision can without changing this caller. + pkill -HUP -u temps-git -f temps-git-credential-daemon 2>/dev/null || true exit 0 fi +# Stale socket from a crashed daemon will block bind() with EADDRINUSE. +# Remove it before relaunching. The daemon also does this on its own +# startup, but doing it here makes the relaunch path explicit. +rm -f \"$socket\" # setsid + redirect detaches from the docker exec lifecycle so this -# process survives `start_exec`'s wait. Logs go to syslog-ish stdout -# of the container which `docker logs` surfaces. +# process survives `start_exec`'s wait. Logs go to /tmp on the +# container so `docker exec cat /tmp/temps-git-credential-daemon.log` +# can surface daemon failures during diagnosis. setsid /usr/local/bin/temps-git-credential-daemon \ >> /tmp/temps-git-credential-daemon.log \ 2>&1 < /dev/null & diff --git a/web/src/components/containers/ContainerLogsViewer.tsx b/web/src/components/containers/ContainerLogsViewer.tsx index e3bb5639ae..5dbfbb64e3 100644 --- a/web/src/components/containers/ContainerLogsViewer.tsx +++ b/web/src/components/containers/ContainerLogsViewer.tsx @@ -204,49 +204,59 @@ export function ContainerLogsViewer({ fetchUrl }: ContainerLogsViewerProps) { )} - {/* Logs Container */} -
    - {logs.length === 0 && connectionStatus === 'connecting' && ( -
    Connecting to logs...
    - )} + {/* Logs Container — wrapped in a bordered rounded card so the live + tail visually matches the history viewer (which uses the same + shell at `runtime-logs/history-log-viewer.tsx:631`). The outer + padding lets the card breathe inside the parent flex column; + inner div is the scroll viewport. */} +
    +
    +
    + {logs.length === 0 && connectionStatus === 'connecting' && ( +
    + Connecting to logs... +
    + )} - {logs.length === 0 && connectionStatus !== 'connecting' && ( -
    No logs available
    - )} + {logs.length === 0 && connectionStatus !== 'connecting' && ( +
    No logs available
    + )} -
    - {virtualizer.getVirtualItems().map((virtualItem) => { - const log = filteredLogs[virtualItem.index] +
    + {virtualizer.getVirtualItems().map((virtualItem) => { + const log = filteredLogs[virtualItem.index] - return ( -
    - -
    - ) - })} + return ( +
    + +
    + ) + })} +
    +
    diff --git a/web/src/components/runtime-logs/history-log-viewer.tsx b/web/src/components/runtime-logs/history-log-viewer.tsx index 222c93b395..f02c28fc56 100644 --- a/web/src/components/runtime-logs/history-log-viewer.tsx +++ b/web/src/components/runtime-logs/history-log-viewer.tsx @@ -13,6 +13,14 @@ import { SelectTrigger, SelectValue, } from '@/components/ui/select' +import { + DropdownMenu, + DropdownMenuCheckboxItem, + DropdownMenuContent, + DropdownMenuLabel, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/components/ui/dropdown-menu' import { Tooltip, TooltipContent, @@ -34,6 +42,7 @@ import { AlertCircle, ArrowUp, Clock, + Columns3, Loader2, Search, } from 'lucide-react' @@ -89,7 +98,19 @@ function presetTimeRange(range: string): { start: string; end: string } { } -function HistoryLogLine({ line }: { line: LogSearchLine }) { +interface ColumnVisibility { + timestamp: boolean + level: boolean + service: boolean +} + +function HistoryLogLine({ + line, + columns, +}: { + line: LogSearchLine + columns: ColumnVisibility +}) { const d = new Date(line.timestamp) const base = d.toLocaleTimeString('en-US', { hour12: false, @@ -101,21 +122,27 @@ function HistoryLogLine({ line }: { line: LogSearchLine }) { return (
    - - {ts} - - - {line.level} - - - {line.service} - + {columns.timestamp && ( + + {ts} + + )} + {columns.level && ( + + {line.level} + + )} + {columns.service && ( + + {line.service} + + )} () const [olderExhausted, setOlderExhausted] = useState(false) + // Column visibility — persisted to localStorage so the user's choice + // survives navigation. Defaults to all visible. The "deployment" column + // is the per-line service name (container/service that emitted the log). + const [columns, setColumns] = useState(() => { + if (typeof window === 'undefined') { + return { timestamp: true, level: true, service: true } + } + try { + const raw = window.localStorage.getItem('temps.history-log.columns') + if (raw) { + const parsed = JSON.parse(raw) as Partial + return { + timestamp: parsed.timestamp ?? true, + level: parsed.level ?? true, + service: parsed.service ?? true, + } + } + } catch { + // Ignore corrupted storage and fall through to defaults. + } + return { timestamp: true, level: true, service: true } + }) + useEffect(() => { + try { + window.localStorage.setItem( + 'temps.history-log.columns', + JSON.stringify(columns), + ) + } catch { + // Storage may be unavailable (private mode, quota); not worth surfacing. + } + }, [columns]) const parentRef = useRef(null) // Include the custom range in the filterKey so picking a different window @@ -573,6 +632,43 @@ export default function HistoryLogViewer({ )} + + + + + + + Show columns + + + setColumns((c) => ({ ...c, timestamp: v === true })) + } + > + Timestamp + + + setColumns((c) => ({ ...c, level: v === true })) + } + > + Level + + + setColumns((c) => ({ ...c, service: v === true })) + } + > + Deployment + + +
    {/* Level filter chips */} @@ -712,7 +808,10 @@ export default function HistoryLogViewer({ width: '100%', }} > - +
    ))}
    From e33b72656b1a3e02812cde1e6a32fa49129bb4ae Mon Sep 17 00:00:00 2001 From: David Viejo Date: Thu, 7 May 2026 18:19:01 +0200 Subject: [PATCH 7/8] fix(web): Projects.tsx tweak --- web/src/pages/Projects.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/pages/Projects.tsx b/web/src/pages/Projects.tsx index 5588634173..e95a90eadc 100644 --- a/web/src/pages/Projects.tsx +++ b/web/src/pages/Projects.tsx @@ -78,7 +78,7 @@ function formatTrendChange(trendPercentage: number | null | undefined): { } } -const ITEMS_PER_PAGE = 8 +const ITEMS_PER_PAGE = 9 // Mirrors the storage key used by ImprovedOnboardingDashboard. When the // dashboard reaches its 'complete' step it persists the state here; we From 07a4eba313818292b9d9b36452d5b994fd86d88c Mon Sep 17 00:00:00 2001 From: David Viejo Date: Sat, 9 May 2026 13:13:01 +0200 Subject: [PATCH 8/8] fix: respect automatic_deploy flag on git push + Observe theme tokens - Gate process_git_push_event on the merged project+environment automatic_deploy flag so pushes no longer deploy when the user has disabled deploy-on-push. Adds is_automatic_deploy_enabled helper with merge semantics and 6 unit tests covering each project/env combination. - Migrate Observe page from hardcoded zinc-* classes to semantic theme tokens (bg-background, text-foreground, border-border, text-muted-foreground, bg-card) for proper light/dark theming. --- CHANGELOG.md | 1 + .../src/services/job_processor.rs | 84 +++++++++++++++++++ crates/temps-observability/src/filters.rs | 7 ++ .../src/handlers/events.rs | 5 ++ crates/temps-observability/src/service.rs | 13 +++ web/src/api/client/types.gen.ts | 6 ++ .../components/observe/ObserveFilterBar.tsx | 1 + web/src/components/observe/useObserveQuery.ts | 6 ++ web/src/pages/Observe.tsx | 74 +++++++++++----- 9 files changed, 178 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f0642c83f..eb133ccb57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`get_temps_api_url` is async**: now reads platform settings to surface the configured `external_url`. `WorkspacePlugin` plumbs `Arc` into `MessageExecutor` so the resolver is reachable from the session-write path ### Fixed +- **Git push events deployed projects with auto-deploy disabled**: the `automatic_deploy` flag was stored on `DeploymentConfig`, exposed via API, and rendered in the project UI, but `process_git_push_event` (in `temps-deployments/src/services/job_processor.rs`) never read it before queuing a deployment. Toggling "deploy on push" off had no effect — every webhook still produced a build, container, and rollout. The push pipeline now resolves the effective flag via a new `is_automatic_deploy_enabled(project_cfg, env_cfg)` helper that mirrors `DeploymentConfig::merge` semantics (env overrides project; OR-on-booleans; both-`None` → false, since auto-deploy is opt-in) and returns early with an `info!` log when disabled, before the duplicate-deployment scan, in-flight cancellation, or job-queue work runs. Both GitHub and GitLab webhooks funnel through `git_provider_manager.handle_push_event` → `GitPushEventJob`, so the single gate covers both providers. Six unit tests cover each project/env combination - **Log History pagination was emitted but never honored**: `archive_search` returned a `next_cursor` in the response but the search service ignored `filter.cursor` on the way back in, so clicking the next-page chevron returned the same first page. Cursor is now parsed (`:`), narrows the effective `end_time`, and a strict `< cursor.ts` check inside the chunk scan prevents the boundary line from being returned twice. Pagination tests previously asserted only `lines.length <= page_size`, which couldn't have caught this — replaced with a real test that fetches page 1, sends its cursor as page 2's request, and verifies both no-overlap and strictly-older timestamps - **Log History silently truncated wide time windows to the OLDEST 200 lines**: `archive_search` ran chunks in `find_chunks` order (oldest-first by insertion), accumulated matches into an unbounded `Vec`, then `break`'d the chunk loop the moment `all_matches.len() > page_size`. Result: a "Last 6 hours" query returned roughly the same lines as "Last 1 hour" because the scanner bailed before reading any chunk past the start of the window — and the lines it did return were the oldest in the window, not the newest. Replaced with a bounded `BinaryHeap>` of size `page_size + 1`. Memory stays at O(K) regardless of total matches; time is O(N log K). All chunks are now scanned. Regression test seeds two batches at -3h30m and -30m, asserts that "Last 1h" returns 5 lines and "Last 4h" returns all 10 — would have failed under the old early-exit - **`total_scanned` was a misleading raw byte count, not a match count**: the field was incremented before the time/level/service/text filter ran, so a query showing "200 logs (344 scanned)" implied 144 lines were filtered out by the user's level/text predicates. In reality most of those 144 were just chunk bleed-through outside the requested time window — chunk files store many minutes of logs and the scanner reads the whole file then filters. Counter now increments only after every filter passes, so the displayed number actually reflects "matches in window" the way users expect diff --git a/crates/temps-deployments/src/services/job_processor.rs b/crates/temps-deployments/src/services/job_processor.rs index aa658c39af..2793ae73c5 100644 --- a/crates/temps-deployments/src/services/job_processor.rs +++ b/crates/temps-deployments/src/services/job_processor.rs @@ -567,6 +567,22 @@ async fn copy_environment_variables_to_preview( Ok(()) } +/// Returns true if a git push should auto-deploy given the project and +/// environment deployment configs. The merge mirrors `DeploymentConfig::merge` +/// (env overrides project; OR-semantics for booleans). When both configs are +/// absent the answer is false — auto-deploy is opt-in, not opt-out. +fn is_automatic_deploy_enabled( + project_config: Option<&temps_entities::deployment_config::DeploymentConfig>, + environment_config: Option<&temps_entities::deployment_config::DeploymentConfig>, +) -> bool { + match (project_config, environment_config) { + (Some(project_cfg), Some(env_cfg)) => project_cfg.merge(env_cfg).automatic_deploy, + (Some(project_cfg), None) => project_cfg.automatic_deploy, + (None, Some(env_cfg)) => env_cfg.automatic_deploy, + (None, None) => false, + } +} + // Extracted free function for testing async fn process_git_push_event( workflow_planner: Arc, @@ -622,6 +638,24 @@ async fn process_git_push_event( } }; + // ── Auto-deploy gate ───────────────────────────────────────────────── + // + // Respect the user's "deploy on push" setting before doing any work. + // The effective value is computed by merging project + environment + // deployment configs (env overrides project). When both sides have + // automatic_deploy=false, push events must NOT trigger a deployment — + // the user has explicitly opted out. + if !is_automatic_deploy_enabled( + project.deployment_config.as_ref(), + environment.deployment_config.as_ref(), + ) { + info!( + "Skipping push event for project {} environment {} ({}): automatic_deploy is disabled", + project.id, environment.id, environment.name + ); + return; + } + // Check for duplicate deployment (same project, environment, and commit) // This prevents duplicate deployments from being created if: // - Multiple webhook URLs are configured in GitHub (both /webhook/git/github/events and /webhook/source/github/events) @@ -1904,4 +1938,54 @@ mod tests { Ok(()) } + + fn cfg_with_auto_deploy(value: bool) -> temps_entities::deployment_config::DeploymentConfig { + temps_entities::deployment_config::DeploymentConfig { + automatic_deploy: value, + ..Default::default() + } + } + + #[test] + fn auto_deploy_disabled_when_both_configs_missing() { + assert!(!is_automatic_deploy_enabled(None, None)); + } + + #[test] + fn auto_deploy_disabled_when_project_off_and_env_missing() { + let project_cfg = cfg_with_auto_deploy(false); + assert!(!is_automatic_deploy_enabled(Some(&project_cfg), None)); + } + + #[test] + fn auto_deploy_enabled_when_project_on_and_env_missing() { + let project_cfg = cfg_with_auto_deploy(true); + assert!(is_automatic_deploy_enabled(Some(&project_cfg), None)); + } + + #[test] + fn auto_deploy_disabled_when_both_sides_off() { + let project_cfg = cfg_with_auto_deploy(false); + let env_cfg = cfg_with_auto_deploy(false); + assert!(!is_automatic_deploy_enabled( + Some(&project_cfg), + Some(&env_cfg) + )); + } + + #[test] + fn auto_deploy_enabled_when_env_overrides_project_off() { + let project_cfg = cfg_with_auto_deploy(false); + let env_cfg = cfg_with_auto_deploy(true); + assert!(is_automatic_deploy_enabled( + Some(&project_cfg), + Some(&env_cfg) + )); + } + + #[test] + fn auto_deploy_enabled_when_only_env_set_on() { + let env_cfg = cfg_with_auto_deploy(true); + assert!(is_automatic_deploy_enabled(None, Some(&env_cfg))); + } } diff --git a/crates/temps-observability/src/filters.rs b/crates/temps-observability/src/filters.rs index db7d10ac0d..7e6d4d56cc 100644 --- a/crates/temps-observability/src/filters.rs +++ b/crates/temps-observability/src/filters.rs @@ -25,6 +25,11 @@ pub struct EventFilters { pub environment_id: Option, pub search: Option, pub limit: u64, + /// When `Some(true)`, exclude rows flagged as bots (`is_bot = true`). + /// When `Some(false)`, only include bot rows. `None` includes everything. + /// Currently only applied to the `Request` kind — other kinds don't + /// carry a bot flag. + pub hide_bots: Option, } impl EventFilters { @@ -233,6 +238,7 @@ mod tests { environment_id: None, search: None, limit: 50, + hide_bots: None, }; assert!(matches!( f.validate(), @@ -251,6 +257,7 @@ mod tests { environment_id: None, search: None, limit: 50, + hide_bots: None, }; assert!(f.validate().is_ok()); } diff --git a/crates/temps-observability/src/handlers/events.rs b/crates/temps-observability/src/handlers/events.rs index a68431cc1a..ff25fe5cbe 100644 --- a/crates/temps-observability/src/handlers/events.rs +++ b/crates/temps-observability/src/handlers/events.rs @@ -71,6 +71,10 @@ pub struct EventsQuery { pub search: Option, /// Page size (default 50, max 200). pub limit: Option, + /// When `true`, exclude bot/crawler request rows. When `false`, only + /// include bot rows. Omitted means "include everything" (default). + /// Only affects the `Request` kind. + pub hide_bots: Option, } #[derive(Debug, Serialize, ToSchema)] @@ -125,6 +129,7 @@ pub async fn observability_list_events( environment_id: query.environment_id, search: query.search, limit, + hide_bots: query.hide_bots, }; let events = state.service.query(filters).await?; diff --git a/crates/temps-observability/src/service.rs b/crates/temps-observability/src/service.rs index d5a0c6b0e9..07868d8224 100644 --- a/crates/temps-observability/src/service.rs +++ b/crates/temps-observability/src/service.rs @@ -247,6 +247,19 @@ impl ObservabilityService { if let Some(ref s) = filters.search { q = q.filter(proxy_logs::Column::Path.contains(s)); } + if let Some(hide) = filters.hide_bots { + if hide { + // Treat NULL `is_bot` as not-a-bot so we don't hide rows + // that simply lack detection metadata (older rows). + q = q.filter( + proxy_logs::Column::IsBot + .eq(false) + .or(proxy_logs::Column::IsBot.is_null()), + ); + } else { + q = q.filter(proxy_logs::Column::IsBot.eq(true)); + } + } let rows = q .order_by_desc(proxy_logs::Column::Timestamp) .limit(filters.limit) diff --git a/web/src/api/client/types.gen.ts b/web/src/api/client/types.gen.ts index ad500f48fd..f34be31f4a 100644 --- a/web/src/api/client/types.gen.ts +++ b/web/src/api/client/types.gen.ts @@ -32298,6 +32298,12 @@ export type ObservabilityListEventsData = { * Page size (default 50, max 200). */ limit?: number; + /** + * When `true`, exclude bot/crawler request rows. When `false`, only + * include bot rows. Omitted means "include everything" (default). + * Only affects the `Request` kind. + */ + hide_bots?: boolean; }; url: '/projects/{project_id}/observe/events'; }; diff --git a/web/src/components/observe/ObserveFilterBar.tsx b/web/src/components/observe/ObserveFilterBar.tsx index 2138788d78..bf46c3fb5e 100644 --- a/web/src/components/observe/ObserveFilterBar.tsx +++ b/web/src/components/observe/ObserveFilterBar.tsx @@ -43,6 +43,7 @@ export interface ObserveFilters { timeRange: TimeRange search: string environmentId: number | null + hideBots: boolean } export function ObserveFilterBar({ diff --git a/web/src/components/observe/useObserveQuery.ts b/web/src/components/observe/useObserveQuery.ts index 6622129805..cec9b5c792 100644 --- a/web/src/components/observe/useObserveQuery.ts +++ b/web/src/components/observe/useObserveQuery.ts @@ -12,6 +12,7 @@ interface UseObserveQueryArgs { environmentId?: number search?: string limit?: number + hideBots?: boolean } /** @@ -29,6 +30,7 @@ export function useObserveQuery(args: UseObserveQueryArgs) { environmentId, search, limit, + hideBots, } = args // Omit `kinds` when the user has every kind selected so the server @@ -49,6 +51,10 @@ export function useObserveQuery(args: UseObserveQueryArgs) { environment_id: environmentId, search: search || undefined, limit, + // Only send `hide_bots` when actively filtering out bots — leaving + // it unset means "include everything" so we keep URL/query state + // minimal in the default-on-bots case. + hide_bots: hideBots ? true : undefined, }, }), refetchInterval: 15_000, diff --git a/web/src/pages/Observe.tsx b/web/src/pages/Observe.tsx index 95f45c5fb6..b8d37d8078 100644 --- a/web/src/pages/Observe.tsx +++ b/web/src/pages/Observe.tsx @@ -1,4 +1,6 @@ import { ProjectResponse } from '@/api/client' +import { usePageTitle } from '@/hooks/usePageTitle' +import { Button } from '@/components/ui/button' import { Skeleton } from '@/components/ui/skeleton' import { Input } from '@/components/ui/input' import { @@ -22,6 +24,7 @@ import { } from '@/components/observe/types' import { AlertOctagon, + Bot, CircleDollarSign, Inbox, Network, @@ -77,6 +80,7 @@ function sameKindSet( * "here's the link, look at the 500s after the deploy at 14:02". */ export default function Observe({ project }: ObserveProps) { + usePageTitle(`Observe · ${project.name}`) const [searchParams, setSearchParams] = useSearchParams() const [selectedEvent, setSelectedEvent] = useState(null) @@ -98,6 +102,11 @@ export default function Observe({ project }: ObserveProps) { ? timeRangeParam : '24h' + // Bots hidden by default (mirrors ResourceMonitoring). Only the + // explicit `hide_bots=false` URL param flips it off so the default + // URL stays clean. + const hideBots = searchParams.get('hide_bots') !== 'false' + return { kinds: kinds.length > 0 ? kinds : ([...DEFAULT_KINDS] as EventKind[]), timeRange, @@ -105,6 +114,7 @@ export default function Observe({ project }: ObserveProps) { environmentId: searchParams.get('environment_id') ? Number(searchParams.get('environment_id')) : null, + hideBots, } }, [searchParams]) @@ -130,6 +140,13 @@ export default function Observe({ project }: ObserveProps) { } else { params.delete('environment_id') } + // Bots hidden is the default — only persist when the user has flipped + // it off so the URL doesn't carry redundant state. + if (next.hideBots) { + params.delete('hide_bots') + } else { + params.set('hide_bots', 'false') + } setSearchParams(params, { replace: true }) } @@ -145,6 +162,7 @@ export default function Observe({ project }: ObserveProps) { environmentId: filters.environmentId ?? undefined, search: filters.search || undefined, limit: 100, + hideBots: filters.hideBots, }) const events = query.data?.events ?? [] @@ -153,7 +171,7 @@ export default function Observe({ project }: ObserveProps) { return ( <> -
    +
    +
    -

    +

    Observe

    @@ -312,8 +330,8 @@ function CockpitHeader({ className={cn( 'group flex flex-col gap-2 rounded-lg border p-3 text-left transition-colors', active - ? 'border-zinc-700 bg-zinc-900' - : 'border-dashed border-zinc-800 bg-zinc-950 opacity-60 hover:opacity-100', + ? 'border-border bg-card' + : 'border-dashed border-border bg-background opacity-60 hover:opacity-100', )} >
    @@ -323,7 +341,7 @@ function CockpitHeader({ {meta.label}
    - + {active ? total : 'off'}

    @@ -335,7 +353,7 @@ function CockpitHeader({
    - + onChange({ ...filters, search: e.target.value }) } - className="border-zinc-800 bg-zinc-900 pl-9 font-mono text-zinc-200 placeholder:text-zinc-600" + className="pl-9 font-mono" />
    +
    ) @@ -442,14 +478,14 @@ function ConsoleRow({ type="button" onClick={onClick} className={cn( - 'group flex w-full items-baseline gap-3 border-b border-zinc-900 border-l-2 px-4 py-1.5 text-left text-zinc-300 hover:bg-zinc-900/60', + 'group flex w-full items-baseline gap-3 border-b border-border border-l-2 px-4 py-1.5 text-left text-foreground hover:bg-muted/60', meta.gutter, )} > @@ -458,7 +494,7 @@ function ConsoleRow({ {primary} {suffix && ( - {suffix} + {suffix} )} ) @@ -525,12 +561,12 @@ function ListSkeleton() { {Array.from({ length: 12 }).map((_, i) => (
    - - - - + + + +
    ))}
    @@ -539,7 +575,7 @@ function ListSkeleton() { function EmptyState() { return ( -
    +

    No events match the current filters.

    Try widening the time range or toggling kinds.

    @@ -553,7 +589,7 @@ function ErrorState({ message }: { message: string }) {

    Failed to load events

    -

    {message}

    +

    {message}

    ) }