t1363.4: Self-evolution loop — entity pattern scanner#2536
t1363.4: Self-evolution loop — entity pattern scanner#2536alex-solovyev merged 7 commits intomainfrom
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds an interval guard with persisted state and a --force bypass to pulse-scan, new SQL LIKE-escape helper, tolerant CLI handlers for unknown options, timestamp recording for scans, and tests validating the force bypass and related behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "CLI User"
participant Helper as "self-evolution-helper.sh"
participant FS as "EVOL_STATE_FILE (disk)"
participant DB as "SQLite DB"
CLI->>Helper: run `pulse-scan` [--force?]
Helper->>FS: read last-run timestamp
alt not forced and interval not elapsed
Helper-->>CLI: warn and exit (interval guard)
else
Helper->>DB: perform scan queries
DB-->>Helper: scan results
Helper->>DB: detect/insert/update gaps
Helper->>FS: write current timestamp (record_scan_timestamp)
Helper-->>CLI: report summary
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Feb 28 00:34:50 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
.agents/scripts/evolution-scanner-helper.sh (2)
65-68: SQL escape function only handles single quotes.The
sql_escapefunction doubles single quotes, which is the primary concern for SQLite string literals. However, the function is used in contexts where the escaped value is embedded in LIKE patterns (e.g., line 256), and special LIKE characters (%,_) are not escaped. This could cause unexpected matching behavior if user-supplied descriptions contain these characters.For a self-evolution scanner with internal data, this is low risk, but worth noting.
💡 Optional: Escape LIKE wildcards when used in pattern matching
+sql_escape_like() { + local val="$1" + val="${val//\'/\'\'}" + val="${val//%/\\%}" + val="${val//_/\\_}" + echo "$val" +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/evolution-scanner-helper.sh around lines 65 - 68, The sql_escape function only doubles single quotes, but values used inside SQL LIKE patterns also need % and _ escaped to avoid unintended matches; update sql_escape (or add a new helper like sql_escape_like) to optionally escape LIKE wildcards by replacing % with \% and _ with \_ (and ensure the escape-char syntax matches the SQLite query using ESCAPE '\\'), then update call sites that build LIKE patterns (e.g., where descriptions are wrapped in LIKE) to use the LIKE-aware escape function so wildcard characters in user-supplied values are neutralized.
406-406: Unknown arguments are silently ignored.The pattern
*) shift ;;silently discards unrecognized options. While this provides forward compatibility, it can mask typos (e.g.,--dry-rninstead of--dry-run). Consider logging a warning for unknown options in verbose mode, or documenting this behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/evolution-scanner-helper.sh at line 406, The argument-parsing fallback `*) shift ;;` silently drops unknown options; update the case branch that matches unknown args to emit a warning when verbose mode is enabled (e.g., check `VERBOSE` or `-v` flag) instead of silently discarding: replace the `*) shift ;;` branch with logic that prints a warning like "Unknown option: $1" to stderr when `VERBOSE` is set (and still `shift` afterwards), or accumulate unknown args in an array (e.g., `UNKNOWN_ARGS+=("$1")`) for later reporting; reference the existing case branch pattern `*) shift ;;` to locate and modify this behavior.tests/test-evolution-scanner.sh (2)
162-169: Entity ID extraction could be fragile.The regex
'ent_[0-9]+_[0-9a-f]+'assumes a specific entity ID format. Ifentity-helper.shchanges its ID format, this test will silently skip entity-dependent tests. Consider also checking the helper's documented output format.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test-evolution-scanner.sh` around lines 162 - 169, The entity ID extraction using the brittle regex 'ent_[0-9]+_[0-9a-f]+' when calling ENTITY_HELPER can fail if the helper output format changes; update the invocation to request a stable machine-readable format (e.g., JSON or an explicit ID flag) from ENTITY_HELPER and parse that (for example use ENTITY_HELPER ... --json and extract the id with jq or use a dedicated --id-only flag), then assign the result to entity_id and only skip when that parsed value is empty; reference the ENTITY_HELPER invocation and the entity_id variable so the change replaces the grep/regex extraction with a robust parser of the helper's documented output.
425-431: Test doesn't verify--forceactually bypasses interval guard.This test invokes
pulse --forceand checks for generic output, but doesn't verify that the interval guard was actually bypassed. Given the bug identified incmd_pulsewhere--forceis not properly tracked, this test provides false confidence. Consider adding a test that:
- Runs pulse twice in quick succession with
--force- Verifies both scans executed (vs. second being skipped)
💡 Proposed additional test
# Test --force actually bypasses interval guard pulse_force1=$(bash "$EVOLUTION_HELPER" pulse --force 2>&1) || true pulse_force2=$(bash "$EVOLUTION_HELPER" pulse --force 2>&1) || true # Both should run (not skip due to interval) if echo "$pulse_force1" | grep -qi "scan\|complete" && echo "$pulse_force2" | grep -qi "scan\|complete"; then pass "--force bypasses interval guard on consecutive runs" else fail "--force should allow consecutive scans" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test-evolution-scanner.sh` around lines 425 - 431, Add a test that asserts --force truly bypasses the interval guard by invoking the pulse helper twice in quick succession with --force (e.g., capture outputs into pulse_force1 and pulse_force2 using bash "$EVOLUTION_HELPER" pulse --force) and then verify both outputs contain scan/complete indicators (same grep logic used for the existing pulse_output check); if both contain the expected strings call pass with a descriptive message, otherwise call fail indicating --force did not allow consecutive scans. Ensure you reuse the same grep tokens ("scan|scanning|complete|not initialized") and the same pass/fail helpers so the new check integrates with the existing test harness.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/scripts/evolution-scanner-helper.sh:
- Around line 419-421: The timestamp fallback is inconsistent between scan_start
and scan_end causing wrong scan_duration_ms; detect the supported date format
once at the beginning (e.g., test if date +%s%3N works) and store a flag or use
a normalized function, then always produce millisecond timestamps for both
scan_start and scan_end (or convert seconds to milliseconds by multiplying by
1000) before computing scan_duration_ms; update references to scan_start,
scan_end, and scan_duration_ms to use this single normalized timestamp method.
- Around line 254-261: The SQL LIKE uses the raw keyword variable so escape it
before embedding: run the same sql_escape utility used elsewhere on the keyword
(e.g., call sql_escape on the variable produced by the tr pipeline) and use that
escaped value in the evolution_db query for EVOLUTION_MEMORY_DB (the SELECT on
capability_gaps.description). Update the code around the match assignment that
invokes evolution_db to reference the escaped keyword variable instead of
$keyword to prevent injection and follow the established pattern.
- Around line 980-991: The --force flag is being shifted away in the
argument-parsing loop and never recorded, so the later check if [[ "$1" !=
"--force" ]] is meaningless; modify the parser to set a boolean variable (e.g.,
FORCE=true) when the --force case is hit in the while/case that currently
contains "--force) shift ;;", and then change the interval guard to check that
variable (e.g., if [[ "$FORCE" != "true" ]] && ! check_scan_interval; then) so
the flag actually bypasses check_scan_interval; reference the case branch that
handles "--force" and the check_scan_interval call.
- Around line 79-80: The DB initialization call using evolution_db and
EVOLUTION_MEMORY_DB must enable SQLite foreign key enforcement so ON DELETE
CASCADE works; update the evolution_db invocation (the line calling evolution_db
"$EVOLUTION_MEMORY_DB" "PRAGMA journal_mode=WAL; PRAGMA busy_timeout=5000;"
>/dev/null 2>&1) to include PRAGMA foreign_keys=ON in the same PRAGMA string,
preserving the redirection and ordering with the existing journal_mode and
busy_timeout settings.
---
Nitpick comments:
In @.agents/scripts/evolution-scanner-helper.sh:
- Around line 65-68: The sql_escape function only doubles single quotes, but
values used inside SQL LIKE patterns also need % and _ escaped to avoid
unintended matches; update sql_escape (or add a new helper like sql_escape_like)
to optionally escape LIKE wildcards by replacing % with \% and _ with \_ (and
ensure the escape-char syntax matches the SQLite query using ESCAPE '\\'), then
update call sites that build LIKE patterns (e.g., where descriptions are wrapped
in LIKE) to use the LIKE-aware escape function so wildcard characters in
user-supplied values are neutralized.
- Line 406: The argument-parsing fallback `*) shift ;;` silently drops unknown
options; update the case branch that matches unknown args to emit a warning when
verbose mode is enabled (e.g., check `VERBOSE` or `-v` flag) instead of silently
discarding: replace the `*) shift ;;` branch with logic that prints a warning
like "Unknown option: $1" to stderr when `VERBOSE` is set (and still `shift`
afterwards), or accumulate unknown args in an array (e.g.,
`UNKNOWN_ARGS+=("$1")`) for later reporting; reference the existing case branch
pattern `*) shift ;;` to locate and modify this behavior.
In `@tests/test-evolution-scanner.sh`:
- Around line 162-169: The entity ID extraction using the brittle regex
'ent_[0-9]+_[0-9a-f]+' when calling ENTITY_HELPER can fail if the helper output
format changes; update the invocation to request a stable machine-readable
format (e.g., JSON or an explicit ID flag) from ENTITY_HELPER and parse that
(for example use ENTITY_HELPER ... --json and extract the id with jq or use a
dedicated --id-only flag), then assign the result to entity_id and only skip
when that parsed value is empty; reference the ENTITY_HELPER invocation and the
entity_id variable so the change replaces the grep/regex extraction with a
robust parser of the helper's documented output.
- Around line 425-431: Add a test that asserts --force truly bypasses the
interval guard by invoking the pulse helper twice in quick succession with
--force (e.g., capture outputs into pulse_force1 and pulse_force2 using bash
"$EVOLUTION_HELPER" pulse --force) and then verify both outputs contain
scan/complete indicators (same grep logic used for the existing pulse_output
check); if both contain the expected strings call pass with a descriptive
message, otherwise call fail indicating --force did not allow consecutive scans.
Ensure you reuse the same grep tokens ("scan|scanning|complete|not initialized")
and the same pass/fail helpers so the new check integrates with the existing
test harness.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 932566e and 0ddaee2badf255ec829637f05628fb9bf19c072f.
📒 Files selected for processing (2)
.agents/scripts/evolution-scanner-helper.shtests/test-evolution-scanner.sh
| # Ensure WAL mode | ||
| evolution_db "$EVOLUTION_MEMORY_DB" "PRAGMA journal_mode=WAL; PRAGMA busy_timeout=5000;" >/dev/null 2>&1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for foreign_keys pragma usage across the codebase
rg -n "foreign_keys" --type=shRepository: marcusquinn/aidevops
Length of output: 598
🏁 Script executed:
# Also search in all file types to be thorough
rg -n "foreign_keys"Repository: marcusquinn/aidevops
Length of output: 598
🏁 Script executed:
# Read the evolution-scanner-helper.sh file to understand the context
wc -l .agents/scripts/evolution-scanner-helper.shRepository: marcusquinn/aidevops
Length of output: 112
🏁 Script executed:
# Read the relevant section around lines 79-80
sed -n '70,90p' .agents/scripts/evolution-scanner-helper.shRepository: marcusquinn/aidevops
Length of output: 866
🏁 Script executed:
# Search for the schema definition and ON DELETE CASCADE references
rg -n "ON DELETE CASCADE" --type=sh -A 2 -B 2Repository: marcusquinn/aidevops
Length of output: 7566
🏁 Script executed:
# Find the evolution_db function definition
rg -n "evolution_db\s*\(" --type=sh -A 5 | head -40Repository: marcusquinn/aidevops
Length of output: 938
🏁 Script executed:
# Search for any additional PRAGMA statements in evolution-scanner-helper.sh
rg -n "PRAGMA" .agents/scripts/evolution-scanner-helper.shRepository: marcusquinn/aidevops
Length of output: 172
🏁 Script executed:
# Check if there's any initialization or setup that might enable foreign_keys globally
rg -n "EVOLUTION_MEMORY_DB" .agents/scripts/evolution-scanner-helper.sh | head -20Repository: marcusquinn/aidevops
Length of output: 1207
Enable foreign key constraint enforcement in database initialization.
SQLite does not enforce foreign keys by default. While the schema defines ON DELETE CASCADE constraints (lines 109, 124), they will not function without PRAGMA foreign_keys=ON. Add it during initialization alongside WAL mode:
# Ensure WAL mode
- evolution_db "$EVOLUTION_MEMORY_DB" "PRAGMA journal_mode=WAL; PRAGMA busy_timeout=5000;" >/dev/null 2>&1
+ evolution_db "$EVOLUTION_MEMORY_DB" "PRAGMA journal_mode=WAL; PRAGMA busy_timeout=5000; PRAGMA foreign_keys=ON;" >/dev/null 2>&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.
| # Ensure WAL mode | |
| evolution_db "$EVOLUTION_MEMORY_DB" "PRAGMA journal_mode=WAL; PRAGMA busy_timeout=5000;" >/dev/null 2>&1 | |
| # Ensure WAL mode | |
| evolution_db "$EVOLUTION_MEMORY_DB" "PRAGMA journal_mode=WAL; PRAGMA busy_timeout=5000; PRAGMA foreign_keys=ON;" >/dev/null 2>&1 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/scripts/evolution-scanner-helper.sh around lines 79 - 80, The DB
initialization call using evolution_db and EVOLUTION_MEMORY_DB must enable
SQLite foreign key enforcement so ON DELETE CASCADE works; update the
evolution_db invocation (the line calling evolution_db "$EVOLUTION_MEMORY_DB"
"PRAGMA journal_mode=WAL; PRAGMA busy_timeout=5000;" >/dev/null 2>&1) to include
PRAGMA foreign_keys=ON in the same PRAGMA string, preserving the redirection and
ordering with the existing journal_mode and busy_timeout settings.
| local match | ||
| match=$(evolution_db "$EVOLUTION_MEMORY_DB" \ | ||
| "SELECT id FROM capability_gaps WHERE status IN ('detected', 'todo_created') AND lower(description) LIKE '%${keyword}%' LIMIT 1;" 2>/dev/null || echo "") | ||
| if [[ -n "$match" ]]; then | ||
| gap_id="$match" | ||
| break | ||
| fi | ||
| done <<<"$keywords" |
There was a problem hiding this comment.
Missing SQL escape for keyword in LIKE query.
The keyword variable is inserted directly into the SQL LIKE clause without calling sql_escape. While the tr -cs '[:alnum:]' pipeline filters to alphanumeric characters only, making injection unlikely, explicitly escaping is defensive and aligns with the pattern used elsewhere.
🛡️ Proposed fix
while IFS= read -r keyword; do
[[ -z "$keyword" ]] && continue
+ local esc_keyword
+ esc_keyword=$(sql_escape "$keyword")
local match
match=$(evolution_db "$EVOLUTION_MEMORY_DB" \
- "SELECT id FROM capability_gaps WHERE status IN ('detected', 'todo_created') AND lower(description) LIKE '%${keyword}%' LIMIT 1;" 2>/dev/null || echo "")
+ "SELECT id FROM capability_gaps WHERE status IN ('detected', 'todo_created') AND lower(description) LIKE '%${esc_keyword}%' LIMIT 1;" 2>/dev/null || echo "")📝 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.
| local match | |
| match=$(evolution_db "$EVOLUTION_MEMORY_DB" \ | |
| "SELECT id FROM capability_gaps WHERE status IN ('detected', 'todo_created') AND lower(description) LIKE '%${keyword}%' LIMIT 1;" 2>/dev/null || echo "") | |
| if [[ -n "$match" ]]; then | |
| gap_id="$match" | |
| break | |
| fi | |
| done <<<"$keywords" | |
| local esc_keyword | |
| esc_keyword=$(sql_escape "$keyword") | |
| local match | |
| match=$(evolution_db "$EVOLUTION_MEMORY_DB" \ | |
| "SELECT id FROM capability_gaps WHERE status IN ('detected', 'todo_created') AND lower(description) LIKE '%${esc_keyword}%' LIMIT 1;" 2>/dev/null || echo "") | |
| if [[ -n "$match" ]]; then | |
| gap_id="$match" | |
| break | |
| fi | |
| done <<<"$keywords" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/scripts/evolution-scanner-helper.sh around lines 254 - 261, The SQL
LIKE uses the raw keyword variable so escape it before embedding: run the same
sql_escape utility used elsewhere on the keyword (e.g., call sql_escape on the
variable produced by the tr pipeline) and use that escaped value in the
evolution_db query for EVOLUTION_MEMORY_DB (the SELECT on
capability_gaps.description). Update the code around the match assignment that
invokes evolution_db to reference the escaped keyword variable instead of
$keyword to prevent injection and follow the established pattern.
| local scan_start | ||
| scan_start=$(date +%s%3N 2>/dev/null || date +%s) | ||
|
|
There was a problem hiding this comment.
Inconsistent time format fallback may cause incorrect duration calculation.
Lines 420 and 648 use date +%s%3N with a fallback to date +%s. If one succeeds with milliseconds (13 digits) and the other falls back to seconds (10 digits), the subtraction produces an incorrect scan_duration_ms. Both timestamps should use the same format.
🐛 Proposed fix — determine format once at scan start
local scan_start
- scan_start=$(date +%s%3N 2>/dev/null || date +%s)
+ local time_format="%s%3N"
+ if ! date +"$time_format" >/dev/null 2>&1; then
+ time_format="%s"
+ fi
+ scan_start=$(date +"$time_format")
# ... later ...
local scan_end
- scan_end=$(date +%s%3N 2>/dev/null || date +%s)
+ scan_end=$(date +"$time_format")Also applies to: 647-649
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/scripts/evolution-scanner-helper.sh around lines 419 - 421, The
timestamp fallback is inconsistent between scan_start and scan_end causing wrong
scan_duration_ms; detect the supported date format once at the beginning (e.g.,
test if date +%s%3N works) and store a flag or use a normalized function, then
always produce millisecond timestamps for both scan_start and scan_end (or
convert seconds to milliseconds by multiplying by 1000) before computing
scan_duration_ms; update references to scan_start, scan_end, and
scan_duration_ms to use this single normalized timestamp method.
| --force) | ||
| # Skip interval check | ||
| shift | ||
| ;; | ||
| *) shift ;; | ||
| esac | ||
| done | ||
|
|
||
| # Check interval guard (unless --force) | ||
| if [[ "$1" != "--force" ]] && ! check_scan_interval; then | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
Bug: --force flag is consumed but not tracked, making it ineffective.
The --force option is shifted in the while loop (lines 982-983) but no variable records it. Then line 989 checks "$1" != "--force", but $1 is empty after the loop completes. This means --force has no effect and the interval check always runs.
🐛 Proposed fix
cmd_pulse() {
local dry_run=false
+ local force=false
while [[ $# -gt 0 ]]; do
case "$1" in
--dry-run)
dry_run=true
shift
;;
--force)
- # Skip interval check
+ force=true
shift
;;
*) shift ;;
esac
done
# Check interval guard (unless --force)
- if [[ "$1" != "--force" ]] && ! check_scan_interval; then
+ if [[ "$force" != true ]] && ! check_scan_interval; then
return 0
fi📝 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.
| --force) | |
| # Skip interval check | |
| shift | |
| ;; | |
| *) shift ;; | |
| esac | |
| done | |
| # Check interval guard (unless --force) | |
| if [[ "$1" != "--force" ]] && ! check_scan_interval; then | |
| return 0 | |
| fi | |
| --force) | |
| force=true | |
| shift | |
| ;; | |
| *) shift ;; | |
| esac | |
| done | |
| # Check interval guard (unless --force) | |
| if [[ "$force" != true ]] && ! check_scan_interval; then | |
| return 0 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/scripts/evolution-scanner-helper.sh around lines 980 - 991, The
--force flag is being shifted away in the argument-parsing loop and never
recorded, so the later check if [[ "$1" != "--force" ]] is meaningless; modify
the parser to set a boolean variable (e.g., FORCE=true) when the --force case is
hit in the while/case that currently contains "--force) shift ;;", and then
change the interval guard to check that variable (e.g., if [[ "$FORCE" != "true"
]] && ! check_scan_interval; then) so the flag actually bypasses
check_scan_interval; reference the case branch that handles "--force" and the
check_scan_interval call.
marcusquinn
force-pushed
the
feature/self-evolution-loop
branch
from
0ddaee2 to
ebc2614
Compare
|
Worker running 4h+ with no PR activity — killed as stuck. Re-dispatching fresh worker to address CodeRabbit review feedback. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…63.4) - Add --force flag + interval guard (6h) to pulse-scan command - Add evol_sql_escape_like() for safe LIKE pattern escaping - Warn on unknown args in all 7 argument parsers (was silent discard) - Validate entity ID format in test helper (catch upstream changes) - Add --force bypass test for pulse-scan interval guard Issues 2-4 from review (SQL injection in LIKE, timestamp inconsistency, foreign keys) were already resolved in the rewrite from evolution-scanner-helper.sh to self-evolution-helper.sh.
…363.4) - Fix misleading comment: said 'LIKE matching' but query uses exact '=' match - Add SC2329 disable for evol_sql_escape_like() utility (unused but intentional) - Remove unused gap ID variables in tests (SC2034) — side-effect-only calls - All 55 tests pass, ShellCheck clean
marcusquinn
force-pushed
the
feature/self-evolution-loop
branch
from
fbc1665 to
da1923d
Compare
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Feb 28 01:17:24 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
…d (t1363.4) The previous test used --dry-run for both calls, which returns before record_scan_timestamp, so the interval guard was never activated. The test passed but didn't prove --force works. Now: write a recent timestamp to the state file first, verify the guard blocks without --force, then verify --force bypasses it. Restores original state file after test. 58/58 tests pass.
CodeRabbit Review — All Issues AddressedAll 7 actionable items from the review have been resolved across 3 commits: Fixes (in
|
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Feb 28 01:21:53 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.agents/scripts/self-evolution-helper.sh (2)
1061-1064: Minor inconsistency in unknown option warning format.Other commands use the pattern
log_warn "command-name: unknown option: $1"(e.g.,scan-patterns:,detect-gaps:), butpulse-scanomits the command prefix. For consistency and easier log grep:🔧 Suggested fix
*) - log_warn "Unknown option: $1" + log_warn "pulse-scan: unknown option: $1" shift ;;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/self-evolution-helper.sh around lines 1061 - 1064, Update the unknown-option handler in the pulse-scan branch so the warning matches other commands: change the log_warn invocation in the default case (the *) from log_warn "Unknown option: $1" to include the command prefix, e.g. log_warn "pulse-scan: unknown option: $1", then shift and continue; this keeps the message format consistent with other handlers like scan-patterns and detect-gaps.
988-1011: Robust interval guard, but consider corrupted state file edge case.The function handles the missing-file case well. However, if the state file is corrupted (contains non-numeric data), the arithmetic on line 1002 would fail or produce unexpected results under
set -e.🛡️ Optional defensive check
local last_run last_run=$(cat "$EVOL_STATE_FILE" 2>/dev/null || echo "0") + # Validate numeric — treat invalid as stale (allow scan) + if ! [[ "$last_run" =~ ^[0-9]+$ ]]; then + log_warn "Invalid timestamp in state file, allowing scan" + return 0 + fi local now🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/self-evolution-helper.sh around lines 988 - 1011, check_scan_interval currently reads EVOL_STATE_FILE into last_run and assumes it is numeric; if the file is corrupted this breaks the arithmetic on elapsed. Fix by validating last_run after reading (e.g., test it matches ^[0-9]+$), and if not numeric log a warning via log_warn/log_info, reset last_run to 0 (or remove/truncate EVOL_STATE_FILE) so interval_seconds and elapsed calculations remain safe; also ensure elapsed is clamped to a non-negative integer before comparison. Use the existing symbols check_scan_interval, EVOL_STATE_FILE, last_run, elapsed, interval_seconds and keep behavior identical when the file is valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test-self-evolution-helper.sh`:
- Around line 617-623: The grep alternation "\|" in the test's assert_contains
call won't work with the existing assert_contains (which uses plain grep -q);
replace the single assert_contains "$output_no_force" "interval\|Next scan" with
two explicit assertions using the existing helper: assert_contains
"$output_no_force" "interval" "interval guard blocks scan without --force" and
assert_contains "$output_no_force" "Next scan" "interval guard blocks scan
without --force (Next scan)"; this keeps using the current assert_contains
helper and ensures both expected substrings are checked (alternatively implement
a new assert_contains_regex wrapper around grep -E if you prefer a single-regex
check).
---
Nitpick comments:
In @.agents/scripts/self-evolution-helper.sh:
- Around line 1061-1064: Update the unknown-option handler in the pulse-scan
branch so the warning matches other commands: change the log_warn invocation in
the default case (the *) from log_warn "Unknown option: $1" to include the
command prefix, e.g. log_warn "pulse-scan: unknown option: $1", then shift and
continue; this keeps the message format consistent with other handlers like
scan-patterns and detect-gaps.
- Around line 988-1011: check_scan_interval currently reads EVOL_STATE_FILE into
last_run and assumes it is numeric; if the file is corrupted this breaks the
arithmetic on elapsed. Fix by validating last_run after reading (e.g., test it
matches ^[0-9]+$), and if not numeric log a warning via log_warn/log_info, reset
last_run to 0 (or remove/truncate EVOL_STATE_FILE) so interval_seconds and
elapsed calculations remain safe; also ensure elapsed is clamped to a
non-negative integer before comparison. Use the existing symbols
check_scan_interval, EVOL_STATE_FILE, last_run, elapsed, interval_seconds and
keep behavior identical when the file is valid.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 0ddaee2badf255ec829637f05628fb9bf19c072f and 34caca7.
📒 Files selected for processing (2)
.agents/scripts/self-evolution-helper.shtests/test-self-evolution-helper.sh
… (t1363.4) - Add pulse-scan: prefix to unknown option warning for consistent logging - Validate state file contains numeric timestamp before arithmetic (prevents crash on corrupted state file) - Replace non-portable grep \| alternation with grep -E in test for BSD/GNU portability
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Feb 28 01:31:03 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Supervisor pulse dispatched a worker to address the 4 actionable CodeRabbit review items:
Worker will push fixes to this branch. No new PR needed. |
|
Supervisor pulse: CI is green but CodeRabbit has CHANGES_REQUESTED with 2 actionable nitpicks. Dispatching worker to fix:
Worker running — will push fix and re-request review. |
All CodeRabbit Review Items Verified as FixedVerified the current state of Review Round 1 (00:38) —
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/scripts/self-evolution-helper.sh:
- Around line 1000-1013: In check_scan_interval(): after validating last_run is
numeric, add a bounds check against the current time (now) to detect future or
corrupted timestamps (e.g., if last_run > now or elapsed < 0) and treat those
cases as expired by logging a warning and returning 0 so scans are not
permanently suppressed; reference variables last_run, now, elapsed and
PULSE_INTERVAL_HOURS and update the existing elapsed/interval logic accordingly.
In record_scan_timestamp(): make the state write resilient by guarding mkdir and
the file write—use safe practices (mkdir -p, write to a temp file and atomically
mv, or at minimum check command exit codes) and on any persistence error log an
error but do not let the function return a failing exit (allow graceful
degradation) so a successful scan isn’t reported as failed due to timestamp
persistence issues; reference function record_scan_timestamp, the state file
write, and directory creation commands when implementing the recovery.
- Around line 1021-1025: The record_scan_timestamp() persistence currently can
fail and abort cmd_pulse_scan due to set -euo pipefail; make persistence
non-fatal by guarding mkdir/date errors inside record_scan_timestamp() (and any
callers like cmd_pulse_scan) so failures are swallowed and the function still
returns success: e.g., check/mkdir EVOL_STATE_DIR and write timestamp to
EVOL_STATE_FILE but on any mkdir/date write error log a warning to stderr and
return 0 (or use conditional commands with || true) so state-file persistence
cannot cause the overall pulse scan to exit non-zero; reference the existing
function name record_scan_timestamp and its EVOL_STATE_DIR / EVOL_STATE_FILE
variables when applying the change.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.agents/scripts/self-evolution-helper.shtests/test-self-evolution-helper.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test-self-evolution-helper.sh
…t1363.4) Address final CodeRabbit review comment: replace the inline grep -qE alternation block with two separate assert_contains calls for the interval guard message check. This uses the existing test helper consistently and checks both 'interval' and 'Next scan' substrings. 59/59 tests pass, ShellCheck clean.
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Feb 28 02:38:32 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
Supervisor pulse dispatched a worker to address the actionable CodeRabbit review comments: --force flag not recorded in arg-parsing loop (interval guard bypass broken), missing PRAGMA foreign_keys=ON in DB init, inconsistent timestamp format causing wrong scan_duration_ms, unescaped LIKE keyword variable, and missing --force bypass test. Worker running on branch feature/self-evolution-loop. |
…atal (t1363.4) Address remaining CodeRabbit review comments on PR #2536: - check_scan_interval(): detect future/corrupted timestamps that would permanently suppress scans (negative elapsed) and allow scan instead - record_scan_timestamp(): return error on mkdir/write failure instead of aborting under set -euo pipefail - cmd_pulse_scan(): catch record_scan_timestamp failure gracefully so a completed scan is not reported as failed due to state persistence issues
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Feb 28 02:46:10 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
@coderabbitai review All actionable review comments have been addressed in commits 461c644 and 90fd0ae:
All 59 tests pass. ShellCheck clean. Please re-review. |
|
✅ Database integrity (foreign keys pragma) I'll trigger a fresh review to verify all the changes in commits 461c644 and 90fd0ae. ✅ Actions performedReview triggered.
|
…eful degradation (t1363.4) - Use temp file + atomic mv to prevent partial/corrupt state files - Return 0 on all error paths so persistence failures never propagate - Clean up temp files on write or mv failure - Simplify call site since function now always succeeds - Addresses CodeRabbit CHANGES_REQUESTED on PR #2536
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Feb 28 03:12:06 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
|
@coderabbitai review All 5 actionable items from the CHANGES_REQUESTED review have been addressed:
Verification: ShellCheck clean, 59/59 tests pass. |
|
✅ --force flag properly wired through parse → variable → interval guard check With 59/59 tests passing and ShellCheck clean, I'll trigger a fresh review of the latest changes. ✅ Actions performedReview triggered.
|
|
Merging — all CI checks pass (CLEAN), CodeRabbit feedback is nitpick-only (no blocking issues). Supervisor pulse 03:16 UTC. |
2 participants
Summary
evolution-scanner-helper.sh— scans entity interaction patterns to detect capability gaps, with evidence trails linking to specific interaction IDsdetected→todo_created→resolved/wont_fixwith frequency tracking and occurrence historyArchitecture
New Tables (in existing memory.db)
gap_evidence— links gaps to interaction IDs (evidence trail)gap_occurrences— per-occurrence tracking with timestamps (trend analysis)evolution_scans— scan history with metricsCommands
scan— detect patterns in recent interactionsgaps— list detected capability gapsresolve— update gap status, link to TODO refslink— add interaction evidence to a gapstats— evolution statistics and resolution ratepulse— supervisor pulse integration (interval-guarded)Design Decisions
memory.dbalongside entity-helper.sh tables — enables cross-queries without cross-DB joinsVerification
bash tests/test-evolution-scanner.sh --verbose)Closes #2524
Summary by CodeRabbit
New Features
Improvements
Tests