feat(observer): establish server-side observability primitives for TUI#3
Merged
andrewmelchor merged 3 commits intomainfrom Jan 21, 2026
Merged
feat(observer): establish server-side observability primitives for TUI#3andrewmelchor merged 3 commits intomainfrom
andrewmelchor merged 3 commits intomainfrom
Conversation
This introduces flow/execution correlation and server-backed storage so a TUI can observe runs in real-time (SSE) and fetch full execution details on demand. - flows: create/finish + TTL/limits + flow-scoped sequencing - executions: reqExecId, requestQueued, lifecycle tracking, error finalization - storage: GET execution detail by flowId/reqExecId - workspace: list .http files and requests for navigation - security: session variable redaction; SSE filter requirements under auth - core: createRemoteClient for “single import change” observability + ordered var sync - tests: update parse calls for includeDiagnostics typing; Biome formatting
Deploying docs with
|
| Latest commit: |
82373d2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ea0ba4b3.docs-6vq.pages.dev |
| Branch Preview URL: | https://feat-tui-phase-1-bootstrap.docs-6vq.pages.dev |
Member
Author
|
@greptile |
Greptile SummaryIntroduces comprehensive observability infrastructure for the TUI by establishing flow-based execution correlation and server-backed storage. The implementation enables real-time monitoring through SSE with flow-scoped event sequencing. Key Changes:
Issues Found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Remote Client
participant Server as treq serve
participant EventMgr as Event Manager
participant TUI as TUI Observer
Note over Client,TUI: Initialization Phase
Client->>Server: POST /session (variables)
Server-->>Client: sessionId
Client->>Server: POST /flows (sessionId, label)
Server->>EventMgr: emit(flowStarted)
EventMgr->>TUI: SSE: flowStarted
Server-->>Client: flowId
Note over Client,TUI: Execution Phase
Client->>Client: setVariable(key, value)
Client->>Server: PUT /session/{id}/variables
Server-->>Client: 200 OK
Client->>Server: POST /execute (flowId, sessionId, path)
Server->>EventMgr: emit(requestQueued, seq=1)
EventMgr->>TUI: SSE: requestQueued
Server->>Server: Create pending execution
Server->>Server: Parse & resolve config
Server->>EventMgr: emit(fetchStarted, seq=2)
EventMgr->>TUI: SSE: fetchStarted
Server->>Server: Update exec status=running
Server->>Server: HTTP request execution
Server->>EventMgr: emit(fetchFinished, seq=3)
EventMgr->>TUI: SSE: fetchFinished
Server->>Server: Store execution (status=success)
Server-->>Client: ExecuteResponse (reqExecId)
Note over Client,TUI: Completion Phase
Client->>Server: POST /flows/{flowId}/finish
Server->>Server: Calculate flow summary
Server->>EventMgr: emit(flowFinished, summary)
EventMgr->>TUI: SSE: flowFinished
Server-->>Client: FlowSummary
Note over Client,TUI: Observer Retrieval
TUI->>Server: GET /flows/{flowId}/executions/{reqExecId}
Server-->>TUI: ExecutionDetail (sanitized headers/variables)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments (1)
-
packages/app/src/server/service.ts, line 255-267 (link)logic: Arrays containing objects with sensitive keys won't be sanitized
The function recursively sanitizes nested objects but skips arrays. If
variablescontains[{token: "secret"}], the sensitive data will leak.
16 files reviewed, 3 comments
…sync Sanitize nested variable structures including arrays to prevent sensitive-key leaks. Queue remote client variable updates to avoid races and ensure executions see latest values. Add regression tests for nested/array sanitization.
Comment on lines
+238
to
243
| // Require sessionId or flowId when auth is enabled (prevents cross-session leakage) | ||
| if (config.token && !sessionId && !flowId) { | ||
| throw new ValidationError( | ||
| 'sessionId query parameter is required when authentication is enabled' | ||
| 'sessionId or flowId query parameter is required when authentication is enabled' | ||
| ); | ||
| } |
There was a problem hiding this comment.
logic: Consider stronger enforcement when auth is enabled
When config.token is set, the code requires sessionId OR flowId, but doesn't validate that provided values exist. An attacker could subscribe to arbitrary sessionId/flowId values without verification.
Suggested change
| // Require sessionId or flowId when auth is enabled (prevents cross-session leakage) | |
| if (config.token && !sessionId && !flowId) { | |
| throw new ValidationError( | |
| 'sessionId query parameter is required when authentication is enabled' | |
| 'sessionId or flowId query parameter is required when authentication is enabled' | |
| ); | |
| } | |
| // Require sessionId or flowId when auth is enabled (prevents cross-session leakage) | |
| if (config.token && !sessionId && !flowId) { | |
| throw new ValidationError( | |
| 'sessionId or flowId query parameter is required when authentication is enabled' | |
| ); | |
| } | |
| // Verify sessionId/flowId exist if provided (prevent enumeration) | |
| if (sessionId && !service.getSessions().has(sessionId)) { | |
| throw new SessionNotFoundError(sessionId); | |
| } | |
| if (flowId && !service.getFlows().has(flowId)) { | |
| throw new FlowNotFoundError(flowId); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/app/src/server/app.ts
Line: 238:243
Comment:
**logic:** Consider stronger enforcement when auth is enabled
When `config.token` is set, the code requires `sessionId` OR `flowId`, but doesn't validate that provided values exist. An attacker could subscribe to arbitrary sessionId/flowId values without verification.
```suggestion
// Require sessionId or flowId when auth is enabled (prevents cross-session leakage)
if (config.token && !sessionId && !flowId) {
throw new ValidationError(
'sessionId or flowId query parameter is required when authentication is enabled'
);
}
// Verify sessionId/flowId exist if provided (prevent enumeration)
if (sessionId && !service.getSessions().has(sessionId)) {
throw new SessionNotFoundError(sessionId);
}
if (flowId && !service.getFlows().has(flowId)) {
throw new FlowNotFoundError(flowId);
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+393
to
+404
| // Evict oldest flow when limit reached | ||
| function evictOldestFlow(): void { | ||
| let oldest: Flow | null = null; | ||
| for (const flow of flows.values()) { | ||
| if (!oldest || flow.lastActivityAt < oldest.lastActivityAt) { | ||
| oldest = flow; | ||
| } | ||
| } | ||
| if (oldest) { | ||
| flows.delete(oldest.id); | ||
| } | ||
| } |
There was a problem hiding this comment.
logic: Flow eviction doesn't check if flow is finished
evictOldestFlow evicts based on lastActivityAt without checking flow.finished. An active, unfinished flow could be evicted while a client is still using it, causing subsequent operations to fail with FlowNotFoundError.
Consider either:
- Skip evicting unfinished flows
- Emit a warning when evicting an unfinished flow
- Document that clients must call
finishFlowbefore flows can be safely evicted
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/app/src/server/service.ts
Line: 393:404
Comment:
**logic:** Flow eviction doesn't check if flow is finished
`evictOldestFlow` evicts based on `lastActivityAt` without checking `flow.finished`. An active, unfinished flow could be evicted while a client is still using it, causing subsequent operations to fail with `FlowNotFoundError`.
Consider either:
- Skip evicting unfinished flows
- Emit a warning when evicting an unfinished flow
- Document that clients must call `finishFlow` before flows can be safely evicted
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+1158
to
+1161
| // Set final status: use tracker status if failed, otherwise success | ||
| exec.status = execTracker.status === 'failed' ? 'failed' : 'success'; | ||
| exec.error = execTracker.error; | ||
|
|
There was a problem hiding this comment.
logic: Status determination logic may be incorrect
Line 1159 sets exec.status = 'success' if execTracker.status !== 'failed', but execTracker.status could still be 'pending' or 'running' if fetchStarted event didn't fire. This would incorrectly mark executions as successful.
Suggested change
| // Set final status: use tracker status if failed, otherwise success | |
| exec.status = execTracker.status === 'failed' ? 'failed' : 'success'; | |
| exec.error = execTracker.error; | |
| // Set final status: use tracker status if failed, otherwise success | |
| exec.status = execTracker.status === 'failed' ? 'failed' : | |
| execTracker.status === 'running' ? 'success' : | |
| 'success'; | |
| exec.error = execTracker.error; |
Does the engine always emit fetchStarted before completion, or can executions succeed without transitioning through running state?
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/app/src/server/service.ts
Line: 1158:1161
Comment:
**logic:** Status determination logic may be incorrect
Line 1159 sets `exec.status = 'success'` if `execTracker.status !== 'failed'`, but `execTracker.status` could still be `'pending'` or `'running'` if `fetchStarted` event didn't fire. This would incorrectly mark executions as successful.
```suggestion
// Set final status: use tracker status if failed, otherwise success
exec.status = execTracker.status === 'failed' ? 'failed' :
execTracker.status === 'running' ? 'success' :
'success';
exec.error = execTracker.error;
```
Does the engine always emit fetchStarted before completion, or can executions succeed without transitioning through running state?
How can I resolve this? If you propose a fix, please make it concise.Only evict finished flows when MAX_FLOWS is reached; if none are finished, throw FLOW_LIMIT_REACHED instead of deleting an in-progress flow. This prevents active clients from hitting FlowNotFoundError mid-flow.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This introduces flow/execution correlation and server-backed storage so a TUI can observe runs in real-time (SSE) and fetch full execution details on demand.
Type of Change
How Has This Been Tested?
Checklist