Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions .github/schemas/signals.schema.json
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "https://github.com/petry-projects/.github/blob/main/.github/schemas/signals.schema.json",
"$comment": "version: 1.1.0 — must match SCHEMA_VERSION in collect-signals.sh; enforced by bats",
"$comment": "version: 1.0.0 — must match SCHEMA_VERSION in collect-signals.sh; enforced by bats",
"title": "Feature Ideation Signals",
"description": "Canonical contract between collect-signals.sh and the BMAD Analyst (Mary) prompt. Any change to this schema is a breaking change to the workflow.",
"type": "object",
"required": [
"schema_version",
"scan_date",
"last_successful_run",
"repo",
"open_issues",
"closed_issues_30d",
Expand All @@ -29,11 +28,6 @@
"type": "string",
"format": "date-time"
},
"last_successful_run": {
"description": "ISO-8601 timestamp of the previous successful workflow run; used as a feed checkpoint by the analyst to skip already-reviewed content.",
"type": "string",
"format": "date-time"
},
"repo": {
"type": "string",
"pattern": "^[^/]+/[^/]+$"
Expand Down
4 changes: 2 additions & 2 deletions .github/scripts/feature-ideation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ bats test/workflows/feature-ideation/gh-safe.bats
shellcheck -x collect-signals.sh lint-prompt.sh match-discussions.sh \
discussion-mutations.sh lib/*.sh)

# Lint the workflow's direct_prompt block
# Lint workflow prompt blocks (direct_prompt: and prompt:)
bash .github/scripts/feature-ideation/lint-prompt.sh
```

Expand All @@ -76,7 +76,7 @@ signals.json is a breaking change and must:

1. Bump `SCHEMA_VERSION` in `collect-signals.sh`.
2. Update fixtures under `test/workflows/feature-ideation/fixtures/expected/`.
3. Update Mary's prompt in `feature-ideation.yml` if any field references move.
3. Update Mary's prompt in `.github/workflows/feature-ideation-reusable.yml` if any field references move.

CI validates every fixture against the schema, and the workflow validates
the runtime output before handing it to Mary.
69 changes: 25 additions & 44 deletions .github/scripts/feature-ideation/collect-signals.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ set -euo pipefail
# if the constants drift, AND the bats `signals-schema: SCHEMA_VERSION
# constant matches schema file` test enforces this in CI.
# Caught by CodeRabbit review on PR petry-projects/.github#85.
SCHEMA_VERSION="1.1.0"
SCHEMA_VERSION="1.0.0"
Comment thread
don-petry marked this conversation as resolved.

SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
# shellcheck source=lib/gh-safe.sh
Expand All @@ -58,56 +58,39 @@ main() {
local discussion_limit="${DISCUSSION_LIMIT:-100}"
local output_path="${SIGNALS_OUTPUT:-./signals.json}"

local owner repo_name
owner="${REPO%%/*}"
repo_name="${REPO##*/}"
if [ "$owner" = "$REPO" ] || [ -z "$repo_name" ]; then
# Validate that limit overrides are positive integers before forwarding to
# GraphQL — a value like `ISSUE_LIMIT=foo` would cause an opaque downstream
# failure instead of a clean usage error. Caught by CodeRabbit review on
# PR petry-projects/.github#85.
local _lim_name _lim_val
for _lim_name in ISSUE_LIMIT PR_LIMIT DISCUSSION_LIMIT; do
case "$_lim_name" in
ISSUE_LIMIT) _lim_val="$issue_limit" ;;
PR_LIMIT) _lim_val="$pr_limit" ;;
DISCUSSION_LIMIT) _lim_val="$discussion_limit" ;;
esac
if [[ ! $_lim_val =~ ^[1-9][0-9]*$ ]]; then
printf '[collect-signals] %s must be a positive integer, got: %s\n' "$_lim_name" "$_lim_val" >&2
return 64
fi
done

# Strict owner/name format — reject leading/trailing slashes, empty segments,
# and extra path parts (e.g. "org//repo", "/repo", "org/repo/extra").
# Caught by CodeRabbit review on PR petry-projects/.github#85.
if [[ ! $REPO =~ ^[^/]+/[^/]+$ ]]; then
printf '[collect-signals] REPO must be in owner/name format, got: %s\n' "$REPO" >&2
return 64
fi
local owner repo_name
owner="${REPO%%/*}"
repo_name="${REPO##*/}"

local thirty_days_ago
thirty_days_ago=$(date_days_ago 30)
local scan_date
scan_date=$(date_now_iso)

# --- Feed checkpoint: last successful run ----------------------------------
# Used by the analyst to skip feed entries already reviewed. The current run
# is still in-progress, so --status=success --limit=1 reliably returns the
# previous successful run. Falls back to 30 days ago on first-ever run or
# after a long gap so the initial scan is bounded.
# WORKFLOW_FILE — caller-supplied env var for repos that name their stub
# something other than the conventional "feature-ideation.yml". Defaults to
# the conventional name; no change needed for repos that follow the standard.
printf '[collect-signals] resolving feed checkpoint (last successful run)\n' >&2
local last_successful_run _run_stderr _run_err
_run_stderr=$(mktemp)
last_successful_run=$(gh run list \
--repo "$REPO" \
--workflow="${WORKFLOW_FILE:-feature-ideation.yml}" \
--status=success \
--limit=1 \
--json createdAt \
--jq '.[0].createdAt // empty' \
2>"$_run_stderr" || true)
_run_err=$(cat "$_run_stderr")
rm -f "$_run_stderr"
# Validate that the result looks like an ISO-8601 datetime. The real `gh`
# CLI applies the --jq filter and emits a bare timestamp; in test environments
# the gh stub returns raw fixture JSON (without applying --jq), so we guard
# against that here rather than requiring every test to stub this extra call.
if [ -z "$last_successful_run" ] || [ "$last_successful_run" = "null" ] || \
! printf '%s' "$last_successful_run" | grep -qE '^[0-9]{4}-[0-9]{2}-[0-9]{2}T'; then
if [ -n "$_run_err" ]; then
printf '[collect-signals] gh run list warning: %s\n' "$_run_err" >&2
fi
last_successful_run="$(date_days_ago 30)T00:00:00Z"
printf '[collect-signals] no prior successful run found; using 30-day fallback: %s\n' \
"$last_successful_run" >&2
else
printf '[collect-signals] feed checkpoint: %s\n' "$last_successful_run" >&2
fi

local truncation_warnings='[]'

# --- Open issues -----------------------------------------------------------
Expand Down Expand Up @@ -238,7 +221,6 @@ GRAPHQL
"$bug_reports" \
"$REPO" \
"$scan_date" \
"$last_successful_run" \
"$SCHEMA_VERSION" \
"$truncation_warnings")

Expand All @@ -255,7 +237,6 @@ GRAPHQL
printf -- '- **Bug reports:** %s\n' "$(jq '.bug_reports.count' "$output_path")"
printf -- '- **Merged PRs (30d):** %s\n' "$(jq '.merged_prs_30d.count' "$output_path")"
printf -- '- **Existing Ideas discussions:** %s\n' "$(jq '.ideas_discussions.count' "$output_path")"
printf -- '- **Feed checkpoint (last successful run):** %s\n' "$(jq -r '.last_successful_run' "$output_path")"
local warn_count
warn_count=$(jq '.truncation_warnings | length' "$output_path")
if [ "$warn_count" -gt 0 ]; then
Expand Down
23 changes: 11 additions & 12 deletions .github/scripts/feature-ideation/lib/compose-signals.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,16 @@
# $7 bug_reports
# $8 repo (string, e.g. "petry-projects/talkterm")
# $9 scan_date (ISO-8601 string)
# $10 last_successful_run (ISO-8601 string; feed checkpoint)
# $11 schema_version (string)
# $12 truncation_warnings (JSON array, may be [])
# $10 schema_version (string)
# $11 truncation_warnings (JSON array, may be [])
#
# Output: signals.json document on stdout.

set -euo pipefail

compose_signals() {
if [ "$#" -ne 12 ]; then
printf '[compose-signals] expected 12 args, got %d\n' "$#" >&2
if [ "$#" -ne 11 ]; then
printf '[compose-signals] expected 11 args, got %d\n' "$#" >&2
return 64 # EX_USAGE
fi

Expand All @@ -41,25 +40,26 @@ compose_signals() {
local bug_reports="$7"
local repo="$8"
local scan_date="$9"
local last_successful_run="${10}"
local schema_version="${11}"
local truncation_warnings="${12}"
local schema_version="${10}"
local truncation_warnings="${11}"

# Validate every JSON input before composition. Better to fail loudly here
# than to let `jq --argjson` produce a cryptic parse error.
local idx=0
for input in "$open_issues" "$closed_issues" "$ideas_discussions" "$releases" \
"$merged_prs" "$feature_requests" "$bug_reports" "$truncation_warnings"; do
idx=$((idx + 1))
if ! printf '%s' "$input" | jq -e . >/dev/null 2>&1; then
printf '[compose-signals] arg #%d is not valid JSON: %s\n' "$idx" "${input:0:120}" >&2
# Require a JSON array, not just valid JSON. Objects/strings/nulls accepted
# by `jq -e .` would silently produce wrong counts (key count, char count).
# Caught by CodeRabbit review on PR petry-projects/.github#85.
if ! printf '%s' "$input" | jq -e 'type == "array"' >/dev/null 2>&1; then
printf '[compose-signals] arg #%d must be a JSON array: %s\n' "$idx" "${input:0:120}" >&2
return 65 # EX_DATAERR
fi
done
Comment thread
coderabbitai[bot] marked this conversation as resolved.

jq -n \
--arg scan_date "$scan_date" \
--arg last_successful_run "$last_successful_run" \
--arg repo "$repo" \
--arg schema_version "$schema_version" \
--argjson open_issues "$open_issues" \
Expand All @@ -73,7 +73,6 @@ compose_signals() {
'{
schema_version: $schema_version,
scan_date: $scan_date,
last_successful_run: $last_successful_run,
repo: $repo,
open_issues: { count: ($open_issues | length), items: $open_issues },
closed_issues_30d: { count: ($closed_issues | length), items: $closed_issues },
Expand Down
7 changes: 7 additions & 0 deletions .github/scripts/feature-ideation/lib/date-utils.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ set -euo pipefail

# Print an ISO date (YYYY-MM-DD) for N days ago in UTC.
date_days_ago() {
# Guard arg count before reading $1: under set -u a zero-arg call would abort
# the shell with "unbound variable" instead of reaching the validation path.
# Caught by CodeRabbit review on PR petry-projects/.github#85.
if [ "$#" -ne 1 ]; then
printf '[date-utils] expected 1 arg (days), got: %d\n' "$#" >&2
return 64
fi
local days="$1"
if [ -z "$days" ] || ! printf '%s' "$days" | grep -Eq '^[0-9]+$'; then
printf '[date-utils] days must be a non-negative integer, got: %s\n' "$days" >&2
Expand Down
8 changes: 5 additions & 3 deletions .github/scripts/feature-ideation/lib/filter-bots.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ DEFAULT_BOT_AUTHORS=(
filter_bots_build_list() {
local list=("${DEFAULT_BOT_AUTHORS[@]}")
if [ -n "${FEATURE_IDEATION_BOT_AUTHORS:-}" ]; then
local IFS=','
# shellcheck disable=SC2206
local extras=($FEATURE_IDEATION_BOT_AUTHORS)
# Use `IFS=',' read` (not unquoted expansion) to avoid pathname-globbing
# against the filesystem if any entry contains wildcard characters.
# Caught by CodeRabbit review on PR petry-projects/.github#85.
local extras=()
IFS=',' read -r -a extras <<<"${FEATURE_IDEATION_BOT_AUTHORS}"
# Trim leading/trailing whitespace from each comma-separated entry so
# `"bot1, bot2"` resolves to `bot1` and `bot2`, not `bot1` and ` bot2`.
# Caught by CodeRabbit review on PR petry-projects/.github#85.
Expand Down
26 changes: 19 additions & 7 deletions .github/scripts/feature-ideation/lib/gh-safe.sh
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ gh_safe_graphql() {
local i=0
while [ "$i" -lt "${#args[@]}" ]; do
if [ "${args[$i]}" = "--jq" ]; then
# Guard bounds before dereferencing args[i+1]: under set -u an out-of-
# bounds access aborts the shell. Caught by CodeRabbit review on PR
# petry-projects/.github#85.
if [ $((i + 1)) -ge "${#args[@]}" ]; then
_gh_safe_err "graphql-bad-args" "--jq requires a jq filter argument"
return 64
fi
has_jq=1
jq_filter="${args[$((i + 1))]}"
break
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand Down Expand Up @@ -132,13 +139,11 @@ gh_safe_graphql() {
fi

# Reject error envelopes — GraphQL returns 200 OK even on partial errors.
if printf '%s' "$raw" | jq -e '.errors // empty | if . then true else false end' >/dev/null 2>&1; then
if printf '%s' "$raw" | jq -e '(.errors | type) == "array" and (.errors | length > 0)' >/dev/null 2>&1; then
local errs
errs=$(printf '%s' "$raw" | jq -c '.errors')
_gh_safe_err "graphql-errors" "$errs"
return 66 # EX_NOINPUT — repurposed: "GraphQL refused our request"
fi
if printf '%s' "$raw" | jq -e '(.errors // empty | type) == "array" and (.errors | length > 0)' >/dev/null 2>&1; then
local errs
errs=$(printf '%s' "$raw" | jq -c '.errors')
_gh_safe_err "graphql-errors" "$errs"
return 66 # EX_NOINPUT — repurposed: "GraphQL refused our request"
fi

# Reject `data: null` — usually means the path didn't resolve (permissions,
Expand Down Expand Up @@ -187,6 +192,13 @@ gh_safe_graphql() {
# Same defensive contract as `gh_safe_graphql`: any auth/network/schema
# failure exits non-zero with a structured stderr message.
gh_safe_graphql_input() {
# Guard arg count before reading $1: under set -u a zero-arg call aborts the
# shell instead of reaching the JSON validation. Caught by CodeRabbit review
# on PR petry-projects/.github#85.
if [ "$#" -ne 1 ]; then
_gh_safe_err "graphql-bad-input" "expected 1 arg: JSON request body, got $#"
return 64
fi
local body="$1"
if ! gh_safe_is_json "$body"; then
_gh_safe_err "graphql-bad-input" "request body is not valid JSON"
Expand Down
59 changes: 53 additions & 6 deletions .github/scripts/feature-ideation/lint-prompt.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,34 @@ scan_file() {
import re
import sys


def _strip_github_expressions(s: str) -> str:
"""Remove ${{ ... }} GitHub Actions expressions from s.

Uses a stateful scanner instead of `[^}]*` regex so that expressions
containing `}` inside string literals (e.g. format() calls) are fully
consumed rather than prematurely terminated. This prevents false-positive
shell-expansion matches on content that is actually inside a GH expression.
Caught by CodeRabbit review on PR petry-projects/.github#85.
"""
result: list[str] = []
i = 0
while i < len(s):
if s[i : i + 3] == "${{":
# Consume until we find the matching "}}"
j = i + 3
while j < len(s):
if s[j : j + 2] == "}}":
j += 2
break
j += 1
i = j # skip the whole ${{ ... }} expression
else:
result.append(s[i])
i += 1
return "".join(result)


path = sys.argv[1]
try:
with open(path, "r", encoding="utf-8") as f:
Expand All @@ -55,8 +83,10 @@ findings = []
shell_expansion = re.compile(r'(?<![\\$])\$\([^)]*\)|(?<![\\$])\$\{[A-Za-z_][A-Za-z0-9_]*\}')

# Recognise both `direct_prompt:` (v0) and `prompt:` (v1) markers, with
# optional `|` or `>` block scalar indicators.
prompt_marker = re.compile(r'(?:direct_prompt|prompt):\s*[|>]?\s*$')
# optional `|` or `>` block scalar indicators plus YAML chomping modifiers
# (`-` or `+`) so `prompt: |-`, `prompt: |+`, `prompt: >-`, `prompt: >+`
# are all recognised. Caught by CodeRabbit review on PR petry-projects/.github#85.
prompt_marker = re.compile(r'(?:direct_prompt|prompt):\s*[|>]?[-+]?\s*$')

for lineno, raw in enumerate(lines, start=1):
stripped = raw.lstrip(" ")
Expand All @@ -78,8 +108,13 @@ for lineno, raw in enumerate(lines, start=1):
continue

# We're inside the prompt body. Scan for shell expansions.
# First, strip out any GitHub Actions expressions so they don't trip us.
no_gh = re.sub(r'\$\{\{[^}]*\}\}', '', raw)
# First, strip out GitHub Actions ${{ ... }} expressions.
# The naive `[^}]*` regex stops at the first `}`, so expressions that
# contain `}` internally (e.g. format() calls or string literals) are
# not fully removed and leave false-positive shell expansion matches.
# Use a small stateful scanner instead.
# Caught by CodeRabbit review on PR petry-projects/.github#85.
no_gh = _strip_github_expressions(raw)
for match in shell_expansion.finditer(no_gh):
findings.append((lineno, match.group(0), raw.rstrip()))

Expand Down Expand Up @@ -107,15 +142,27 @@ main() {
fi

local exit=0
local file_rc=0
for file in "$@"; do
if [ ! -f "$file" ]; then
printf '[lint-prompt] not found: %s\n' "$file" >&2
exit=2
continue
fi
if ! scan_file "$file"; then
exit=1
# Capture the actual exit code so we preserve exit-2 (file error) over
# exit-1 (lint finding). A later lint failure must not overwrite an earlier
# file error. Caught by CodeRabbit review on PR petry-projects/.github#85.
if scan_file "$file"; then
file_rc=0
else
file_rc=$?
fi
case "$file_rc" in
0) ;;
1) if [ "$exit" -eq 0 ]; then exit=1; fi ;;
2) exit=2 ;;
*) return "$file_rc" ;;
esac
done
return "$exit"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Expand Down
Loading