feat: Scope local-clone scanners to the core-repos allowlist - #38
Conversation
Wire the link-health and dep-bump discovery loops to the shared allowlist, so they act only on the curated core repos instead of every locally cloned directory. - program-lib.sh: add is_core_repo() (exact whole-line membership test). Document canonical_repo_for_dir() as a TRANSITIONAL shim (see rossoctl#37). - Each clone-iteration loop now: maps the dir basename to its canonical repo, skips non-core/archived repos (allowlist), and dedups duplicate clone dirs (e.g. stale kagenti/ alongside rossoctl/) so each canonical repo is processed once. API refs are built as rossoctl/<canonical>. - dep-bump-scanner: this fixes the false "0 open Dependabot PRs" report -- gh pr list --author silently returns empty across a rename redirect, so querying the canonical repo restores results (rossoctl/rossoctl now returns 11 vs 0). - extract-broken-links.sh: arg 2 is now a full owner/name ref emitted verbatim (was a bare name prefixed with kagenti/); test updated. Scope: discovery/read paths only. Fixer WRITE paths (ensure_fork / create_fork_pr / $ORG in PR creation) are intentionally left for Phase 6, where they are resolved alongside the fork-naming decision. Verified against the real host clone set: 8 core repos scanned (from 25), duplicate clones deduped, archived/non-core excluded; full test suite passes on bash 3.2. Part of rossoctl#29 (epic rossoctl#32). Assisted-By: Claude Code (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Gloire Rubambiza <gloire@ibm.com>
clawgenti
left a comment
There was a problem hiding this comment.
Solid refactor — the canonical-remap + allowlist filter + dedup pattern is applied consistently across all five scripts, and the is_core_repo helper uses grep -Fx to avoid substring false positives. Tests updated to match the new owner/name arg contract.
Finding:
dep-bump-fixer.shline 96 initialisesseen_canon=""(lowercase) while the outer-scope loops in the same file useSEEN_CANON/SEEN_CANON_TTM(uppercase). No functional impact here since the variable stays local to theifblock, but the inconsistency is worth normalising.
Reviewed by clawgenti using github:pr-review
|
|
||
| # Query merged Dependabot PRs across all repos (last 90 days) | ||
| : > "$TMPDIR/merged_prs.jsonl" | ||
| seen_canon="" |
There was a problem hiding this comment.
Naming inconsistency: seen_canon is lowercase here, but the equivalent variables in the two outer-scope loops below are SEEN_CANON and SEEN_CANON_TTM (uppercase). No functional issue since this variable is scoped to the if block, but consider renaming to SEEN_CANON for consistency with the rest of the file.
The baseline-capture loop used a lowercase seen_canon while the two later dedup loops in the same file use SEEN_CANON / SEEN_CANON_TTM. No functional change -- the variable was already block-local -- but align the casing for consistency. Addresses review nit on rossoctl#38. Assisted-By: Claude Code (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Gloire Rubambiza <gloire@ibm.com>
cwiklik
left a comment
There was a problem hiding this comment.
Scopes the link-health and dep-bump discovery loops to the core-repos allowlist and fixes the rename-redirect bug — which is the exact class of problem behind operator #498 (bots acting on stale/duplicate clone dirs and pre-rename names).
Verified:
is_core_repo()usesgrep -qxF(exact whole-line, fixed-string) so there are no substring false positives; the test explicitly guardsrosso≠rossoctl.- The allowlist +
canonical_repo_for_dirremap +SEEN_CANONdedup block is consistent across all five scanners,SEEN_CANONis reset before each loop (no cross-loop leakage), and the" $SEEN_CANON "/*" $canon "*padding is the correct whole-word membership idiom. - dep-bump-scanner's Step 1 stores the canonical name and Step 2 prefixes
rossoctl/— source parity holds across the handoff. - The
gh pr list --repo rossoctl/<canon> --authorfix matches the reported '0 vs 11' symptom (the author-filtered list doesn't follow a rename redirect). - extract-broken-links drops the hardcoded
kagenti/prefix for a verbatimowner/name(a latent rename bug removed); no lingering$REPO_NAME; tests updated and assertive. canonical_repo_for_diris clearly documented as transitional with a deprecation note and issue ref.
One non-blocking suggestion inline about factoring the duplicated filter block. DCO green, both commits signed, no .claude/.vscode. LGTM — nice structural fix.
Assisted-By: Claude Code
| # is_core_repo "$canon" || continue | ||
| # Args: | ||
| # $1 - bare repo name (no owner prefix) | ||
| is_core_repo() { |
There was a problem hiding this comment.
suggestion (non-blocking): the allowlist + canonical-remap + dedup block is now copy-pasted ~8 times across the five scanners (3× in dep-bump-fixer alone), which is real drift risk for security-sensitive automation. Consider a small program-lib helper — e.g. canon=$(repo_filter "$dir") || continue that performs the canonical_repo_for_dir remap + is_core_repo gate and echoes the canonical name, leaving only the SEEN_CANON dedup inline — so the allowlist policy lives in exactly one place.
There was a problem hiding this comment.
Great suggestion. This reinforces what I had already noticed. That is, program-lib.sh was growing too long for easy maintainability, and I have an open issue as part of the epic to refactor it into a smaller set of subprograms like you suggested. I will add this suggestion to the context for the planning.
Wire the link-health and dep-bump discovery loops to the shared core-repos allowlist introduced in #36, so they act only on the curated core repos instead of every locally cloned directory.
What changed
program-lib.sh: addis_core_repo()(exact whole-line membership test). Documentcanonical_repo_for_dir()as a transitional shim (see chore: Rename clone dirs to canonical names and retire the remap shim #37).kagenti/dir alongsiderossoctl/) so each canonical repo is processed once. API refs are built asrossoctl/<canonical>.gh pr list --authorsilently returns empty across a rename redirect, so querying the canonical repo restores results (e.g.rossoctl/rossoctlnow returns 11 vs 0).extract-broken-links.sh: arg 2 is now a fullowner/nameref emitted verbatim (was a bare name prefixed withkagenti/); test updated.Scope
Discovery / read paths only. Fixer write paths (
ensure_fork/create_fork_pr/$ORGin PR creation) are intentionally left for Phase 6, where they are resolved alongside the fork-naming decision.Verification
test-core-repos,test-extract-broken-links).Part of #29 (epic #32) — completes the Phase 4 (local-clone scanners) portion of the centralization work; #36 landed Phases 2–3.
Fixes #29
Assisted-By: Claude Code