Skip to content

fix: clear structured logs after authorization denial - #1047

Merged
sfmskywalker merged 2 commits into
mainfrom
codex/preview-ux-feedback
Sep 17, 2026
Merged

sfmskywalker merged 2 commits into
mainfrom
codex/preview-ux-feedback

Conversation

@sfmskywalker

Copy link
Copy Markdown
Member

The Structured Logs page could leave buffered rows visible after REST or SignalR authorization failed. It now treats 401/403 as a terminal denied state, clears rows, selected details, source metadata and counters, disposes the observer, and suppresses late callbacks.

Validation: the focused Structured Logs suite passes 21/21 and git diff --check passes. Root self-review removed duplicate observer disposal and added an idempotency assertion.

Refs #1045 and valence-works/elsa-control#475.

private sealed class DenyingHandler(HttpStatusCode statusCode) : HttpMessageHandler
{
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) =>
Task.FromResult(new HttpResponseMessage(statusCode));
@greptile-apps

greptile-apps Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

Safe to merge with a non-blocking concern: users lose usable REST-loaded diagnostic history if the live SignalR hub is absent.

Findings

  1. P2 Preserve history when unavailable
Fix with agent prompt
### Issue 1
src/modules/Elsa.Studio.Diagnostics.StructuredLogs/UI/Pages/StructuredLogs.razor.cs:643-646
When the live SignalR structured-logs hub returns 404, it reports `Unavailable`. REST sources and historical rows have already loaded at this point, but this branch sends that state through the terminal failure path and clears them. Keep `Unavailable` as a live-subscription failure so users can still inspect available REST history; only authorization denial should purge buffered data. This is a non-blocking concern, but it makes diagnostics unavailable when only live updates are missing.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

  • The diagnostics page removes already loaded REST structured-log history when the optional live SignalR hub is unavailable.
  • This is a non-blocking usability concern, but preserving the available history would let users continue investigating past events when live updates cannot connect.

Reviews (1) · Last reviewed commit: "Merge pull request #1046 from elsa-workf..."

Comment on lines +643 to +646
if (status is StructuredLogConnectionStatus.Unauthorized or StructuredLogConnectionStatus.Unavailable)
{
await EnterTerminalFailureAsync(status);
return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Preserve history when unavailable

When the live SignalR structured-logs hub returns 404, it reports Unavailable. REST sources and historical rows have already loaded at this point, but this branch sends that state through the terminal failure path and clears them. Keep Unavailable as a live-subscription failure so users can still inspect available REST history; only authorization denial should purge buffered data. This is a non-blocking concern, but it makes diagnostics unavailable when only live updates are missing.

Knowledge Base Used: Structured log diagnostics

Artifacts

Evidence from the check

  • The exact shell script authored and executed to attempt the focused bUnit runtime check, then to assert the narrow source conditions when the required .NET runtime was unavailable; it documents the reproducible verification path.

Command output from the check

  • Output captured from executing the evidence script in `/home/user/repo`; it records the missing dotnet blocker and passing focused assertions, confirming the status mapping, load order, and clear behavior.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/modules/Elsa.Studio.Diagnostics.StructuredLogs/UI/Pages/StructuredLogs.razor.cs
Line: 643-646

Comment:
**Preserve history when unavailable**

When the live SignalR structured-logs hub returns 404, it reports `Unavailable`. REST sources and historical rows have already loaded at this point, but this branch sends that state through the terminal failure path and clears them. Keep `Unavailable` as a live-subscription failure so users can still inspect available REST history; only authorization denial should purge buffered data. This is a non-blocking concern, but it makes diagnostics unavailable when only live updates are missing.

**Knowledge Base Used:** [Structured log diagnostics](https://app.greptile.com/skywalker-digital/-/custom-context/knowledge-base/elsa-workflows/elsa-studio/-/docs/structured-log-diagnostics.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@sfmskywalker
sfmskywalker merged commit a0c1315 into main Sep 17, 2026
10 checks passed
@sfmskywalker
sfmskywalker deleted the codex/preview-ux-feedback branch September 17, 2026 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant