feat: unify report artifacts into one folder, add graceful Ctrl+C cancellation#218
Open
jithin23-kv wants to merge 2 commits into
Open
feat: unify report artifacts into one folder, add graceful Ctrl+C cancellation#218jithin23-kv wants to merge 2 commits into
jithin23-kv wants to merge 2 commits into
Conversation
WalkthroughAutonomous runs now support Ctrl+C cancellation, preserve truncated reports and logs, derive report directories from ChangesAutonomous hunt interruption
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant AbortController
participant runAutonomous
participant SDKQuery
participant ReportWriter
User->>CLI: Press Ctrl+C
CLI->>AbortController: abort()
AbortController->>runAutonomous: signal aborted
runAutonomous->>SDKQuery: interrupt query
runAutonomous->>ReportWriter: write truncated report
ReportWriter-->>CLI: report and log paths
CLI-->>User: interrupted status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
jithin23-kv
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@runners/cli/src/commands/hunt.ts`:
- Around line 568-572: Update the markComplete call in the hunt command to pass
the run status, using "interrupted" when interrupted is true and "completed"
otherwise, instead of report.objectiveOutcome. Preserve reportDir, and only
expose the objective verdict through a separate field if the existing completion
payload supports it.
🪄 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: 6767df0c-24b3-4561-ae3e-b7ee92632408
📒 Files selected for processing (9)
core/src/autonomous/index.tscore/src/autonomous/orchestrator/run.tscore/src/autonomous/report/mapRunLog.tscore/src/autonomous/report/types.tscore/src/autonomous/report/writeReport.tsdocs/hunt.mdrunners/cli/src/commands/hunt.tsrunners/cli/src/commands/setup.tsrunners/cli/src/ui/server.ts
Comment on lines
+568
to
572
| const interrupted = ac.signal.aborted; | ||
|
|
||
| const { html, json, dir } = await writeAutonomousReport(report, huntOptions.outputDir); | ||
|
|
||
| uiHandle?.markComplete({ reportDir: dir, outcome: report.objectiveOutcome }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how UiBridge.markComplete consumes/typechecks `outcome`, and how the frontend renders it.
fd -e ts -e tsx bridge --type f
rg -nP -C4 '\bmarkComplete\s*\(' --type=ts
rg -nP -C3 '\boutcome\b' runners/cli/src/uiRepository: KeyValueSoftwareSystems/agent-opfor
Length of output: 202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== bridge.ts outline ==\n'
ast-grep outline runners/cli/src/ui/bridge.ts --view expanded || true
printf '\n== server.ts markComplete usages ==\n'
rg -n -C4 'markComplete\s*\(' runners/cli/src/ui runners/cli/src/commands || true
printf '\n== outcome references in ui ==\n'
rg -n -C3 '\boutcome\b' runners/cli/src/ui || true
printf '\n== relevant file sizes ==\n'
wc -l runners/cli/src/ui/bridge.ts runners/cli/src/ui/server.ts runners/cli/src/commands/hunt.ts || trueRepository: KeyValueSoftwareSystems/agent-opfor
Length of output: 7287
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' runners/cli/src/ui/bridge.ts
printf '\n--- server.ts ---\n'
sed -n '1,260p' runners/cli/src/ui/server.ts
printf '\n--- hunt.ts slice ---\n'
sed -n '540,590p' runners/cli/src/commands/hunt.tsRepository: KeyValueSoftwareSystems/agent-opfor
Length of output: 16257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== outcome consumers across repo ==\n'
rg -n -C3 'completePayload|kind === "complete"|outcome' runners/cli/src runners/cli/ui runners/cli/src/ui-static . || true
printf '\n== snapshot.ts ==\n'
sed -n '1,240p' runners/cli/src/ui/snapshot.ts
printf '\n== UI static files touching complete/outcome ==\n'
rg -n -C3 'complete|outcome|interrupted|completed' runners/cli/src/ui-static runners/cli/src/ui || trueRepository: KeyValueSoftwareSystems/agent-opfor
Length of output: 43159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,240p' runners/cli/src/ui/snapshot.ts
printf '\n--- ui-static search ---\n'
rg -n -C3 'complete|outcome|interrupted|completed' runners/cli/src/ui-static runners/cli/src/ui || trueRepository: KeyValueSoftwareSystems/agent-opfor
Length of output: 11274
Send the run status here, not report.objectiveOutcome.
UiBridge.markComplete() is what the dashboard reads for the final label, and setup-mode already uses "interrupted" / "completed" in this field. Using the assessment verdict here makes a cancelled hunt --ui run look like a normal completion. If the verdict also needs to be shown, add a separate field.
Suggested fix
- uiHandle?.markComplete({ reportDir: dir, outcome: report.objectiveOutcome });
+ uiHandle?.markComplete({
+ reportDir: dir,
+ outcome: interrupted ? "interrupted" : "completed",
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const interrupted = ac.signal.aborted; | |
| const { html, json, dir } = await writeAutonomousReport(report, huntOptions.outputDir); | |
| uiHandle?.markComplete({ reportDir: dir, outcome: report.objectiveOutcome }); | |
| const interrupted = ac.signal.aborted; | |
| const { html, json, dir } = await writeAutonomousReport(report, huntOptions.outputDir); | |
| uiHandle?.markComplete({ | |
| reportDir: dir, | |
| outcome: interrupted ? "interrupted" : "completed", | |
| }); |
🤖 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 `@runners/cli/src/commands/hunt.ts` around lines 568 - 572, Update the
markComplete call in the hunt command to pass the run status, using
"interrupted" when interrupted is true and "completed" otherwise, instead of
report.objectiveOutcome. Preserve reportDir, and only expose the objective
verdict through a separate field if the existing completion payload supports it.
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 (2)
runners/cli/src/commands/hunt.ts (2)
550-550: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winShell-escape
liveLogPathin the displayedtailcommand.Paths supplied through
--outputmay contain spaces or shell metacharacters, making the printed command fail when pasted. Render a properly shell-quoted path.🤖 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 `@runners/cli/src/commands/hunt.ts` at line 550, Update the live-log message in the hunt command to shell-quote liveLogPath before interpolating it into the displayed tail command. Preserve the existing message and path value while ensuring paths containing spaces or shell metacharacters remain safe and pasteable.
464-487: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle errors on both log streams
createWriteStream()can still emiterrorlater on open/write failures (permissions, disk full). Adderrorlisteners forliveLogandeventLog, disable the failed sink, and keep the partial report path from crashing.🤖 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 `@runners/cli/src/commands/hunt.ts` around lines 464 - 487, Attach error listeners to both the liveLog and eventLog write streams after they are created, so asynchronous open/write failures disable the affected sink instead of propagating and crashing the run. Update the emit and emitEvent paths to skip a sink once it has failed, while preserving output through the remaining stream and the partial report flow.
🤖 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 `@runners/cli/src/commands/hunt.ts`:
- Line 550: Update the live-log message in the hunt command to shell-quote
liveLogPath before interpolating it into the displayed tail command. Preserve
the existing message and path value while ensuring paths containing spaces or
shell metacharacters remain safe and pasteable.
- Around line 464-487: Attach error listeners to both the liveLog and eventLog
write streams after they are created, so asynchronous open/write failures
disable the affected sink instead of propagating and crashing the run. Update
the emit and emitEvent paths to skip a sink once it has failed, while preserving
output through the remaining stream and the partial report flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4ca90cdc-1e1e-4930-91d3-15877104d477
📒 Files selected for processing (4)
core/src/autonomous/orchestrator/run.tscore/src/autonomous/report/writeReport.tsrunners/cli/src/commands/hunt.tsrunners/cli/src/ui/server.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- core/src/autonomous/orchestrator/run.ts
- core/src/autonomous/report/writeReport.ts
- runners/cli/src/ui/server.ts
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.
Problem
opfor huntscattered a single run's output across two places: the live log (hunt-live-<ts>.log) and structured event trail (run-<ts>.jsonl) were written directly into.opfor/reports/, while the final*-report.html/*-report.jsonlanded in their ownhunt-report-<ts>-<slug>-<id>/subfolder — created only at the very end, since its name depended on values not known until the run finished. A single hunt's artifacts were never together in one place.Separately,
opfor hunthad no cancellation support at all. Ctrl+C during a run just killed the process — no partial report, no findings preserved, nothing usable from the run so far.The SDK (
@keyvaluesystems/agent-opfor-sdk) is unaffected by the folder issue — it never wrote log/jsonl files to disk in the first place (callers get the equivalent viaonProgress), so this only touches the CLI's file-based output.Solution
One folder per run. The report folder's name (
hunt-report-<startedAt>-<slug>-<shortId>) turned out to be fully determined by values available at run start (target name, run id, start time), not completion — they just weren't threaded through in time. Exposed areportDirFor()helper in core and had the CLI create the folder immediately when the orchestrator hands back itsRunLog(via the existingonRunLoghook), openinghunt-live.logandrun-events.jsonldirectly inside it from the first line.writeAutonomousReport()now reuses the same helper so the final.html/.jsonland in the exact same folder — no rename/move step needed. Also fixed an ordering issue this surfaced along the way: run-log creation is deliberately kept after the seed-knowledge validation check, so a bad--seed-dirconfig still fails with zero artifacts on disk, not an empty report folder.Graceful cancellation. Added an
AbortSignaltorunAutonomous()'s hooks. First Ctrl+C aborts it, which interrupts the in-flight Claude Agent SDK query and lets the orchestrator finalize a partial, explicitly-truncated report from whatever findings were captured so far — with the live log/event trail already safely on disk. Second Ctrl+C force-quits with no report. Wired into both the plain CLI path and the--uisetup-mode path (newabortAssessment()on the UI server handle).Changes
Core (
@keyvaluesystems/agent-opfor-core)autonomous/report/writeReport.ts— extractedreportDirFor();writeAutonomousReport()now derives the folder fromreport.startedAtinstead ofreport.generatedAtautonomous/report/types.ts,autonomous/report/mapRunLog.ts—AutonomousReportnow carriesstartedAtautonomous/orchestrator/run.ts—RunHooks.signal?: AbortSignal; interrupts the SDK query on abort, marks the run truncated with a friendly reason, safety-net check at the endautonomous/index.ts— exportsreportDirForCLI (
@keyvaluesystems/agent-opfor-cli)commands/hunt.ts— creates the report folder + opens both log files fromonRunLog; wiresAbortControllerto SIGINT (first = graceful stop, second = force-quit); adjusts completion messaging for interrupted runsui/server.ts— same folder/log-file change for the--uiin-process path; newabortAssessment()onUiServerHandlecommands/setup.ts— minor UX hint (edit the generated config JSON directly)Docs
docs/hunt.md— documents the single-folder output layout and the new Ctrl+C behaviorIssue
N/A
Summary by CodeRabbit
New Features
Bug Fixes
Documentation