From 9098f3bb6ef0671bf8dedf66a8fa35e3b5004939 Mon Sep 17 00:00:00 2001 From: imkp1 Date: Mon, 13 Jul 2026 01:41:36 +0530 Subject: [PATCH 1/4] perf(repo-finder): cut per-scan API calls from ~59-109 to 5 + 1/candidate Step 1 searched via `gh search repos` across nine queries, then Step 2 re-fetched pushed_at, open_issues_count, archived, and default_branch with one `gh api repos/OWNER/REPO` per candidate, against 50-100 candidates per scan. All four fields already ship inline in every search result. - Search via `gh api search/repositories`. The porcelain cannot return topics[], which the shortlist now carries; the REST projection returns it plus the four fields Step 2 was re-fetching. - Drop the per-candidate repo call. Step 2 reads the Step 1 record. - Nine queries -> five. Repeated `language:` unions (java|python collapse to one request); repeated `topic:` intersects, so at most one topic per query. - Delete the two free-text queries and the bare-stars catch-all: they carried neither a language nor a topic and admitted repos matching no criterion. - `archived:false` server-side; sort by stars, not updated, which reordered hourly and left the per-query cutoff non-deterministic. Live check: 5 requests, 197 unique repos after dedup, every candidate carrying all nine projected fields. Also pin the shortlist contract, which had no schema: - Add schemas/repo_shortlist.schema.json + test. - Validate before the atomic rename. validate_json degrades to a top-level required-key check without python jsonschema, so repo/scores.final are asserted separately with jq. - Fix commands/repo-finder.md rendering `.score`, a key the agent never writes. The score column printed null on every row. --- agents/repo-finder.md | 124 +++++++++++++++----- commands/repo-finder.md | 2 +- schemas/repo_shortlist.schema.json | 64 ++++++++++ tests/scripts/test_repo_shortlist_schema.sh | 65 ++++++++++ 4 files changed, 222 insertions(+), 33 deletions(-) create mode 100644 schemas/repo_shortlist.schema.json create mode 100755 tests/scripts/test_repo_shortlist_schema.sh diff --git a/agents/repo-finder.md b/agents/repo-finder.md index 9ddfec4..db27921 100644 --- a/agents/repo-finder.md +++ b/agents/repo-finder.md @@ -29,35 +29,63 @@ Search in this priority order. Spend more effort on higher-priority categories: ### Step 1: Build Search Queries -Run GitHub searches across each category. Use `gh` CLI for all API access. - -The repo slug field is `fullName`. `gh search repos` rejects `nameWithOwner` with -`Unknown JSON field` — that field belongs to `gh repo` and the GraphQL schema, not -to the search command. Passing it fails every query in this step, so the scan ends -with zero candidates and writes an empty shortlist that reads as a clean run. +Use `gh api search/repositories`. Do not use `gh search repos`: it cannot return +`topics[]`, which the shortlist requires. Every other required field ships inline +in the same response. + +#### Query shape rules + +Repeated qualifiers do not behave alike: + +| Qualifier | Repeated behavior | +|---|---| +| `language:` | OR — `language:go language:rust` returns Go ∪ Rust | +| `topic:` | AND — `topic:cli topic:terminal` returns only repos tagged both | + +- Union languages into a single query. Multi-language costs nothing. +- **Emit at most one `topic:` per query.** A second `topic:` intersects rather + than unions, returning a smaller or empty set with no error. +- Query count is `|topics|`, not `|langs| × |topics|`. +- Every `key:` token must be one of `{language, topic, stars, archived}`. An + unrecognized qualifier (`langauge:go`) is parsed as free text and returns + `total_count: 0` with exit 0. +- Every query must carry a `language:` or a `topic:`. A query with neither is a + catch-all that admits repos matching no criterion. +- Sort by `stars`, not `updated`. `updated` reorders hourly, making the per-query + cutoff non-deterministic. +- Pass `archived:false` as a qualifier so archived repos never consume a result + slot. ```bash -# Category 1: AI/ML -gh search repos --topic machine-learning --stars ">20000" --sort updated --limit 20 --json fullName,stargazersCount,updatedAt,description -gh search repos --topic llm --stars ">5000" --sort updated --limit 20 --json fullName,stargazersCount,updatedAt,description -gh search repos --topic artificial-intelligence --stars ">20000" --sort updated --limit 20 --json fullName,stargazersCount,updatedAt,description - -# Category 2: Language-specific -gh search repos --language java --stars ">20000" --sort updated --limit 20 --json fullName,stargazersCount,updatedAt,description -gh search repos --language python --stars ">20000" --sort updated --limit 20 --json fullName,stargazersCount,updatedAt,description +DEFAULT_QUERIES=" +topic:machine-learning stars:>20000 archived:false +topic:llm stars:>5000 archived:false +topic:artificial-intelligence stars:>20000 archived:false +topic:developer-tools stars:>20000 archived:false +language:java language:python stars:>20000 archived:false +" + +echo "$DEFAULT_QUERIES" | while IFS= read -r q; do + [ -z "$q" ] && continue + gh api -X GET search/repositories \ + -f q="$q" -f sort=stars -f order=desc -f per_page=50 \ + --jq '.items[] | {full_name, language, topics, stargazers_count, pushed_at, + archived, open_issues_count, default_branch, description}' +done +``` -# Category 3: Framework-specific -gh search repos "spring boot" --stars ">10000" --sort updated --limit 10 --json fullName,stargazersCount,updatedAt,description -gh search repos "fastapi" --stars ">5000" --sort updated --limit 10 --json fullName,stargazersCount,updatedAt,description +Guard the projection: a candidate row with a null or absent `full_name` is a hard +abort, not a skipped row. A `jq` field name that drifts from the API yields `null` +silently, and a partial candidate set reads as a clean scan. -# Category 4: Famous tools -gh search repos --topic developer-tools --stars ">20000" --sort updated --limit 20 --json fullName,stargazersCount,updatedAt,description +Retain `total_count` per query and report matched-of-total. Never widen a query to +fill the list. -# Category 5: Other active repos (catch-all) -gh search repos --stars ">20000" --sort updated --limit 20 --json fullName,stargazersCount,updatedAt,description -``` +Deduplicate across all queries. Expect 50-100 unique repos after dedup. -Deduplicate across all queries. You should have 50-100 unique repos after dedup. +Carry the full projected record forward. `topics[]` reaches the shortlist row; +`pushed_at`, `archived`, `open_issues_count`, and `default_branch` are read by +Step 2 from the record in hand. Step 2 does not re-fetch them. ### Step 2: Fast Filter (eliminate obvious non-starters) @@ -96,15 +124,25 @@ candidate in the loop below. Skipped repos do not consume API rate limit on the rest of this step. For each repo that passes the reputation gate, run these quick checks. -Skip repos that fail any: +`$CANDIDATE` is that repo's projected record from Step 1 — the full JSON object, +not just the slug. Skip repos that fail any check: ```bash -# One repo call, not three. -read -r PUSHED_AT OPEN_ISSUES ARCHIVED DEFAULT_BRANCH < <( - gh api repos/OWNER/REPO --jq '[.pushed_at, .open_issues_count, .archived, .default_branch] | @tsv') -# Skip if pushed_at older than 30 days, open_issues_count is 0, or archived. - -# Skip if no merged PRs in the last 30 days. +# Read from the Step 1 record. Do not re-fetch: `gh api repos/OWNER/REPO` here +# costs one request per candidate, 50-100 per scan, for fields already in hand. +PUSHED_AT=$(printf '%s' "$CANDIDATE" | jq -r '.pushed_at') +OPEN_ISSUES=$(printf '%s' "$CANDIDATE" | jq -r '.open_issues_count') +DEFAULT_BRANCH=$(printf '%s' "$CANDIDATE" | jq -r '.default_branch') + +# Skip if pushed_at is older than 30 days, or open_issues_count is 0. +# No archived check: `archived:false` is a Step 1 qualifier, so archived repos +# never enter the candidate set. +# The search index lags the repo endpoint by minutes to hours. That is within +# tolerance for a 30-day gate. A check needing to-the-minute accuracy must make +# its own call. + +# Skip if no merged PRs in the last 30 days. No search result carries this signal, +# so it keeps its own call: one per candidate. # # Ask the search index directly (`merged:>=YYYY-MM-DD`). Do NOT fetch `--state # merged --limit N` and filter on mergedAt: that list is ordered by CREATION @@ -545,11 +583,30 @@ If no issue scores 8+ after applying the hard filter, mark the repo as "no clear Save results to `~/.superhuman/global/repo-shortlist.json` (create the directory if missing; atomic temp-rename write): +Validate against `schemas/repo_shortlist.schema.json` before the rename. A +malformed shortlist must never reach the orchestrator. + ```bash +source "${CLAUDE_PLUGIN_ROOT}/scripts/lib/state.sh" + GLOBAL_DIR="$HOME/.superhuman/global" mkdir -p "$GLOBAL_DIR" TMP="$GLOBAL_DIR/repo-shortlist.json.tmp.$$" -printf '%s' "$SHORTLIST" | jq . > "$TMP" && mv "$TMP" "$GLOBAL_DIR/repo-shortlist.json" +printf '%s' "$SHORTLIST" | jq . > "$TMP" \ + || { echo "FATAL: shortlist is not valid JSON" >&2; rm -f "$TMP"; exit 1; } + +validate_json "${CLAUDE_PLUGIN_ROOT}/schemas/repo_shortlist.schema.json" "$TMP" \ + || { echo "FATAL: shortlist failed schema validation" >&2; rm -f "$TMP"; exit 1; } + +# Row invariant, asserted separately: validate_json degrades to a top-level +# required-key check when python jsonschema is absent (stock macOS runner), and +# `repo`/`scores.final` are nested inside repos[]. A null `repo` (dropped +# full_name -> repo mapping) or null score (written to `.score`) survives that +# fallback and propagates downstream as a valid-looking value. +jq -e '.repos | length > 0 and all(.repo != null and .scores.final != null)' "$TMP" >/dev/null \ + || { echo "FATAL: shortlist row missing repo or scores.final" >&2; rm -f "$TMP"; exit 1; } + +mv "$TMP" "$GLOBAL_DIR/repo-shortlist.json" ``` The orchestrator binds the top result via: @@ -558,7 +615,9 @@ The orchestrator binds the top result via: REPO=$(jq -r '.repos[0].repo' "$GLOBAL_DIR/repo-shortlist.json") ``` -Shortlist payload shape: +Shortlist payload shape. The row key is `repo`, not `full_name`: map Step 1's +`full_name` across on write, or the orchestrator's `.repos[0].repo` binds null. +`topics[]` comes from the Step 1 projection; cite it in the `notes` rationale. ```json { @@ -575,6 +634,7 @@ Shortlist payload shape: "stars": 45000, "category": "ai-ml", "language": "Python", + "topics": ["machine-learning", "llm", "python"], "description": "repo description", "scores": { "responsiveness": 9, diff --git a/commands/repo-finder.md b/commands/repo-finder.md index 14a4bdd..35c517e 100644 --- a/commands/repo-finder.md +++ b/commands/repo-finder.md @@ -58,7 +58,7 @@ if [ ! -f "$SHORTLIST" ]; then fi echo "Top $N candidates:" -jq -r '.repos[] | "\(.score)\t\(.repo)\t\(.notes // "")"' "$SHORTLIST" \ +jq -r '.repos[] | "\(.scores.final)\t\(.repo)\t\(.notes // "")"' "$SHORTLIST" \ | head -"$N" \ | awk -F'\t' 'BEGIN{printf "%-6s %-32s %s\n","score","repo","notes"} {printf "%-6s %-32s %s\n",$1,$2,$3}' diff --git a/schemas/repo_shortlist.schema.json b/schemas/repo_shortlist.schema.json new file mode 100644 index 0000000..900cc34 --- /dev/null +++ b/schemas/repo_shortlist.schema.json @@ -0,0 +1,64 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/gaurav0107/superhuman/schemas/repo_shortlist.schema.json", + "title": "repo_shortlist", + "description": "Ranked repos worth contributing to, with the single best issue candidate per repo. Consumed by opensource-contributor and the contribute/contribution-fleet commands, which bind a target via .repos[0].repo. Owner (sole writer): repo-finder.", + "type": "object", + "additionalProperties": true, + "required": ["generated_at", "repos"], + "properties": { + "generated_at": {"type": "string", "format": "date-time"}, + "criteria": { + "type": "object", + "additionalProperties": true, + "description": "the filter that produced this list, e.g. {\"min_stars\":20000,\"min_score\":60}" + }, + "repos": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": true, + "required": ["repo", "scores"], + "properties": { + "rank": {"type": "integer", "minimum": 1}, + "repo": {"type": "string", "pattern": "^[^/]+/[^/]+$", "description": "owner/repo slug. Search returns this field as full_name; map it across on write or .repos[0].repo binds null downstream."}, + "stars": {"type": "integer", "minimum": 0}, + "category": {"type": "string"}, + "language": {"type": ["string", "null"], "description": "null for repos GitHub cannot classify"}, + "topics": {"type": "array", "items": {"type": "string"}, "description": "from the Step 1 search projection; cited by the notes rationale"}, + "description": {"type": ["string", "null"]}, + "scores": { + "type": "object", + "additionalProperties": true, + "required": ["final"], + "description": "commands/repo-finder.md renders scores.final. A score written to a bare .score key renders as null on every row.", + "properties": { + "responsiveness": {"type": "number"}, + "opportunity_quality": {"type": "number"}, + "outside_contributor_track": {"type": "number"}, + "ai_friendliness": {"type": "number"}, + "category_bonus": {"type": "number"}, + "final": {"type": "number"} + } + }, + "responsiveness_detail": {"type": "object", "additionalProperties": true}, + "best_issue": { + "type": "object", + "additionalProperties": true, + "required": ["number", "url"], + "properties": { + "number": {"type": "integer"}, + "title": {"type": "string"}, + "type": {"type": "string"}, + "labels": {"type": "array", "items": {"type": "string"}}, + "has_maintainer_approval": {"type": "boolean"}, + "issue_score": {"type": "number"}, + "url": {"type": "string"} + } + }, + "notes": {"type": "string"} + } + } + } + } +} diff --git a/tests/scripts/test_repo_shortlist_schema.sh b/tests/scripts/test_repo_shortlist_schema.sh new file mode 100755 index 0000000..dbde3a1 --- /dev/null +++ b/tests/scripts/test_repo_shortlist_schema.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +set -euo pipefail +export CLAUDE_PLUGIN_ROOT="$(cd "$(dirname "$0")/../.." && pwd)" +source "$CLAUDE_PLUGIN_ROOT/scripts/lib/state.sh" +SCHEMA="$CLAUDE_PLUGIN_ROOT/schemas/repo_shortlist.schema.json" +tmpdir=$(mktemp -d) +trap 'rm -rf "$tmpdir"' EXIT + +cat > "$tmpdir/ok.json" <<'EOF' +{ + "generated_at": "2026-07-13T00:00:00Z", + "criteria": {"min_stars": 20000, "min_score": 60}, + "repos": [ + { + "rank": 1, + "repo": "owner/repo", + "stars": 45000, + "category": "ai-ml", + "language": "Python", + "topics": ["machine-learning", "llm"], + "description": "a repo", + "scores": {"responsiveness": 9, "final": 87}, + "best_issue": {"number": 1234, "url": "https://github.com/owner/repo/issues/1234"}, + "notes": "tagged llm, responsive maintainers" + } + ] +} +EOF +validate_json "$SCHEMA" "$tmpdir/ok.json" || { echo "FAIL valid repo_shortlist rejected"; exit 1; } + +echo '{"generated_at":"2026-07-13T00:00:00Z"}' > "$tmpdir/no_repos.json" +validate_json "$SCHEMA" "$tmpdir/no_repos.json" 2>/dev/null && { + echo "FAIL repo_shortlist missing repos accepted"; exit 1; } + +# validate_json degrades to a top-level required-key check when python jsonschema +# is absent, and repo/scores.final are nested inside repos[]. agents/repo-finder.md +# Step 5 asserts them with jq at write time. Keep this guard in lockstep with it. +row_guard() { # row_guard -> 0 if every row has repo + scores.final + jq -e '.repos | length > 0 and all(.repo != null and .scores.final != null)' "$1" >/dev/null +} + +row_guard "$tmpdir/ok.json" || { echo "FAIL row guard rejected a valid shortlist"; exit 1; } + +# full_name -> repo mapping dropped on write. +cat > "$tmpdir/null_repo.json" <<'EOF' +{"generated_at":"2026-07-13T00:00:00Z","repos":[{"repo":null,"scores":{"final":87}}]} +EOF +row_guard "$tmpdir/null_repo.json" 2>/dev/null && { echo "FAIL null repo accepted"; exit 1; } + +# Score written to a bare .score key instead of .scores.final. +cat > "$tmpdir/bare_score.json" <<'EOF' +{"generated_at":"2026-07-13T00:00:00Z","repos":[{"repo":"owner/repo","score":87,"scores":{}}]} +EOF +row_guard "$tmpdir/bare_score.json" 2>/dev/null && { echo "FAIL bare .score accepted"; exit 1; } + +# commands/repo-finder.md must render scores.final, not score. +RENDERED=$(jq -r '.repos[] | "\(.scores.final)\t\(.repo)"' "$tmpdir/ok.json") +case "$RENDERED" in + "87 owner/repo") ;; + *) echo "FAIL render projection: got '$RENDERED'"; exit 1 ;; +esac +grep -q 'scores\.final' "$CLAUDE_PLUGIN_ROOT/commands/repo-finder.md" || { + echo "FAIL commands/repo-finder.md does not render .scores.final"; exit 1; } + +echo "OK test_repo_shortlist_schema.sh" From dd9ae6bc0e76a5df7d62fd253b3ba756d9b184b4 Mon Sep 17 00:00:00 2001 From: imkp1 Date: Mon, 13 Jul 2026 01:54:47 +0530 Subject: [PATCH 2/4] fix(repo-finder): implement the Step 1 guards the prose already promised Review caught that Step 1 documented a hard abort on null `full_name` and matched-of-total reporting, but the query loop implemented neither: the --jq filter drilled straight into .items[], dropping total_count, with no null check. A guard that exists only in prose is the failure mode this agent is meant to remove. - Abort on a failed `gh api` search. An unchecked exit status let a rate-limited query contribute zero rows to a scan that then read as clean. - Abort on null full_name. Field-name drift yields null, not an error. - Retain total_count and report matched-of-total, reusing the same response so the request count per query is unchanged. - Feed the loop by heredoc, not `echo | while`. A piped while runs in a subshell where `exit` terminates only the subshell, so both aborts above would have been swallowed and the scan would have continued. Verified: piped form exits 0 and runs to completion after a fatal abort; heredoc form exits 1. - Pin all of it in the test, including that the loop is not piped and that the per-candidate repo call stays deleted. Also SC2155 in the new test: assign CLAUDE_PLUGIN_ROOT, then export, so a failed cd is not masked by export's exit status. --- agents/repo-finder.md | 42 +++++++++++++++------ tests/scripts/test_repo_shortlist_schema.sh | 22 ++++++++++- 2 files changed, 51 insertions(+), 13 deletions(-) diff --git a/agents/repo-finder.md b/agents/repo-finder.md index db27921..4f50d75 100644 --- a/agents/repo-finder.md +++ b/agents/repo-finder.md @@ -65,21 +65,39 @@ topic:developer-tools stars:>20000 archived:false language:java language:python stars:>20000 archived:false " -echo "$DEFAULT_QUERIES" | while IFS= read -r q; do +# Feed the loop by heredoc, not `echo ... | while`. A piped while runs in a +# subshell, where `exit` terminates only the subshell and the scan continues past +# a fatal error. +CANDIDATES="" +while IFS= read -r q; do [ -z "$q" ] && continue - gh api -X GET search/repositories \ - -f q="$q" -f sort=stars -f order=desc -f per_page=50 \ - --jq '.items[] | {full_name, language, topics, stargazers_count, pushed_at, - archived, open_issues_count, default_branch, description}' -done -``` -Guard the projection: a candidate row with a null or absent `full_name` is a hard -abort, not a skipped row. A `jq` field name that drifts from the API yields `null` -silently, and a partial candidate set reads as a clean scan. + # An error is not a verdict. A failed search must abort, never contribute zero + # rows to a scan that then reads as clean. + RESP=$(gh api -X GET search/repositories \ + -f q="$q" -f sort=stars -f order=desc -f per_page=50) \ + || { echo "FATAL: search failed for query: $q" >&2; exit 1; } + + # Field-name drift against the API yields null, not an error. Abort; do not skip + # the row. A partial candidate set is indistinguishable from a clean scan. + if printf '%s' "$RESP" | jq -e '.items | any(.full_name == null)' >/dev/null; then + echo "FATAL: null full_name in search response — projection drifted: $q" >&2 + exit 1 + fi + + # Report matched-of-total from the same response. Never widen a query to fill + # the list. + echo "$(printf '%s' "$RESP" | jq -r '.items | length') of \ +$(printf '%s' "$RESP" | jq -r '.total_count') matched: $q" >&2 -Retain `total_count` per query and report matched-of-total. Never widen a query to -fill the list. + CANDIDATES="$CANDIDATES$(printf '%s' "$RESP" | jq -c '.items[] | + {full_name, language, topics, stargazers_count, pushed_at, + archived, open_issues_count, default_branch, description}') +" +done < Date: Mon, 13 Jul 2026 02:01:31 +0530 Subject: [PATCH 3/4] test(repo-finder): execute the Step 1 guards instead of grepping for them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught that the regression checks matched literal text anywhere in the markdown, so a comment or a line of prose satisfied them. They asserted the documentation, not the behavior — the same doc-vs-code drift they were added to catch. Extract the Step 1 block and run it against a stubbed gh (no network): - failed search must exit nonzero and report FATAL - null full_name must exit nonzero and report FATAL - valid response must return rows and report matched-of-total Structural checks now run on the block with comments stripped: reject a piped `while` (its aborts cannot exit the scan) and require the heredoc form. The per-candidate call is matched on its resurrection signature — a repos/ call carrying pushed_at/open_issues_count/default_branch — not on `gh api repos/` wholesale, as review proposed: the workflow-tree, contributors, comments, and pulls calls are legitimate and must keep working. Each assertion verified against a negative control: all five fail when the behavior they guard is reverted. --- tests/scripts/test_repo_shortlist_schema.sh | 78 ++++++++++++++++----- 1 file changed, 61 insertions(+), 17 deletions(-) diff --git a/tests/scripts/test_repo_shortlist_schema.sh b/tests/scripts/test_repo_shortlist_schema.sh index 175d0b1..97a1d13 100755 --- a/tests/scripts/test_repo_shortlist_schema.sh +++ b/tests/scripts/test_repo_shortlist_schema.sh @@ -63,23 +63,67 @@ esac grep -q 'scores\.final' "$CLAUDE_PLUGIN_ROOT/commands/repo-finder.md" || { echo "FAIL commands/repo-finder.md does not render .scores.final"; exit 1; } -# Step 1 guards must exist in code, not only in prose. Each of these silently -# yields a partial candidate set that reads as a clean scan. AGENT="$CLAUDE_PLUGIN_ROOT/agents/repo-finder.md" -grep -q 'FATAL: search failed for query' "$AGENT" || { - echo "FAIL Step 1 does not abort on a failed search"; exit 1; } -grep -q 'any(.full_name == null)' "$AGENT" || { - echo "FAIL Step 1 does not abort on null full_name"; exit 1; } -grep -q 'total_count' "$AGENT" || { - echo "FAIL Step 1 does not retain total_count"; exit 1; } - -# The query loop must not be fed by a pipe: a piped `while` runs in a subshell, -# where the aborts above terminate only the subshell and the scan continues. -grep -q 'DEFAULT_QUERIES" | while' "$AGENT" && { - echo "FAIL Step 1 query loop is piped; aborts cannot exit the scan"; exit 1; } - -# The per-candidate repo call this agent exists to avoid must stay deleted. -grep -q 'gh api repos/OWNER/REPO --jq' "$AGENT" && { - echo "FAIL Step 2 re-fetches fields already carried by the Step 1 projection"; exit 1; } + +# Step 1's guards are executed, not grepped: a literal-text assertion is satisfied +# by prose or a comment and proves nothing about behavior. Extract the Step 1 +# block and run it against a stubbed gh. No network. +STEP1=$(awk ' + /^```bash$/ {inb=1; buf=""; next} + /^```$/ {if (inb && buf ~ /DEFAULT_QUERIES=/) {printf "%s", buf; exit} inb=0; buf=""; next} + inb {buf = buf $0 "\n"} +' "$AGENT") +[ -n "$STEP1" ] || { echo "FAIL could not extract the Step 1 query block"; exit 1; } +printf '%s' "$STEP1" > "$tmpdir/step1.sh" +bash -n "$tmpdir/step1.sh" || { echo "FAIL Step 1 block is not valid bash"; exit 1; } + +mkdir -p "$tmpdir/bin" +cat > "$tmpdir/bin/gh" <<'EOF' +#!/usr/bin/env bash +case "${GH_STUB_MODE:-ok}" in + fail) echo "gh: HTTP 403 rate limit exceeded" >&2; exit 1 ;; + null) echo '{"total_count":2,"items":[{"full_name":null,"language":"Go"}]}' ;; + *) echo '{"total_count":63,"items":[{"full_name":"owner/repo","language":"Go","topics":["cli"],"stargazers_count":1,"pushed_at":"2026-07-01T00:00:00Z","archived":false,"open_issues_count":3,"default_branch":"main","description":"d"}]}' ;; +esac +EOF +chmod +x "$tmpdir/bin/gh" + +run_step1() { # run_step1 -> Step 1's exit code; stdout/stderr in $tmpdir/out + GH_STUB_MODE="$1" PATH="$tmpdir/bin:$PATH" \ + bash "$tmpdir/step1.sh" > "$tmpdir/out" 2>&1 +} + +# A failed search must abort the scan. Contributing zero rows instead leaves a +# partial candidate set that reads as a clean run. +run_step1 fail && { echo "FAIL Step 1 did not abort on a failed search"; exit 1; } +grep -q 'FATAL' "$tmpdir/out" || { echo "FAIL failed search did not report FATAL"; exit 1; } + +# Field-name drift against the API yields null, not an error. +run_step1 null && { echo "FAIL Step 1 did not abort on null full_name"; exit 1; } +grep -q 'FATAL' "$tmpdir/out" || { echo "FAIL null full_name did not report FATAL"; exit 1; } + +# Happy path: still returns rows, and reports matched-of-total from the response. +run_step1 ok || { echo "FAIL Step 1 aborted on a valid response"; exit 1; } +grep -q 'of 63 matched' "$tmpdir/out" || { + echo "FAIL Step 1 does not report matched-of-total"; cat "$tmpdir/out"; exit 1; } + +# Both aborts above only terminate the scan if the loop runs in the current shell. +# A piped `while` runs in a subshell, where `exit` kills the subshell and the scan +# continues — the guards would pass review and do nothing. Checked on code with +# comments stripped, so a comment cannot satisfy or trip it. +STEP1_CODE=$(printf '%s' "$STEP1" | sed 's/#.*//') +printf '%s' "$STEP1_CODE" | grep -qE '\|[[:space:]]*while' && { + echo "FAIL Step 1 query loop is piped; its aborts cannot exit the scan"; exit 1; } +printf '%s' "$STEP1_CODE" | grep -q 'done <<' || { + echo "FAIL Step 1 query loop is not heredoc-fed"; exit 1; } + +# The per-candidate repo call must stay deleted. Matched on the resurrection +# signature — a repos/ call carrying the fields Step 1 already projects — not on +# `gh api repos/` wholesale: the workflow-tree, contributors, comments, and pulls +# calls are legitimate and must keep working. +sed 's/#.*//' "$AGENT" \ + | grep -E 'gh api .*repos/' \ + | grep -qE 'pushed_at|open_issues_count|default_branch' && { + echo "FAIL Step 2 re-fetches fields already carried by the Step 1 projection"; exit 1; } echo "OK test_repo_shortlist_schema.sh" From 0b05f34e1dd0c790a53f071c49c7e559061d0736 Mon Sep 17 00:00:00 2001 From: imkp1 Date: Mon, 13 Jul 2026 02:09:12 +0530 Subject: [PATCH 4/4] test(repo-finder): close two holes in the Step 1 regression checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught both, and both were real. Happy path asserted only the matched-of-total line, which "0 of 63 matched" satisfies — a regression dropping every projected candidate passed. Step 1 accumulates rows into $CANDIDATES without printing them, so the runner now dumps the variable and asserts the row and default_branch survive the projection. Refetch check matched field names on the same line as a repos/ call, so `gh api "repos/$full_name"` evaded it (root endpoint returns all four fields, no field name on the line) and a line-continued command hid the field list from a line-oriented grep. Now allowlists subresources — /git/trees, /contributors, /issues, /pulls and friends stay legal — and rejects any other repos/ path, however spelled. Matching the root form directly does not work: the owner/repo may be a variable and carry no literal slash. Fixing the first hole surfaced a bug in the harness itself: command substitution strips the trailing newline, so the appended dump fused onto the extracted block's heredoc terminator and the heredoc never closed. Verified against negative controls: root refetch via variable, line-continued, literal OWNER/REPO, and -X GET forms are each caught; the four legitimate subresource calls still pass. --- tests/scripts/test_repo_shortlist_schema.sh | 55 ++++++++++++++++----- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/tests/scripts/test_repo_shortlist_schema.sh b/tests/scripts/test_repo_shortlist_schema.sh index 97a1d13..46e0fdc 100755 --- a/tests/scripts/test_repo_shortlist_schema.sh +++ b/tests/scripts/test_repo_shortlist_schema.sh @@ -74,7 +74,10 @@ STEP1=$(awk ' inb {buf = buf $0 "\n"} ' "$AGENT") [ -n "$STEP1" ] || { echo "FAIL could not extract the Step 1 query block"; exit 1; } -printf '%s' "$STEP1" > "$tmpdir/step1.sh" +# Trailing newline is load-bearing: the block ends in a heredoc terminator, and +# command substitution strips it. Without it, anything appended below fuses onto +# the EOF line and the heredoc never closes. +printf '%s\n' "$STEP1" > "$tmpdir/step1.sh" bash -n "$tmpdir/step1.sh" || { echo "FAIL Step 1 block is not valid bash"; exit 1; } mkdir -p "$tmpdir/bin" @@ -88,9 +91,13 @@ esac EOF chmod +x "$tmpdir/bin/gh" +# Step 1 accumulates rows into $CANDIDATES without printing them, so append a dump +# to observe what the loop actually collected. An abort exits before reaching it. +{ cat "$tmpdir/step1.sh"; echo 'printf "%s" "$CANDIDATES"'; } > "$tmpdir/run.sh" + run_step1() { # run_step1 -> Step 1's exit code; stdout/stderr in $tmpdir/out GH_STUB_MODE="$1" PATH="$tmpdir/bin:$PATH" \ - bash "$tmpdir/step1.sh" > "$tmpdir/out" 2>&1 + bash "$tmpdir/run.sh" > "$tmpdir/out" 2>&1 } # A failed search must abort the scan. Contributing zero rows instead leaves a @@ -102,9 +109,15 @@ grep -q 'FATAL' "$tmpdir/out" || { echo "FAIL failed search did not report FATAL run_step1 null && { echo "FAIL Step 1 did not abort on null full_name"; exit 1; } grep -q 'FATAL' "$tmpdir/out" || { echo "FAIL null full_name did not report FATAL"; exit 1; } -# Happy path: still returns rows, and reports matched-of-total from the response. +# Happy path: the projected row must actually be collected. Asserting only on the +# matched-of-total line is satisfied by "0 of 63 matched", so a regression that +# drops every candidate would pass. run_step1 ok || { echo "FAIL Step 1 aborted on a valid response"; exit 1; } -grep -q 'of 63 matched' "$tmpdir/out" || { +grep -q '"full_name":"owner/repo"' "$tmpdir/out" || { + echo "FAIL Step 1 collected no candidate row"; cat "$tmpdir/out"; exit 1; } +grep -q '"default_branch":"main"' "$tmpdir/out" || { + echo "FAIL Step 1 projection dropped default_branch (raw() breaks without it)"; exit 1; } +grep -q '1 of 63 matched' "$tmpdir/out" || { echo "FAIL Step 1 does not report matched-of-total"; cat "$tmpdir/out"; exit 1; } # Both aborts above only terminate the scan if the loop runs in the current shell. @@ -117,13 +130,31 @@ printf '%s' "$STEP1_CODE" | grep -qE '\|[[:space:]]*while' && { printf '%s' "$STEP1_CODE" | grep -q 'done <<' || { echo "FAIL Step 1 query loop is not heredoc-fed"; exit 1; } -# The per-candidate repo call must stay deleted. Matched on the resurrection -# signature — a repos/ call carrying the fields Step 1 already projects — not on -# `gh api repos/` wholesale: the workflow-tree, contributors, comments, and pulls -# calls are legitimate and must keep working. -sed 's/#.*//' "$AGENT" \ - | grep -E 'gh api .*repos/' \ - | grep -qE 'pushed_at|open_issues_count|default_branch' && { - echo "FAIL Step 2 re-fetches fields already carried by the Step 1 projection"; exit 1; } +# The per-candidate repo call must stay deleted. Match the ROOT repos/OWNER/REPO +# endpoint, not the field names: `gh api "repos/$name"` returns all four projected +# fields whether or not a field name appears on that line, and a line-continued +# command hides the field list from a line-oriented grep entirely. +# +# Subresources are legitimate and must keep working: /git/trees, /contributors, +# /issues/comments, /pulls carry data no search result holds. Banning `gh api +# repos/` wholesale, as review proposed, would break all four. +# +# Normalize first: strip comments, join line continuations, collapse whitespace. +NORM=$(sed 's/#.*//' "$AGENT" \ + | sed -e :a -e '/\\$/{N; s/\\\n//; ta' -e '}' \ + | tr -s '[:space:]' ' ') +# +# Allowlist the subresources rather than pattern-matching the root form: the owner +# and repo may be a variable (`gh api "repos/$full_name"`), which carries no +# literal owner/repo slash to match on. Anything without a subresource is a root +# fetch, however it is spelled. +REFETCH=$(printf '%s\n' "$NORM" \ + | grep -oE "gh api [^|;]*repos/[^\"' )]*" \ + | grep -oE "repos/[^\"' )]*" \ + | grep -vE "/(git/trees|contributors|issues|pulls|commits|compare|releases|contents|labels)" \ + || true) +[ -z "$REFETCH" ] || { + echo "FAIL Step 2 re-fetches the root repos endpoint ($REFETCH); Step 1 already carries those fields" + exit 1; } echo "OK test_repo_shortlist_schema.sh"