Expose workflow node summaries in verbose JSON - #2408
Conversation
Verbose workflow JSON returned raw event streams, forcing consumers to duplicate the CLI's lifecycle fold and leaving running nodes without start timestamps. Changes: - Reuse the shared node-summary reducer for verbose JSON output - Add startedAt metadata and an explicit --events escape hatch - Document and test ordering, truncation, lifecycle states, and failure behavior Fixes #2359
📝 WalkthroughWalkthroughThe CLI adds ChangesWorkflow verbose output
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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 |
Consolidated Review: PR #2408Date: 2026-08-03T09:04:30.000Z Executive SummaryThe available code-review report finds the CLI change focused, typed, and well aligned across command wiring, tests, raw-event compatibility, and documentation. It identifies one medium-severity contract issue: the newly documented node ordering is not deterministic when multiple lifecycle events share the same timestamp. SQLite records event timestamps at second precision, so this can occur for concurrently started DAG nodes. The other four specialist artifacts required for a complete five-agent synthesis were not present when this report was generated, so their review areas cannot be represented or attributed. Overall Verdict: REQUEST_CHANGES Auto-fix Candidates: 0 CRITICAL + HIGH issues Artifact Availability
Statistics
CRITICAL Issues (Must Fix)None found in the available artifact. HIGH Issues (Should Fix)None found in the available artifact. MEDIUM Issues (Options for User)Issue 1: Node summary order is nondeterministic for timestamp tiesSource Agent: code-review Problem: Verbose JSON exposes Options:
Recommendation: Fix now. Add // packages/core/src/db/workflow-events.ts
const result = await pool.query<WorkflowEventRow>(
`SELECT * FROM remote_agent_workflow_events
WHERE workflow_run_id = $1
ORDER BY created_at ASC, id ASC`,
[workflowRunId]
);LOW Issues (For Consideration)None found in the available artifact. Positive Observations
Suggested Follow-up Issues
Next Steps
Metadata
|
⚡ Self-Fix Report (Aggressive)Status: COMPLETE Fixes Applied (1 total)
View all fixes
Tests Added
Skipped (0)(none — all findings addressed) Suggested Follow-up Issues(none) Validation✅ Type check | ✅ Lint | ✅ Full workspace test suite Self-fix by Archon · aggressive mode · fixes pushed to |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/commands/workflow.ts (1)
2001-2050: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReset summary state on
node_startedre-entry.
node_startedpreserves an existing summary’sstatewhen it updates onlystartedAt, while terminal branches replace the full summary. If the samestep_nameenters a new run after a prior terminal result, that path reports the old terminal state instead of the current running state. Set the existing summary torunninghere, and clear/reset derived fields only on terminal completion.🤖 Prompt for 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. In `@packages/cli/src/commands/workflow.ts` around lines 2001 - 2050, Update the node_started branch in the event-summary loop so an existing entry in summaries is reset to state running when a step re-enters. Clear stale terminal-derived fields such as durationMs, outputPreview, and error while preserving the new startedAt; keep terminal branches responsible for populating those fields.
🧹 Nitpick comments (2)
packages/cli/src/commands/workflow.ts (2)
2106-2131: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winJSON verbose output cannot distinguish "no events" from "event fetch failed" in either command. Both
workflowStatusCommandandworkflowGetCommandfall back to an emptynodes/eventsarray on afetchVerboseEventsfailure, with no field signaling the failure to a JSON consumer, even though text mode prints an explicit'(node events unavailable — see logs)'line for the same failure.
packages/cli/src/commands/workflow.ts#L2106-L2131: in theworkflowStatusCommandJSON branch, add an explicit indicator (for examplenodesUnavailable: true) to the per-run object whenfetchVerboseEventsreportsfailed.packages/cli/src/commands/workflow.ts#L2217-L2225: apply the same indicator to theworkflowGetCommandJSON branch wheneventsFailedistrue.🤖 Prompt for 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. In `@packages/cli/src/commands/workflow.ts` around lines 2106 - 2131, Update the JSON verbose branches of workflowStatusCommand (packages/cli/src/commands/workflow.ts:2106-2131) and workflowGetCommand (packages/cli/src/commands/workflow.ts:2217-2225) to add an explicit nodesUnavailable indicator to each per-run result when fetchVerboseEvents reports failed or eventsFailed is true; preserve the existing empty nodes/events fallback for successful fetches and genuinely eventless runs.
2106-2110: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider signaling event-fetch failure explicitly in JSON output.
When
fetchVerboseEventsfails,runEventsfalls back to[], and the JSON payload gets an emptynodes(orevents) array with no other indication of the failure. Text mode prints' (node events unavailable — see logs)', but JSON mode has no equivalent field. A JSON consumer cannot tell a genuine empty-events run apart from a fetch failure.This is documented as an intentional fallback (see the comment above
fetchVerboseEvents) and is covered by tests, so it is not a functional defect. Consider adding a small explicit field (for examplenodesUnavailable: true) to the per-run object when the fetch failed, so automated consumers do not need to guess.Also applies to: 2121-2131
🤖 Prompt for 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. In `@packages/cli/src/commands/workflow.ts` around lines 2106 - 2110, The JSON branch of workflowStatusCommand should explicitly indicate when fetchVerboseEvents fails instead of representing the failure only as an empty nodes/events array. Track the runEvents fallback outcome and add a per-run unavailability field such as nodesUnavailable: true only for failed event fetches, while preserving the existing payload for successful fetches and genuine empty results.
🤖 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.
Outside diff comments:
In `@packages/cli/src/commands/workflow.ts`:
- Around line 2001-2050: Update the node_started branch in the event-summary
loop so an existing entry in summaries is reset to state running when a step
re-enters. Clear stale terminal-derived fields such as durationMs,
outputPreview, and error while preserving the new startedAt; keep terminal
branches responsible for populating those fields.
---
Nitpick comments:
In `@packages/cli/src/commands/workflow.ts`:
- Around line 2106-2131: Update the JSON verbose branches of
workflowStatusCommand (packages/cli/src/commands/workflow.ts:2106-2131) and
workflowGetCommand (packages/cli/src/commands/workflow.ts:2217-2225) to add an
explicit nodesUnavailable indicator to each per-run result when
fetchVerboseEvents reports failed or eventsFailed is true; preserve the existing
empty nodes/events fallback for successful fetches and genuinely eventless runs.
- Around line 2106-2110: The JSON branch of workflowStatusCommand should
explicitly indicate when fetchVerboseEvents fails instead of representing the
failure only as an empty nodes/events array. Track the runEvents fallback
outcome and add a per-run unavailability field such as nodesUnavailable: true
only for failed event fetches, while preserving the existing payload for
successful fetches and genuine empty results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9be5f735-95c3-4c51-825a-6658978ea0dd
📒 Files selected for processing (7)
packages/cli/src/cli.tspackages/cli/src/commands/workflow.test.tspackages/cli/src/commands/workflow.tspackages/core/src/db/workflow-events.since.integration.test.tspackages/core/src/db/workflow-events.test.tspackages/core/src/db/workflow-events.tspackages/docs-web/src/content/docs/reference/cli.md
|
Closing in favour of a fresh run against current This PR implements the #2365 contract correctly — It cannot merge as-is: #2389 landed in between and both touch Re-running #2359 against current |
1 participant
Summary
workflow statusandworkflow getexposed raw lifecycle events, forcing API consumers to recreate the CLI's node-state fold.nodessummaries by default;--eventspreserves access to raw event rows; CLI docs and parity coverage define the contract.UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory (list every module-to-module edge, mark changes):
cli.tscommands/workflow.ts--eventsforstatusandget.commands/workflow.ts@archon/core/db/workflow-eventscommands/workflow.tsnodes; raw rows require--events.workflow.test.tscommands/workflow.tsreference/cli.mdLabel Snapshot
risk: lowsize: Scli,docs,testscli:workflowChange Metadata
featurecliLinked Issue
Validation Evidence (required)
Commands and result summary:
bun run validate; CLI package tests, type check, lint, formatting, whitespace check, and manualstatus/get --verbose --jsonchecks also passed. Manual checks verified defaultnodes,--eventsraw rows, node states, and runningstartedAt..archon/edits made bundled-default validation stale.Security Impact (required)
No)No)No)No)Yes, describe risk and mitigation: N/A.Compatibility / Migration
Yes)No)No)Human Verification (required)
What was personally validated beyond CI:
workflow status --verbose --jsonandworkflow get --verbose --jsonemit ordered node summaries; the same commands with--eventsemit raw events.startedAt; summary ordering; output truncation; non-fatal event fetch failures.Side Effects / Blast Radius (required)
statusandgetverbose JSON output, CLI help/argument forwarding, command tests, and CLI reference documentation.eventsin default verbose JSON must opt in with--events.Rollback Plan (required)
a414f7d9fromdevafter merge.--eventsis an explicit per-invocation compatibility escape hatch.Risks and Mitigations
events.--events, document it, and test both payloads.Fixes #2359
Summary by CodeRabbit
--eventsoption to verbose JSON workflow status and get commands for raw event output.