[MISC] Show alert with execution-logs link on workflow run#1997
Merged
Conversation
The old WebSocket-based progress updates on the Agency page no longer fire, leaving the UI silent after a workflow run is triggered. Surface a sticky info notification with a react-router link to the execution's logs page so the user can navigate there (browser-back returns to the workflow page via SPA history). Extracts a shared NotificationIdLine component so the new Execution ID and the existing Request ID render through one renderer, and generalises the related CSS class to .notification-id-line.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughThe PR adds execution ID support to alerts and notifications (component, CSS, store, and App integration), displays a workflow-start alert with a logs link from Agency, centralizes back-navigation via a new hook and wires it into logs pages, removes a DetailedLogs back button, and makes worker log messages JSON-safe before emit. ChangesExecution ID notification flow
Back navigation and logs pages
Worker log emission
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/agency/agency/Agency.jsx`:
- Line 786: The alert link is missing the org prefix; update the string where
content is built (the template using execIdValue) to include the org scope by
prepending the org variable (e.g., change `/logs/WF/${execIdValue}` to
`/${orgName}/logs/WF/${execIdValue}` or the appropriate org identifier in
scope), ensuring the route becomes org-scoped
(/${orgName}/logs/WF/${executionId}/) so navigation matches other flows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 40b30b49-a185-46ba-81db-a3611a3ebc02
📒 Files selected for processing (5)
frontend/src/App.jsxfrontend/src/components/agency/agency/Agency.jsxfrontend/src/components/notification/NotificationIdLine.jsxfrontend/src/index.cssfrontend/src/store/alert-store.js
Contributor
|
| Filename | Overview |
|---|---|
| frontend/src/store/alert-store.js | Adds executionId: null to the default alert shape (additive, safe); the key is still always auto-generated, making caller-supplied deduplication of sticky notifications impossible. |
| frontend/src/components/agency/agency/Agency.jsx | Adds sticky info notification after handleInitialExecution returns an execution ID; the duration: 0 combined with unconditional key generation in the store means every Run Workflow click stacks a permanent banner with no way to replace the previous one. |
| frontend/src/hooks/useBackNavigation.js | New generic hook: prefers location.state.from, falls back to navigate(-1) (SPA history present), then fallbackPath. Clean priority chain; dependency array is correct. |
| frontend/src/components/notification/NotificationIdLine.jsx | Tiny new shared component for rendering a labelled, copyable ID line (with optional stacked layout). Null-guards value, PropTypes correct. |
| frontend/src/App.jsx | Replaces inline Request-ID JSX with NotificationIdLine; adds Execution-ID rendering; builds logSuffix from whichever IDs are present. Refactor is clean and backward-compatible. |
| frontend/src/components/logging/execution-logs/ExecutionLogs.jsx | Replaces inline back-route computation with useBackNavigation; passes onNavigateBack to ToolNavBar. Backward-compatible since ToolNavBar still accepts previousRoute. |
| frontend/src/components/metrics-dashboard/RecentActivity.jsx | Corrects navigation state from: 'dashboard' → from: /${orgName}/dashboard so useBackNavigation receives a routable absolute path instead of a label string. |
| workers/log_consumer/tasks.py | Round-trips log_message through json.dumps(default=str) + json.loads to coerce non-primitive types (UUID, datetime) before Socket.IO emission. Safe defensive fix. |
Sequence Diagram
sequenceDiagram
participant U as User (Agency page)
participant AJ as Agency.jsx
participant AS as alert-store
participant APP as App.jsx
participant NT as Ant Design Notification
participant LP as Logs Page
U->>AJ: Click Run Workflow
AJ->>AJ: handleInitialExecution()
AJ->>AJ: handleWfExecutionApi(body) [1st call]
AJ-->>AS: "setAlertDetails({ type:info, executionId, duration:0 })"
AS-->>APP: alertDetails updated (new unique key)
APP->>NT: notificationAPI.open() [sticky info banner]
NT-->>U: Workflow run started + View logs link + copyable Execution ID
AJ->>AJ: handleWfExecutionApi(body) [2nd call]
alt "execution_status === ERROR"
AJ-->>AS: "setAlertDetails({ type:error, content })"
AS-->>APP: alertDetails updated (new unique key)
APP->>NT: notificationAPI.open() [error banner]
NT-->>U: Both sticky info AND error banners shown
end
U->>LP: Click View logs link (no state passed)
LP->>LP: useBackNavigation — navigate(-1)
LP-->>U: Back to Agency page
Comments Outside Diff (1)
-
frontend/src/store/alert-store.js, line 37 (link)The
keyis always generated fresh, so a caller-suppliedkeyis silently discarded. Agency.jsx usesduration: 0(sticky), meaning every "Run Workflow" click appends a new permanent banner with no way to replace the previous one. Honouring a caller-supplied key as a fallback lets Agency.jsx pass a stable constant (e.g."workflow-run-notification") so each new run replaces rather than stacks the previous sticky banner.Prompt To Fix With AI
This is a comment left during a code review. Path: frontend/src/store/alert-store.js Line: 37 Comment: The `key` is always generated fresh, so a caller-supplied `key` is silently discarded. Agency.jsx uses `duration: 0` (sticky), meaning every "Run Workflow" click appends a new permanent banner with no way to replace the previous one. Honouring a caller-supplied key as a fallback lets Agency.jsx pass a stable constant (e.g. `"workflow-run-notification"`) so each new run replaces rather than stacks the previous sticky banner. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
frontend/src/store/alert-store.js:37
The `key` is always generated fresh, so a caller-supplied `key` is silently discarded. Agency.jsx uses `duration: 0` (sticky), meaning every "Run Workflow" click appends a new permanent banner with no way to replace the previous one. Honouring a caller-supplied key as a fallback lets Agency.jsx pass a stable constant (e.g. `"workflow-run-notification"`) so each new run replaces rather than stacks the previous sticky banner.
```suggestion
key: details.key || `open${Date.now()}-${uniqueId()}`,
```
Reviews (4): Last reviewed commit: "Merge branch 'main' into fix/workflow-ru..." | Re-trigger Greptile
…k button
Adds useBackNavigation hook so any page that links to the execution
logs page returns to where the user came from. Priority:
1. location.state.from (explicit caller hint; restores scroll state)
2. navigate(-1) when SPA history exists (covers markdown links in
alerts and any future linker that didn't pass state)
3. fallback path (deep link / refresh)
Wires the ToolNavBar back button on ExecutionLogs through the hook and
removes the duplicate back button inside DetailedLogs (the cameFromDashboard
branch is subsumed; RecentActivity now passes a real path in state.from).
Contributor
|
Actionable comments posted: 0 |
chandrasekharan-zipstack
changed the title
Socket.IO's stdlib-json serializer raises on UUID/datetime values in the log payload, which silently drops the WebSocket event from the log consumer. Round-trip through json.dumps(..., default=str) so non- primitive types degrade to strings instead of failing the emit.
Contributor
|
Actionable comments posted: 0 |
muhammad-ali-e
approved these changes
May 29, 2026
johnyrahul
approved these changes
May 29, 2026
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
Contributor
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
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
3 participants
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.
What
View logslink that opens the logs page filtered to the just-started execution. Browser back returns to the workflow page via SPA history.Execution IDas a stacked, copyable code value below the message — same widget already used for theRequest IDon error alerts (UN-3444 [FEAT] Propagate frontend request ID and surface it on error notifications #1955).NotificationIdLinecomponent used by bothExecution ID(info / workflow-run) andRequest ID(error). Generalise the related CSS class from.notification-request-idto.notification-id-line(with a--stackedmodifier).Why
How
Agency.jsx#handleInitialExecution: after the firsthandleWfExecutionApicall returns anexecution_id, firesetAlertDetailswithtype: "info",contentcontaining a markdown link[View logs](/logs/WF/${execId}), and the newexecutionIdfield.duration: 0makes it sticky. Skipped for step-execution since stepping stays on-page.CustomMarkdownalready rewrites internal[text](/path)into a react-router<Link>and auto-prefixes the org name, so theView logslink participates in SPA history (browser back works).alert-store.js: addsexecutionId: nullto the default alert shape (additive, optional).App.jsx: replaces the inlineRequest IDJSX with<NotificationIdLine label value [stacked] />; renders bothExecution ID(when set) andRequest ID(when set on error) using the same component. Log-panel suffix is built once from whichever ids are present.NotificationIdLine.jsx: tiny new component — secondary label +Typography.Text code copyable={{ text: value }}, with an optionalstackedvariant (label above the value) for the workflow-run case.index.css: renames.notification-request-id→.notification-id-line, adds.notification-id-line--stacked(flex-direction: column,align-items: flex-start).Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
Request IDline renders through the same component with identical props, so the existing error-alert layout/styling carries over 1:1 (CSS rules were renamed, not changed). The newexecutionIdfield is additive onalertDetails; alerts that don't set it are unaffected. The Agency.jsx alert only fires when anexecution_idis returned and we're not in step-execution mode.Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Workflow run started, bodyView logs to track progress(link), followed by a stackedExecution ID:label and a copyable code-styled UUID.View logslink → navigates to/{orgName}/logs/WF/{executionId}. Press the browser back button → returns to the workflow page.Request IDline inline as before; layout and class names map cleanly to the renamed CSS.Execution ID: \`` on its own line.Screenshots
Checklist
I have read and understood the Contribution Guidelines.