-
Notifications
You must be signed in to change notification settings - Fork 39
fix: blocked-by parser matches backtick-quoted code in task descriptions #2232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -466,7 +466,7 @@ build_todo_context() { | |||||
| if [[ -n "$task_id" ]] && echo "$_db_done_ids" | grep -qxF "$task_id"; then | ||||||
| continue | ||||||
| fi | ||||||
| blocker=$(echo "$line" | grep -oE 'blocked-by:[^ ]+' | head -1) | ||||||
| blocker=$(echo "$line" | grep -oE 'blocked-by:t[0-9][^ ]*' | head -1) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While the new regex
Suggested change
References
|
||||||
| output+="- $task_id ($blocker)\n" | ||||||
| done <<<"$blocked_tasks" | ||||||
| fi | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -389,9 +389,10 @@ is_task_blocked() { | |||||||||||||||||||||||||||
| local task_line="$1" | ||||||||||||||||||||||||||||
| local todo_file="$2" | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # Extract blocked-by: field | ||||||||||||||||||||||||||||
| # Extract blocked-by: field — require value to start with a task ID (tNNN) | ||||||||||||||||||||||||||||
| # to avoid matching backtick-quoted mentions like `blocked-by:` in descriptions | ||||||||||||||||||||||||||||
| local blocked_by | ||||||||||||||||||||||||||||
| blocked_by=$(printf '%s' "$task_line" | grep -oE 'blocked-by:[^ ]+' | sed 's/blocked-by://' || true) | ||||||||||||||||||||||||||||
| blocked_by=$(printf '%s' "$task_line" | grep -oE 'blocked-by:t[0-9][^ ]*' | sed 's/blocked-by://' || true) | ||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While the new regex
Suggested change
Comment on lines
+392
to
+395
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Harden blocked‑by parsing to ignore backtick‑quoted inline code Line 395 can still match 🔧 Suggested regex tightening- blocked_by=$(printf '%s' "$task_line" | grep -oE 'blocked-by:t[0-9][^ ]*' | sed 's/blocked-by://' || true)
+ blocked_by=$(printf '%s' "$task_line" |
+ grep -oE '(^|[[:space:]])blocked-by:t[0-9]+(\.[0-9]+)*(,[[:space:]]*t[0-9]+(\.[0-9]+)*)*' |
+ sed -E 's/^[[:space:]]*blocked-by://' || true)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if [[ -z "$blocked_by" ]]; then | ||||||||||||||||||||||||||||
| return 1 # No dependencies — not blocked | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -558,7 +558,7 @@ _execute_sanity_action() { | |||||
| local current_line | ||||||
| current_line=$(sed -n "${line_num}p" "$todo_file") | ||||||
| local blocked_by | ||||||
| blocked_by=$(printf '%s' "$current_line" | grep -oE 'blocked-by:[^ ]+' | head -1 | sed 's/blocked-by://' || echo "") | ||||||
| blocked_by=$(printf '%s' "$current_line" | grep -oE 'blocked-by:t[0-9][^ ]*' | head -1 | sed 's/blocked-by://' || echo "") | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While the new regex
Suggested change
References
|
||||||
|
|
||||||
| if [[ -z "$blocked_by" ]]; then | ||||||
| echo "no blocked-by field found" | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1174,7 +1174,7 @@ auto_unblock_resolved_tasks() { | |||||
|
|
||||||
| # Extract blocked-by dependencies | ||||||
| local blocked_by="" | ||||||
| blocked_by=$(printf '%s' "$line" | grep -oE 'blocked-by:[^ ]+' | head -1 | sed 's/blocked-by://' || echo "") | ||||||
| blocked_by=$(printf '%s' "$line" | grep -oE 'blocked-by:t[0-9][^ ]*' | head -1 | sed 's/blocked-by://' || echo "") | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While the new regex
Suggested change
References
|
||||||
| [[ -z "$blocked_by" ]] && continue | ||||||
|
|
||||||
| # Check each blocker | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the new regex
t[0-9][^ ]*correctly fixes the issue with backticks, it's still a bit permissive as[^ ]*can match any non-space characters. To make the pattern more robust and specific to the task ID format (which consists oft, digits, dots, and commas for lists), I suggest usingt[0-9.,]+. This will prevent matching invalid characters that might appear adjacent to theblocked-by:field.