Continue nerds development and add features #3
Conversation
This commit introduces a new test runner executable and incorporates several new source files into the build system. These additions are essential for expanding the project's testing capabilities and integrating new functionalities. Co-authored-by: ayushpratap16 <ayushpratap16@gmail.com>
WalkthroughAdds an Enhanced CLI, WorkflowManager, and FlowPersistence modules with headers and implementations; introduces a new TestFramework and test runner executable; updates main to launch the Enhanced CLI; adds documentation and demo scripts; and extends CMake/Makefiles to build new components and the nerd_test_runner target. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as EnhancedCLI
participant FE as FlowEditor
participant WM as WorkflowManager
participant FP as FlowPersistence
participant NDH as NetworkDisruptionHandler
User->>CLI: start/run()
CLI->>CLI: initialize_components()
CLI->>WM: create_default_workflows()
CLI->>FP: setup_persistence()
CLI->>NDH: start_monitoring()
loop Interactive commands
User->>CLI: command (e.g., open/save/run workflow)
alt Flow ops
CLI->>FE: open/close/list/status/info
else Workflow ops
CLI->>WM: execute_workflow()/list/enable/disable
else Persistence ops
CLI->>FP: save/load/backup/checkpoint
else System/help/config
CLI->>CLI: show_help/config/status
end
end
par Network disruption
NDH-->>CLI: disruption detected
NDH->>FP: trigger_emergency_persistence()
and Exit
User->>CLI: exit
CLI->>NDH: stop_monitoring()
end
sequenceDiagram
autonumber
actor Dev as Developer
participant TR as TestRunner (main)
participant TF as TestFramework
participant Suites as Test Suites
Dev->>TR: ./nerd_test_runner [-v] [-o file] [-s suite]
TR->>TF: configure(verbose, output)
alt Specific suite
TR->>TF: run_test_suite(suite)
else All suites
TR->>TF: run_all_tests()
end
TF->>Suites: execute registered tests
TF-->>TR: results, stats
TR->>Dev: print results, exit code (0/1)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
|
Cursor Agent can help with this pull request. Just |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on October 22
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| flow->update_circulation_pattern(pattern); | ||
| std::cout << "Adjusted circulation rate from " << pattern.circulation_rate | ||
| << " to " << new_rate << " pps" << std::endl; | ||
| } |
There was a problem hiding this comment.
Bug: Rate Update Logging Shows Incorrect Initial Value
In adjust_circulation_rate, the log message for rate changes shows an incorrect "from" value. The circulation rate is updated before logging, making the "from" and "to" values appear identical (e.g., "from X to X pps").
| std::cerr << "Error: Failed to open flow '" << flow_name << "'" << std::endl; | ||
| return 1; | ||
| } | ||
| // The CLI will handle flow opening |
There was a problem hiding this comment.
Bug: Network Initialization and Flow Opening Logic Missing
The migration to EnhancedCLI inadvertently removed core network interface initialization and flow opening logic. The interface parameter is now ignored, and network setup is skipped. Flow opening, including its error handling for specified flow names, is also missing, replaced by an unrelated auto_save check and a comment.
|
|
||
| std::this_thread::sleep_for(std::chrono::seconds(5)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: Thread Hangs Due to Infinite Loop
The monitoring_worker() method contains an infinite loop, preventing its thread from terminating. This causes the NetworkDisruptionHandler's destructor to hang indefinitely when attempting to join the monitoring thread.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (17)
build/CMakeFiles/Makefile.cmake (1)
1-61: Remove generated build artifacts from source controlThis CMake file (and the rest of
build/CMakeFiles/**) is generated output that hard-codes the original build host paths (e.g.,/workspace/...). Keeping it in the repo will produce constant churn, leak local paths, and break other developers’ builds. Please drop the generatedbuild/tree from the commit and add it to.gitignore.build/CMakeFiles/nerd_test_runner.dir/src/network/flow_manager.cpp.o.d (1)
1-278: Remove generated build outputs from version control.
This.o.ddependency list is produced by the local CMake build; it changes per environment and is already regenerated by the build system. Keeping it under git will cause constant churn and makes the repo non-reproducible. Please drop the entirebuild/tree (including thesenerd_test_runnerartifacts) from the PR and ignore it going forward.build/CMakeFiles/nerd_test_runner.dir/src/testing/test_runner.cpp.o.d (1)
1-279: Remove generated build dependency file.This
.dfile is emitted by CMake during compilation and regenerates every build. Keeping it under version control will bloat the repo and cause perpetual churn. Please delete it and ensure the wholebuild/tree is ignored before merging.build/CMakeFiles/nerd_test_runner.dir/DependInfo.cmake (1)
1-32: Exclude CMake DependInfo metadata from the PR.
DependInfo.cmakeis another generated artifact. It changes whenever dependencies shift and shouldn’t live in the repository. Remove it (along with the rest ofbuild/) and add the directory to.gitignoreto keep the tree clean.build/CMakeFiles/Makefile2 (1)
65-146: Generated build outputs should not be versionedThis file lives under
build/and even begins with “CMAKE generated file: DO NOT EDIT!”. Keeping generated CMake artifacts in git will create noisy diffs every time someone configures the project, makes rebases painful, and risks stale build metadata sneaking into CI. Please drop this file (and the rest of thebuild/tree) from the PR and make surebuild/stays ignored going forward.build/CMakeFiles/nerd.dir/src/workflow/workflow_manager.cpp.o.d (1)
1-292: Do not commit dependency.dfilesThis dependency file is produced by the compiler each build and is specific to the local environment. Keeping it in git will cause nonstop conflicts and stale dependency data. Please delete it along with the rest of the generated
build/outputs.build/CMakeFiles/nerd.dir/progress.make (1)
1-10: Remove generated CMake progress files from source control.
build/CMakeFiles/nerd.dir/progress.makeis produced by CMake during the build and will churn on every local compile. Please delete it from the PR and keep thebuild/directory out of version control (gitignore it if necessary).build/CMakeFiles/nerd_test_runner.dir/src/core/flow_file.cpp.o.d (1)
1-277: Don’t commit compiler.ddependency artifacts.This
.o.dfile is another build output emitted by the compiler. It’s regenerated on every build and shouldn’t live in git because it will constantly trigger noisy diffs. Please remove it and exclude the generatedbuild/tree from tracking.build/CMakeFiles/nerd_test_runner.dir/src/workflow/workflow_manager.cpp.o.d (1)
1-290: Exclude generated dependency files from the repo.Same issue here—the
.o.ddependency list is generated by the compiler for each build. Keeping it versioned will cause perpetual merge conflicts. Drop the file and ensurebuild/remains untracked.build/CMakeFiles/nerd_test_runner.dir/link.d (1)
1-119: Remove linker manifest artifacts from source control.
build/CMakeFiles/nerd_test_runner.dir/link.dis created by CMake/Make during linking and should not be committed. Please delete it from this PR and keep generated build outputs out of git.build/CMakeFiles/nerd.dir/src/main.cpp.o.d (1)
1-286: Drop generated CMake dependency file from version control.
build/CMakeFiles/nerd.dir/src/main.cpp.o.dis emitted by CMake during compilation; committing it will create constant churn and stale dependency info. Please remove it from the PR and add thebuild/artifacts to.gitignore(or equivalent) so the directory stays out of the repo.build/CMakeFiles/nerd_test_runner.dir/build.make (1)
1-227: Exclude generated build.make artifact.This makefile lives under
build/and is CMake-generated. Keeping it tracked invites merge conflicts and breaks reproducibility assumptions. Drop it from source control and ignorebuild/outputs instead.build/CMakeFiles/nerd.dir/src/cli/enhanced_cli.cpp.o.d (1)
1-300: Remove CMake-generated.o.dfile.
build/CMakeFiles/nerd.dir/src/cli/enhanced_cli.cpp.o.dis another compilation byproduct. Please delete it from the PR and ensurebuild/remains untracked going forward.build/Makefile (1)
177-519: Do not commit the CMake top-level build/Makefile.The Makefile under
build/is regenerated on every configure and varies per host. It shouldn’t live in the repo; remove it and ignore thebuild/directory.build/CMakeFiles/nerd_test_runner.dir/compiler_depend.make (1)
1-1343: Purge compiler dependency metadata from VCS.
build/CMakeFiles/nerd_test_runner.dir/compiler_depend.makeis generated metadata, not source. Delete it from the PR and keepbuild/out of version control to avoid noise and accidental stale deps.build/CMakeFiles/nerd.dir/build.make (1)
1-243: Generated Makefile added to repo.This file is produced by CMake and must not be versioned. Keep only CMakeLists.txt and sources; exclude build/ tree.
Use the .gitignore snippet shared above and remove build/* from the PR.
build/CMakeFiles/nerd.dir/compiler_depend.internal (1)
1-2753: Remove tracked build artifacts and update.gitignore
73 files underbuild/are checked in; rungit rm -r --cached build/(and other build outputs) and add the following to.gitignore:build/ cmake-build-*/ *.o *.o.d *.obj *.pch *.log
🧹 Nitpick comments (5)
PROJECT_SUMMARY.md (1)
54-72: Add language annotation to the fenced diagram block.Markdown lint flagged this fence because no language is declared. Tagging it as
textsatisfies the check and keeps formatting predictable.-``` +```text ┌─────────────────┐ ┌─────────────────┐ ┌─────────────────┐ │ FlowEditor │ │ WorkflowManager │ │ FlowPersistence │ │ │ │ │ │ │ │ - CLI Interface │ │ - Cost Tracking │ │ - Checkpoints │ │ - Command Parse │ │ - Auto Workflows│ │ - Backup/Restore│ │ - User Experience│ │ - Optimization │ │ - Recovery │ └─────────────────┘ └─────────────────┘ └─────────────────┘ │ │ │ └───────────────────────┼───────────────────────┘ │ ┌─────────────────┐ │ NetworkFlow │ │ │ │ - Raw Sockets │ │ - Packet Mgmt │ │ - Circulation │ └─────────────────┘Based on static analysis hints
CMakeLists.txt (1)
31-51: Share common sources via a library instead of duplicating them.
Lines 31-51 recompile the entire runtime stack (core/network/workflow/persistence sources) for bothnerdandnerd_test_runner. That doubles build time and creates two places that must be kept in sync whenever a source file is added or removed. Factor the shared translation units into a library and link both executables against it so warnings, compile options, and deps stay unified.-# Source files -set(SOURCES - src/main.cpp - src/network/flow.cpp - src/network/packet.cpp - src/network/flow_manager.cpp - src/editor/flow_editor.cpp - src/core/flow_file.cpp - src/workflow/workflow_manager.cpp - src/persistence/flow_persistence.cpp - src/cli/enhanced_cli.cpp -) - -# Create executable -add_executable(nerd ${SOURCES}) - -# Create test runner executable -add_executable(nerd_test_runner - src/testing/test_runner.cpp - src/testing/test_framework.cpp - src/core/flow_file.cpp - src/network/flow.cpp - src/network/packet.cpp - src/network/flow_manager.cpp - src/workflow/workflow_manager.cpp - src/persistence/flow_persistence.cpp -) +set(NERD_CORE_SOURCES + src/network/flow.cpp + src/network/packet.cpp + src/network/flow_manager.cpp + src/editor/flow_editor.cpp + src/core/flow_file.cpp + src/workflow/workflow_manager.cpp + src/persistence/flow_persistence.cpp +) + +add_library(nerd_core STATIC ${NERD_CORE_SOURCES}) + +add_executable(nerd + src/main.cpp + src/cli/enhanced_cli.cpp +) +target_link_libraries(nerd PRIVATE nerd_core) + +add_executable(nerd_test_runner + src/testing/test_runner.cpp + src/testing/test_framework.cpp +) +target_link_libraries(nerd_test_runner PRIVATE nerd_core) # Link libraries target_link_libraries(nerd Threads::Threads ${CMAKE_DL_LIBS} ) - -target_link_libraries(nerd_test_runner - Threads::Threads - ${CMAKE_DL_LIBS} -) +target_link_libraries(nerd_test_runner + Threads::Threads + ${CMAKE_DL_LIBS} +)src/persistence/flow_persistence.cpp (3)
253-257: Trimmed checkpoints leak files on disk.When enforcing max_checkpoints_, the removed checkpoint isn’t deleted from disk; delete it to avoid unbounded storage growth.
- if (checkpoints_.size() > max_checkpoints_) { - checkpoints_.erase(checkpoints_.begin()); - } + if (checkpoints_.size() > max_checkpoints_) { + const auto old_id = checkpoints_.front().checkpoint_id; + checkpoints_.erase(checkpoints_.begin()); + const auto old_path = get_checkpoint_file_path(old_id); + try { + std::filesystem::remove(old_path); + } catch (const std::filesystem::filesystem_error& e) { + std::cerr << "Failed to remove old checkpoint " << old_id << ": " << e.what() << std::endl; + } + }Also applies to: 318-329
169-180: list_persisted_flows only reports in-memory snapshots.For LOCAL_DISK/HYBRID you’ll miss flows that exist on disk but not in memory. Consider scanning persistence_dir_/flows.
Example sketch (optional):
if (strategy_ == PersistenceStrategy::LOCAL_DISK || strategy_ == PersistenceStrategy::HYBRID) { for (auto& p : std::filesystem::directory_iterator(persistence_dir_ + "/flows")) { // parse flow_<id>.dat and push_back id if parseable } }
214-226: get_flow_history returns only the latest in-memory snapshot.Docs mention versioning; consider reading all on-disk versions (snapshots/flow__v*.dat), sorting by timestamp/version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
build/CMakeFiles/nerd.dir/src/cli/enhanced_cli.cpp.ois excluded by!**/*.obuild/CMakeFiles/nerd.dir/src/main.cpp.ois excluded by!**/*.obuild/CMakeFiles/nerd.dir/src/persistence/flow_persistence.cpp.ois excluded by!**/*.obuild/CMakeFiles/nerd.dir/src/workflow/workflow_manager.cpp.ois excluded by!**/*.obuild/CMakeFiles/nerd_test_runner.dir/src/core/flow_file.cpp.ois excluded by!**/*.obuild/CMakeFiles/nerd_test_runner.dir/src/network/flow.cpp.ois excluded by!**/*.obuild/CMakeFiles/nerd_test_runner.dir/src/network/flow_manager.cpp.ois excluded by!**/*.obuild/CMakeFiles/nerd_test_runner.dir/src/network/packet.cpp.ois excluded by!**/*.obuild/CMakeFiles/nerd_test_runner.dir/src/persistence/flow_persistence.cpp.ois excluded by!**/*.obuild/CMakeFiles/nerd_test_runner.dir/src/testing/test_framework.cpp.ois excluded by!**/*.obuild/CMakeFiles/nerd_test_runner.dir/src/testing/test_runner.cpp.ois excluded by!**/*.obuild/CMakeFiles/nerd_test_runner.dir/src/workflow/workflow_manager.cpp.ois excluded by!**/*.o
📒 Files selected for processing (51)
CMakeLists.txt(1 hunks)ENHANCED_FEATURES.md(1 hunks)PROJECT_SUMMARY.md(1 hunks)build/CMakeFiles/Makefile.cmake(1 hunks)build/CMakeFiles/Makefile2(4 hunks)build/CMakeFiles/TargetDirectories.txt(1 hunks)build/CMakeFiles/nerd.dir/DependInfo.cmake(1 hunks)build/CMakeFiles/nerd.dir/build.make(2 hunks)build/CMakeFiles/nerd.dir/cmake_clean.cmake(2 hunks)build/CMakeFiles/nerd.dir/compiler_depend.internal(50 hunks)build/CMakeFiles/nerd.dir/compiler_depend.make(69 hunks)build/CMakeFiles/nerd.dir/link.d(2 hunks)build/CMakeFiles/nerd.dir/link.txt(1 hunks)build/CMakeFiles/nerd.dir/progress.make(1 hunks)build/CMakeFiles/nerd.dir/src/cli/enhanced_cli.cpp.o.d(1 hunks)build/CMakeFiles/nerd.dir/src/main.cpp.o.d(3 hunks)build/CMakeFiles/nerd.dir/src/persistence/flow_persistence.cpp.o.d(1 hunks)build/CMakeFiles/nerd.dir/src/workflow/workflow_manager.cpp.o.d(1 hunks)build/CMakeFiles/nerd_test_runner.dir/DependInfo.cmake(1 hunks)build/CMakeFiles/nerd_test_runner.dir/build.make(1 hunks)build/CMakeFiles/nerd_test_runner.dir/cmake_clean.cmake(1 hunks)build/CMakeFiles/nerd_test_runner.dir/compiler_depend.internal(1 hunks)build/CMakeFiles/nerd_test_runner.dir/compiler_depend.make(1 hunks)build/CMakeFiles/nerd_test_runner.dir/compiler_depend.ts(1 hunks)build/CMakeFiles/nerd_test_runner.dir/depend.make(1 hunks)build/CMakeFiles/nerd_test_runner.dir/flags.make(1 hunks)build/CMakeFiles/nerd_test_runner.dir/link.d(1 hunks)build/CMakeFiles/nerd_test_runner.dir/link.txt(1 hunks)build/CMakeFiles/nerd_test_runner.dir/progress.make(1 hunks)build/CMakeFiles/nerd_test_runner.dir/src/core/flow_file.cpp.o.d(1 hunks)build/CMakeFiles/nerd_test_runner.dir/src/network/flow.cpp.o.d(1 hunks)build/CMakeFiles/nerd_test_runner.dir/src/network/flow_manager.cpp.o.d(1 hunks)build/CMakeFiles/nerd_test_runner.dir/src/network/packet.cpp.o.d(1 hunks)build/CMakeFiles/nerd_test_runner.dir/src/persistence/flow_persistence.cpp.o.d(1 hunks)build/CMakeFiles/nerd_test_runner.dir/src/testing/test_framework.cpp.o.d(1 hunks)build/CMakeFiles/nerd_test_runner.dir/src/testing/test_runner.cpp.o.d(1 hunks)build/CMakeFiles/nerd_test_runner.dir/src/workflow/workflow_manager.cpp.o.d(1 hunks)build/CMakeFiles/progress.marks(1 hunks)build/Makefile(14 hunks)demo_script.txt(1 hunks)include/cli/enhanced_cli.h(1 hunks)include/persistence/flow_persistence.h(1 hunks)include/testing/test_framework.h(1 hunks)include/workflow/workflow_manager.h(1 hunks)quick_demo.sh(1 hunks)src/cli/enhanced_cli.cpp(1 hunks)src/main.cpp(2 hunks)src/persistence/flow_persistence.cpp(1 hunks)src/testing/test_framework.cpp(1 hunks)src/testing/test_runner.cpp(1 hunks)src/workflow/workflow_manager.cpp(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/persistence/flow_persistence.cpp (2)
include/testing/test_framework.h (2)
save_flow(178-181)load_flow(183-187)include/persistence/flow_persistence.h (1)
get_persistence_size(96-157)
include/workflow/workflow_manager.h (3)
include/cli/enhanced_cli.h (1)
nerd(14-195)include/persistence/flow_persistence.h (1)
nerd(13-159)src/workflow/workflow_manager.cpp (18)
WorkflowManager(9-13)WorkflowManager(15-17)list_workflows(49-59)list_workflows(49-49)create_default_workflows(290-294)create_default_workflows(290-290)workflow_worker(296-301)workflow_worker(296-296)evaluate_trigger(303-332)evaluate_trigger(303-303)execute_action(334-407)execute_action(334-334)optimize_circulation_pattern(409-430)optimize_circulation_pattern(409-409)compress_flow_data(432-444)compress_flow_data(432-432)migrate_flow_to_optimal_node(446-454)migrate_flow_to_optimal_node(446-446)
include/cli/enhanced_cli.h (4)
include/persistence/flow_persistence.h (1)
nerd(13-159)src/cli/enhanced_cli.cpp (48)
EnhancedCLI(21-26)EnhancedCLI(28-28)run(30-38)run(30-30)run_interactive(40-65)run_interactive(40-40)run_script(67-86)run_script(67-67)process_command(88-99)process_command(88-88)register_command_help(170-172)register_command_help(170-170)print_success(174-180)print_success(174-174)print_error(182-189)print_error(182-182)print_info(199-205)print_info(199-199)parse_arguments(621-631)parse_arguments(621-621)get_command_name(633-635)get_command_name(633-633)get_command_args(637-643)get_command_args(637-637)route_command(645-768)route_command(645-645)read_line(775-779)read_line(775-775)add_to_history(799-806)add_to_history(799-799)print_prompt(819-822)print_prompt(819-819)print_banner(824-842)print_banner(824-824)print_version(844-846)print_version(844-844)handle_exception(860-862)handle_exception(860-860)initialize_components(864-869)initialize_components(864-864)register_builtin_commands(871-903)register_builtin_commands(871-871)setup_workflows(905-908)setup_workflows(905-905)setup_persistence(910-913)setup_persistence(910-910)log_command(935-945)log_command(935-935)src/workflow/workflow_manager.cpp (2)
WorkflowManager(9-13)WorkflowManager(15-17)src/persistence/flow_persistence.cpp (4)
FlowPersistence(11-16)FlowPersistence(18-18)NetworkDisruptionHandler(687-690)NetworkDisruptionHandler(692-694)
include/persistence/flow_persistence.h (3)
include/cli/enhanced_cli.h (1)
nerd(14-195)include/testing/test_framework.h (3)
nerd(14-190)save_flow(178-181)load_flow(183-187)src/persistence/flow_persistence.cpp (52)
FlowPersistence(11-16)FlowPersistence(18-18)save_flow(20-79)save_flow(20-20)load_flow(81-145)load_flow(81-81)create_snapshot(182-198)create_snapshot(182-182)create_checkpoint(228-264)create_checkpoint(228-228)get_flow_file_path(427-429)get_flow_file_path(427-427)get_snapshot_file_path(431-434)get_snapshot_file_path(431-431)get_checkpoint_file_path(436-438)get_checkpoint_file_path(436-436)serialize_flow(440-467)serialize_flow(440-440)deserialize_flow(469-502)deserialize_flow(469-469)serialize_snapshot(504-534)serialize_snapshot(504-504)deserialize_snapshot(536-569)deserialize_snapshot(536-536)serialize_checkpoint(571-596)serialize_checkpoint(571-571)deserialize_checkpoint(598-629)deserialize_checkpoint(598-598)ensure_persistence_directory(631-636)ensure_persistence_directory(631-631)create_directory_if_not_exists(638-648)create_directory_if_not_exists(638-638)create_snapshot_from_flow(673-684)create_snapshot_from_flow(673-673)NetworkDisruptionHandler(687-690)NetworkDisruptionHandler(692-694)start_monitoring(696-699)start_monitoring(696-696)stop_monitoring(701-706)stop_monitoring(701-701)handle_disruption(708-724)handle_disruption(708-708)update_heartbeat(736-739)update_heartbeat(736-736)get_time_since_last_heartbeat(741-745)get_time_since_last_heartbeat(741-741)monitoring_worker(747-756)monitoring_worker(747-747)detect_network_disruption(758-763)detect_network_disruption(758-758)trigger_emergency_persistence(765-771)trigger_emergency_persistence(765-765)
src/cli/enhanced_cli.cpp (2)
include/cli/enhanced_cli.h (1)
EnhancedCLI(53-193)src/main.cpp (4)
print_version(36-39)print_version(36-36)print_usage(6-34)print_usage(6-6)
src/testing/test_framework.cpp (2)
include/testing/test_framework.h (1)
milliseconds(25-96)src/workflow/workflow_manager.cpp (2)
WorkflowManager(9-13)WorkflowManager(15-17)
include/testing/test_framework.h (4)
include/persistence/flow_persistence.h (1)
nerd(13-159)src/testing/test_framework.cpp (53)
TestFramework(11-20)TestFramework(22-22)create_flow_tests(137-217)create_flow_tests(137-137)create_network_tests(219-317)create_network_tests(219-219)create_workflow_tests(319-398)create_workflow_tests(319-319)create_persistence_tests(400-476)create_persistence_tests(400-400)create_integration_tests(478-529)create_integration_tests(478-478)create_performance_tests(531-599)create_performance_tests(531-531)execute_test(601-619)execute_test(601-601)update_statistics(621-638)update_statistics(621-621)result(105-105)result(604-604)result(612-612)create_test_flow(719-723)create_test_flow(719-719)create_test_network_flow(725-727)create_test_network_flow(725-725)create_test_workflow_manager(729-733)create_test_workflow_manager(729-729)create_test_persistence(735-737)create_test_persistence(735-735)generate_test_content(739-752)generate_test_content(739-739)generate_test_packet_data(754-767)generate_test_packet_data(754-754)create_test_circulation_pattern(769-777)create_test_circulation_pattern(769-769)simulate_network_operation(779-783)simulate_network_operation(779-779)simulate_network_disruption(785-789)simulate_network_disruption(785-785)simulate_high_latency(791-795)simulate_high_latency(791-791)measure_execution_time(797-802)measure_execution_time(797-797)measure_memory_usage(804-807)measure_memory_usage(804-804)create_test_directory(809-812)create_test_directory(809-809)cleanup_test_directory(814-817)cleanup_test_directory(814-814)file_exists(819-822)file_exists(819-819)get_file_size(824-827)get_file_size(824-824)src/workflow/workflow_manager.cpp (2)
WorkflowManager(9-13)WorkflowManager(15-17)src/persistence/flow_persistence.cpp (6)
FlowPersistence(11-16)FlowPersistence(18-18)save_flow(20-79)save_flow(20-20)load_flow(81-145)load_flow(81-81)
🪛 markdownlint-cli2 (0.18.1)
ENHANCED_FEATURES.md
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
214-214: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
PROJECT_SUMMARY.md
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
214-214: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Biome (2.1.2)
build/CMakeFiles/nerd_test_runner.dir/compiler_depend.ts
[error] 1-1: expected ! following a #, but found none
(parse)
[error] 2-2: expected ( but instead found compiler
Remove compiler
(parse)
[error] 2-2: expected ; but instead found generated
Remove generated
(parse)
[error] 2-2: expected ; but instead found dependencies
Remove dependencies
(parse)
[error] 2-2: expected ) but instead found management
Remove management
(parse)
[error] 2-2: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 2-2: expected ( but instead found nerd_test_runner
Remove nerd_test_runner
(parse)
[error] 2-2: Expected an identifier but instead found the end of the file.
Expected an identifier here.
(parse)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (9)
build/CMakeFiles/nerd_test_runner.dir/src/testing/test_framework.cpp.o.d (1)
1-327: Covered by earlier note about generated artifactsPlease refer to my comment on build/CMakeFiles/nerd_test_runner.dir/flags.make (Lines 1-9); the same concern applies here.
build/CMakeFiles/nerd_test_runner.dir/src/network/packet.cpp.o.d (1)
1-196: Covered by earlier note about generated artifactsPlease refer to my comment on build/CMakeFiles/nerd_test_runner.dir/flags.make (Lines 1-9); the same concern applies here.
build/CMakeFiles/nerd_test_runner.dir/src/persistence/flow_persistence.cpp.o.d (1)
1-299: Covered by earlier note about generated artifactsPlease refer to my comment on build/CMakeFiles/nerd_test_runner.dir/flags.make (Lines 1-9); the same concern applies here.
build/CMakeFiles/nerd_test_runner.dir/compiler_depend.internal (1)
1-651: Generated build metadata should stay out of git.
Same issue as noted forbuild/CMakeFiles/nerd_test_runner.dir/src/network/flow_manager.cpp.o.d: this file is CMake-generated and should be removed along with the rest of thebuild/artifacts.build/CMakeFiles/nerd_test_runner.dir/cmake_clean.cmake (1)
1-27: Generated clean script should not be committed.
Again, this lives under the generatedbuild/tree—please drop it together with the other CMake outputs referenced above.build/CMakeFiles/nerd.dir/link.txt (1)
1-1: Same generated build artifact needs to be dropped.
Line 1 is another auto-generated file underbuild/. This is the same issue I called out inbuild/CMakeFiles/nerd.dir/DependInfo.cmake; please remove the committed build outputs instead of versioning them.build/CMakeFiles/nerd.dir/link.d (1)
11-13: Generated dependency file should be removed from git.
Line 11 showslink.d, which, like the otherbuild/artifacts, is generated by CMake. Refer back to the first comment—these files should not be committed.build/CMakeFiles/nerd.dir/src/persistence/flow_persistence.cpp.o.d (1)
1-302: Generated.ddependency file belongs in the build tree, not the repo.
Line 1 introduces another CMake-generated file. Please remove this along with the rest of thebuild/directory from the PR.src/persistence/flow_persistence.cpp (1)
458-461: Raw struct serialization may be non-portable.Writing CirculationPattern with a raw write assumes trivial layout and identical endianness. If it isn’t a simple enum/trivial POD, switch to a stable field-wise serialization.
Can you confirm CirculationPattern is trivially copyable? If not, I can draft structured serializers.
Also applies to: 519-520, 553-554
| # CMAKE generated file: DO NOT EDIT! | ||
| # Generated by "Unix Makefiles" Generator, CMake Version 3.31 | ||
|
|
||
| # compile CXX with /usr/bin/c++ | ||
| CXX_DEFINES = | ||
|
|
||
| CXX_INCLUDES = -I/workspace/src -I/workspace/include | ||
|
|
||
| CXX_FLAGS = -std=gnu++17 |
There was a problem hiding this comment.
Stop tracking generated build outputs
This file is produced by CMake in the build tree and hardcodes environment-specific paths (e.g., /workspace). Keeping it in version control will cause constant churn on every local build and makes the repository platform-dependent. Please drop the generated build/ artifacts from the PR and add the directory to .gitignore (or regenerate them in CI instead).
🤖 Prompt for AI Agents
In build/CMakeFiles/nerd_test_runner.dir/flags.make around lines 1 to 9 the file
is a CMake-generated build output containing environment-specific paths and must
not be tracked; remove this generated file from the PR (git rm --cached
build/CMakeFiles/nerd_test_runner.dir/flags.make or delete it then commit), add
the appropriate build/ or build/CMakeFiles/ pattern to .gitignore so generated
artifacts are not committed in future, and ensure CI regenerates any required
build outputs rather than checking them into VCS.
| @@ -0,0 +1 @@ | |||
| /usr/bin/c++ -Xlinker --dependency-file=CMakeFiles/nerd_test_runner.dir/link.d CMakeFiles/nerd_test_runner.dir/src/testing/test_runner.cpp.o CMakeFiles/nerd_test_runner.dir/src/testing/test_framework.cpp.o CMakeFiles/nerd_test_runner.dir/src/core/flow_file.cpp.o CMakeFiles/nerd_test_runner.dir/src/network/flow.cpp.o CMakeFiles/nerd_test_runner.dir/src/network/packet.cpp.o CMakeFiles/nerd_test_runner.dir/src/network/flow_manager.cpp.o CMakeFiles/nerd_test_runner.dir/src/workflow/workflow_manager.cpp.o CMakeFiles/nerd_test_runner.dir/src/persistence/flow_persistence.cpp.o -o nerd_test_runner -ldl | |||
There was a problem hiding this comment.
Drop committed linker command files
link.txt is emitted by CMake during configuration and should never be checked in. Different toolchains, paths, or even incremental builds will rewrite it, so having it tracked guarantees spurious diffs and brittle builds. Please remove this file (and other generated siblings) from the PR.
🤖 Prompt for AI Agents
In build/CMakeFiles/nerd_test_runner.dir/link.txt (line 1), this is a generated
CMake linker command file that must not be committed; remove the file from the
repo, stop tracking generated CMake files, and add appropriate ignore rules.
Delete the committed link.txt (git rm path), add a .gitignore entry for CMake
generated artifacts (e.g. build/**/link.txt or build/**/CMakeFiles/*) or the
build/ directory as appropriate, commit the removal, and if the file was
previously tracked ensure it's removed from history in this branch (or use git
rm --cached) so future CMake runs don't produce spurious diffs.
| CMAKE_PROGRESS_1 = 11 | ||
| CMAKE_PROGRESS_2 = 12 | ||
| CMAKE_PROGRESS_3 = 13 | ||
| CMAKE_PROGRESS_4 = 14 | ||
| CMAKE_PROGRESS_5 = 15 | ||
| CMAKE_PROGRESS_6 = 16 | ||
| CMAKE_PROGRESS_7 = 17 | ||
| CMAKE_PROGRESS_8 = 18 | ||
| CMAKE_PROGRESS_9 = 19 |
There was a problem hiding this comment.
Drop generated progress markers from version control.
progress.make is also auto-generated by CMake and will churn on every build. Please remove it (and ignore build/) so future diffs stay signal-only.
🤖 Prompt for AI Agents
In build/CMakeFiles/nerd_test_runner.dir/progress.make lines 1-9: this file is
an auto-generated CMake progress marker that should not be tracked; remove it
from the repository and stop future churn by adding the build/ directory (or the
specific path build/CMakeFiles/) to .gitignore, run git rm --cached
build/CMakeFiles/nerd_test_runner.dir/progress.make (or git rm -r --cached
build/), commit the removal and the updated .gitignore, and push the changes so
generated progress.make is no longer tracked.
| "/workspace/src/cli/enhanced_cli.cpp" "CMakeFiles/nerd.dir/src/cli/enhanced_cli.cpp.o" "gcc" "CMakeFiles/nerd.dir/src/cli/enhanced_cli.cpp.o.d" | ||
| "/workspace/src/core/flow_file.cpp" "CMakeFiles/nerd.dir/src/core/flow_file.cpp.o" "gcc" "CMakeFiles/nerd.dir/src/core/flow_file.cpp.o.d" | ||
| "/workspace/src/editor/flow_editor.cpp" "CMakeFiles/nerd.dir/src/editor/flow_editor.cpp.o" "gcc" "CMakeFiles/nerd.dir/src/editor/flow_editor.cpp.o.d" | ||
| "/workspace/src/main.cpp" "CMakeFiles/nerd.dir/src/main.cpp.o" "gcc" "CMakeFiles/nerd.dir/src/main.cpp.o.d" | ||
| "/workspace/src/network/flow.cpp" "CMakeFiles/nerd.dir/src/network/flow.cpp.o" "gcc" "CMakeFiles/nerd.dir/src/network/flow.cpp.o.d" | ||
| "/workspace/src/network/flow_manager.cpp" "CMakeFiles/nerd.dir/src/network/flow_manager.cpp.o" "gcc" "CMakeFiles/nerd.dir/src/network/flow_manager.cpp.o.d" | ||
| "/workspace/src/network/packet.cpp" "CMakeFiles/nerd.dir/src/network/packet.cpp.o" "gcc" "CMakeFiles/nerd.dir/src/network/packet.cpp.o.d" | ||
| "/workspace/src/persistence/flow_persistence.cpp" "CMakeFiles/nerd.dir/src/persistence/flow_persistence.cpp.o" "gcc" "CMakeFiles/nerd.dir/src/persistence/flow_persistence.cpp.o.d" | ||
| "/workspace/src/workflow/workflow_manager.cpp" "CMakeFiles/nerd.dir/src/workflow/workflow_manager.cpp.o" "gcc" "CMakeFiles/nerd.dir/src/workflow/workflow_manager.cpp.o.d" |
There was a problem hiding this comment.
Remove generated build metadata from version control.
Line 11 shows build/CMakeFiles/nerd.dir/DependInfo.cmake, which is produced by CMake during the build. Checking the whole build/ tree into git will churn on every local build and cause noisy merge conflicts. Please drop these artifacts from the PR and add build/ (and similar generator outputs) to .gitignore before merging.
🤖 Prompt for AI Agents
In build/CMakeFiles/nerd.dir/DependInfo.cmake around lines 11 to 19, this file
is a generated CMake build artifact and should not be checked into source
control; remove the generated build artifacts from the PR, add build/ (and other
CMake generator outputs) to .gitignore, and ensure the repo stops tracking
existing files in that directory (remove them from the index and commit the
change) so future local builds do not create noisy diffs or merge conflicts.
| @@ -1 +1 @@ | |||
| 7 | |||
| 19 | |||
There was a problem hiding this comment.
Remove tracked progress.marks artifact
build/CMakeFiles/progress.marks is another CMake-generated byproduct. Letting it live in the repo will churn constantly and doesn’t belong under source control. Please delete it together with the rest of the generated build/ directory.
🤖 Prompt for AI Agents
In build/CMakeFiles/progress.marks around line 1: this file is a CMake-generated
build artifact that should not be tracked; remove progress.marks (and preferably
the entire generated build/ directory) from the repository, ensure build/ is
added to .gitignore so it won’t be re-added, and commit the removal; if the file
is already tracked, untrack it via a git remove-from-index step (so it’s deleted
from the repo but not reintroduced) before committing the .gitignore update.
| void NetworkDisruptionHandler::start_monitoring() { | ||
| monitoring_thread_ = std::thread(&NetworkDisruptionHandler::monitoring_worker, this); | ||
| std::cout << "Started network disruption monitoring" << std::endl; | ||
| } | ||
|
|
||
| void NetworkDisruptionHandler::stop_monitoring() { | ||
| if (monitoring_thread_.joinable()) { | ||
| monitoring_thread_.join(); | ||
| } | ||
| std::cout << "Stopped network disruption monitoring" << std::endl; | ||
| } |
There was a problem hiding this comment.
Monitoring thread never exits; destructor can hang (infinite loop).
start_monitoring launches a thread, stop_monitoring only joins without a stop signal, and monitoring_worker loops while (true). This can deadlock on destruction. Add a running flag and exit condition.
Apply this diff here, and add the header change shown below:
@@
-void NetworkDisruptionHandler::start_monitoring() {
- monitoring_thread_ = std::thread(&NetworkDisruptionHandler::monitoring_worker, this);
+void NetworkDisruptionHandler::start_monitoring() {
+ if (running_.exchange(true)) {
+ return; // already running
+ }
+ monitoring_thread_ = std::thread(&NetworkDisruptionHandler::monitoring_worker, this);
std::cout << "Started network disruption monitoring" << std::endl;
}
-void NetworkDisruptionHandler::stop_monitoring() {
- if (monitoring_thread_.joinable()) {
- monitoring_thread_.join();
- }
+void NetworkDisruptionHandler::stop_monitoring() {
+ if (!running_.exchange(false)) {
+ return; // not running
+ }
+ if (monitoring_thread_.joinable()) {
+ monitoring_thread_.join();
+ }
std::cout << "Stopped network disruption monitoring" << std::endl;
}
@@
-void NetworkDisruptionHandler::monitoring_worker() {
- while (true) {
+void NetworkDisruptionHandler::monitoring_worker() {
+ while (running_) {
if (detect_network_disruption()) {
std::lock_guard<std::mutex> lock(handler_mutex_);
disruption_detected_ = true;
}
std::this_thread::sleep_for(std::chrono::seconds(5));
}
}Also update the header (include/persistence/flow_persistence.h):
class NetworkDisruptionHandler {
private:
FlowPersistence* persistence_;
std::atomic<bool> disruption_detected_;
+ std::atomic<bool> running_{false};
std::chrono::time_point<std::chrono::system_clock> last_heartbeat_;
std::thread monitoring_thread_;
mutable std::mutex handler_mutex_;Also applies to: 747-756
🤖 Prompt for AI Agents
In src/persistence/flow_persistence.cpp around lines 696-706 (and likewise for
the similar block at 747-756), the monitoring thread is started but never given
a stop signal and the worker loops forever; modify start_monitoring to set a
running flag to true before launching the thread, change monitoring_worker to
loop while that running flag is true (instead of while(true)), and make
stop_monitoring set the running flag to false and then join the thread if
joinable so the worker can exit cleanly; also update the header
include/persistence/flow_persistence.h to add an std::atomic<bool> running_
member (and include <atomic> in the header or cpp as needed) and ensure
monitoring_thread_ is declared there so stop/start manage the same thread
object.
| for (int i = 1; i < argc; ++i) { | ||
| std::string arg = argv[i]; | ||
|
|
||
| if (arg == "-v" || arg == "--verbose") { | ||
| verbose = true; | ||
| } else if (arg == "-o" || arg == "--output") { | ||
| if (i + 1 < argc) { | ||
| output_file = argv[++i]; | ||
| } else { | ||
| std::cerr << "Error: Missing output filename" << std::endl; | ||
| return 1; | ||
| } | ||
| } else if (arg == "-s" || arg == "--suite") { | ||
| if (i + 1 < argc) { | ||
| suite_name = argv[++i]; | ||
| } else { | ||
| std::cerr << "Error: Missing suite name" << std::endl; | ||
| return 1; | ||
| } | ||
| } else if (arg == "-h" || arg == "--help") { | ||
| std::cout << "Usage: " << argv[0] << " [OPTIONS]" << std::endl; | ||
| std::cout << "Options:" << std::endl; | ||
| std::cout << " -v, --verbose Enable verbose output" << std::endl; | ||
| std::cout << " -o, --output FILE Save results to file" << std::endl; | ||
| std::cout << " -s, --suite NAME Run specific test suite" << std::endl; | ||
| std::cout << " -h, --help Show this help message" << std::endl; | ||
| return 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
Don't silently ignore unknown CLI flags.
Right now, an argument like --foo falls through this chain and is silently discarded, which makes diagnosing typos or misconfigurations difficult for the test runner. Please reject unexpected - options so users get immediate feedback instead of running with unintended defaults.
Apply this diff to fail fast on unknown options:
@@
- } else if (arg == "-h" || arg == "--help") {
+ } else if (arg == "-h" || arg == "--help") {
std::cout << "Usage: " << argv[0] << " [OPTIONS]" << std::endl;
std::cout << "Options:" << std::endl;
std::cout << " -v, --verbose Enable verbose output" << std::endl;
std::cout << " -o, --output FILE Save results to file" << std::endl;
std::cout << " -s, --suite NAME Run specific test suite" << std::endl;
std::cout << " -h, --help Show this help message" << std::endl;
return 0;
+ } else if (!arg.empty() && arg.front() == '-') {
+ std::cerr << "Error: Unknown option " << arg << std::endl;
+ return 1;
}📝 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.
| for (int i = 1; i < argc; ++i) { | |
| std::string arg = argv[i]; | |
| if (arg == "-v" || arg == "--verbose") { | |
| verbose = true; | |
| } else if (arg == "-o" || arg == "--output") { | |
| if (i + 1 < argc) { | |
| output_file = argv[++i]; | |
| } else { | |
| std::cerr << "Error: Missing output filename" << std::endl; | |
| return 1; | |
| } | |
| } else if (arg == "-s" || arg == "--suite") { | |
| if (i + 1 < argc) { | |
| suite_name = argv[++i]; | |
| } else { | |
| std::cerr << "Error: Missing suite name" << std::endl; | |
| return 1; | |
| } | |
| } else if (arg == "-h" || arg == "--help") { | |
| std::cout << "Usage: " << argv[0] << " [OPTIONS]" << std::endl; | |
| std::cout << "Options:" << std::endl; | |
| std::cout << " -v, --verbose Enable verbose output" << std::endl; | |
| std::cout << " -o, --output FILE Save results to file" << std::endl; | |
| std::cout << " -s, --suite NAME Run specific test suite" << std::endl; | |
| std::cout << " -h, --help Show this help message" << std::endl; | |
| return 0; | |
| } | |
| } | |
| for (int i = 1; i < argc; ++i) { | |
| std::string arg = argv[i]; | |
| if (arg == "-v" || arg == "--verbose") { | |
| verbose = true; | |
| } else if (arg == "-o" || arg == "--output") { | |
| if (i + 1 < argc) { | |
| output_file = argv[++i]; | |
| } else { | |
| std::cerr << "Error: Missing output filename" << std::endl; | |
| return 1; | |
| } | |
| } else if (arg == "-s" || arg == "--suite") { | |
| if (i + 1 < argc) { | |
| suite_name = argv[++i]; | |
| } else { | |
| std::cerr << "Error: Missing suite name" << std::endl; | |
| return 1; | |
| } | |
| } else if (arg == "-h" || arg == "--help") { | |
| std::cout << "Usage: " << argv[0] << " [OPTIONS]" << std::endl; | |
| std::cout << "Options:" << std::endl; | |
| std::cout << " -v, --verbose Enable verbose output" << std::endl; | |
| std::cout << " -o, --output FILE Save results to file" << std::endl; | |
| std::cout << " -s, --suite NAME Run specific test suite" << std::endl; | |
| std::cout << " -h, --help Show this help message" << std::endl; | |
| return 0; | |
| } else if (!arg.empty() && arg.front() == '-') { | |
| std::cerr << "Error: Unknown option " << arg << std::endl; | |
| return 1; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/testing/test_runner.cpp around lines 16 to 44, the CLI parsing silently
ignores unrecognized flags (e.g. --foo); modify the loop to detect any argument
that starts with '-' and isn't one of the known options and treat it as an
error: print an error to std::cerr including the offending argument and return a
non-zero exit code (e.g. return 1) so the program fails fast on unknown options;
keep existing handling for positional/non-dash arguments unchanged.
| void WorkflowManager::execute_workflow(const std::string& name, WorkflowContext& context) { | ||
| Workflow* workflow = get_workflow(name); | ||
| if (!workflow || !workflow->enabled) { | ||
| return; | ||
| } | ||
|
|
||
| // Check if trigger conditions are met | ||
| if (!evaluate_trigger(workflow->trigger, context)) { | ||
| return; | ||
| } | ||
|
|
||
| std::cout << "Executing workflow: " << name << std::endl; | ||
|
|
||
| // Execute all actions in priority order | ||
| std::vector<WorkflowAction> actions = workflow->actions; | ||
| std::sort(actions.begin(), actions.end(), | ||
| [](const WorkflowAction& a, const WorkflowAction& b) { | ||
| return a.priority > b.priority; | ||
| }); | ||
|
|
||
| for (const auto& action : actions) { | ||
| if (action.enabled) { | ||
| execute_action(action, context); | ||
| } | ||
| } | ||
|
|
||
| // Update workflow statistics | ||
| workflow->last_run = std::chrono::system_clock::now(); | ||
| workflow->run_count++; | ||
| } |
There was a problem hiding this comment.
Avoid returning raw map pointers without holding the lock.
get_workflow() (Line 44) returns a pointer to an element in workflows_ after releasing workflows_mutex_. execute_workflow() then dereferences and mutates that object (Lines 63–90) while no lock is held. Another thread can call remove_workflow() or enable_workflow() in parallel, erase or mutate the same entry, and leave this pointer dangling or cause torn reads. Please either (a) keep the mutex locked while using the workflow object or (b) copy out the workflow state under lock, operate on the copy, and reacquire the lock only when writing back stats.
| void WorkflowManager::execute_all_workflows() { | ||
| std::lock_guard<std::mutex> lock(workflows_mutex_); | ||
|
|
||
| for (auto& pair : workflows_) { | ||
| if (pair.second.enabled) { | ||
| WorkflowContext context; | ||
| execute_workflow(pair.first, context); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix the self-deadlock in execute_all_workflows.
Line 93 grabs workflows_mutex_, and while still holding it Line 98 calls execute_workflow, which immediately calls get_workflow (Line 62) and tries to lock the same mutex again. Because std::mutex is non-recursive, this call path deadlocks on the first workflow execution, wedging the workflow engine. Please release the lock before invoking execute_workflow (e.g., collect the enabled workflow names inside the critical section, then iterate outside it).
void WorkflowManager::execute_all_workflows() {
- std::lock_guard<std::mutex> lock(workflows_mutex_);
-
- for (auto& pair : workflows_) {
- if (pair.second.enabled) {
- WorkflowContext context;
- execute_workflow(pair.first, context);
- }
- }
+ std::vector<std::string> enabled;
+ {
+ std::lock_guard<std::mutex> lock(workflows_mutex_);
+ enabled.reserve(workflows_.size());
+ for (const auto& pair : workflows_) {
+ if (pair.second.enabled) {
+ enabled.push_back(pair.first);
+ }
+ }
+ }
+
+ for (const auto& name : enabled) {
+ WorkflowContext context;
+ execute_workflow(name, context);
+ }
}📝 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.
| void WorkflowManager::execute_all_workflows() { | |
| std::lock_guard<std::mutex> lock(workflows_mutex_); | |
| for (auto& pair : workflows_) { | |
| if (pair.second.enabled) { | |
| WorkflowContext context; | |
| execute_workflow(pair.first, context); | |
| } | |
| } | |
| } | |
| void WorkflowManager::execute_all_workflows() { | |
| // Collect the names of enabled workflows under lock | |
| std::vector<std::string> enabled; | |
| { | |
| std::lock_guard<std::mutex> lock(workflows_mutex_); | |
| enabled.reserve(workflows_.size()); | |
| for (const auto& pair : workflows_) { | |
| if (pair.second.enabled) { | |
| enabled.push_back(pair.first); | |
| } | |
| } | |
| } | |
| // Now that the mutex is released, execute each workflow | |
| for (const auto& name : enabled) { | |
| WorkflowContext context; | |
| execute_workflow(name, context); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/workflow/workflow_manager.cpp around lines 92 to 101, holding
workflows_mutex_ while calling execute_workflow causes a self-deadlock because
execute_workflow calls get_workflow which tries to lock the same mutex; fix by
acquiring the mutex only to collect the enabled workflow names/IDs into a local
vector (or similar), release the lock, and then iterate that vector calling
execute_workflow for each entry outside the critical section so no double-lock
occurs.
| double WorkflowManager::get_total_cost() const { | ||
| return total_bandwidth_cost_ + total_cpu_cost_ + total_memory_cost_; | ||
| } |
There was a problem hiding this comment.
Guard get_total_cost() with the mutex.
total_bandwidth_cost_, total_cpu_cost_, and total_memory_cost_ are mutated under costs_mutex_ (e.g., Line 131), but get_total_cost() reads them without acquiring the same lock. Concurrent readers/writers hit undefined behavior. Wrap the read with the mutex to keep the cost totals race-free.
double WorkflowManager::get_total_cost() const {
- return total_bandwidth_cost_ + total_cpu_cost_ + total_memory_cost_;
+ std::lock_guard<std::mutex> lock(costs_mutex_);
+ return total_bandwidth_cost_ + total_cpu_cost_ + total_memory_cost_;
}📝 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.
| double WorkflowManager::get_total_cost() const { | |
| return total_bandwidth_cost_ + total_cpu_cost_ + total_memory_cost_; | |
| } | |
| double WorkflowManager::get_total_cost() const { | |
| std::lock_guard<std::mutex> lock(costs_mutex_); | |
| return total_bandwidth_cost_ + total_cpu_cost_ + total_memory_cost_; | |
| } |
🤖 Prompt for AI Agents
In src/workflow/workflow_manager.cpp around lines 143-145, get_total_cost()
reads total_bandwidth_cost_, total_cpu_cost_, and total_memory_cost_ without
holding costs_mutex_, causing a data race; update the method to acquire the same
mutex when reading (e.g., use std::lock_guard<std::mutex> guard(costs_mutex_))
and then compute and return the sum while the lock is held; if costs_mutex_ is
not declared mutable, mark it mutable so the const method can lock it; ensure
appropriate header (<mutex>) is available.
Implement core roadmap features for NERD, including workflow, cost optimization, persistence, enhanced CLI, and a testing framework.
This PR addresses the user's request to continue NERD development by implementing key roadmap items. The new workflow system provides automated management, while cost tracking and optimization directly tackle the need for cost-effective operations. Flow persistence ensures resilience against network disruptions, and the enhanced CLI improves usability. A comprehensive testing framework was also added to ensure stability and readiness for production.
Note
Cursor Bugbot is generating a summary for commit 0b9ba58. Configure here.
Summary by CodeRabbit
New Features
Documentation
Tests