fix(prioritize): inline CSMA library into post-script#35
Conversation
The post-prioritize script sources lib/github-api-csma.sh via SCRIPT_DIR, but base composition fetches scripts as individual content-addressed blobs without their sibling directories. This breaks every prioritize run under base composition (since 8382e8ba). Inline the CSMA functions directly into post-prioritize.sh and post-prioritize-test.sh, matching the approach other post-scripts use (e.g., post-retro.sh handles retries inline). Delete the now-unused scripts/lib/ directory. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
PR Summary by QodoFix prioritize: inline GitHub CSMA helpers into post scripts
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
|
🤖 Finished Review · ✅ Success · Started 10:05 PM UTC · Completed 10:16 PM UTC |
Code Review by Qodo
Context used✅ Compliance rules (platform):
55 rules 1. CSMA guard uses return
|
| # shellcheck shell=bash | ||
|
|
||
| [[ -n "${GITHUB_API_CSMA_SH_LOADED:-}" ]] && return 0 | ||
| GITHUB_API_CSMA_SH_LOADED=1 |
There was a problem hiding this comment.
1. Csma guard uses return 📜 Skill insight ≡ Correctness
scripts/post-prioritize.sh is executed as a standalone script, but it inlines a CSMA “already loaded” guard that uses return 0, which is invalid outside a sourced context and can error/abort unexpectedly under set -e. As a result, when GITHUB_API_CSMA_SH_LOADED is present the guard can cause the post-prioritize script to fail immediately instead of reliably handling the pre-loaded CSMA condition.
Agent Prompt
## Issue description
The inlined CSMA guard in `scripts/post-prioritize.sh` uses `return 0` at script top-level. Because the script is executed (not sourced), `return` is invalid and can cause the script to error/terminate (especially under `set -euo pipefail`) when `GITHUB_API_CSMA_SH_LOADED` is set, meaning the guard does not reliably handle the “CSMA already loaded” scenario.
## Issue Context
The guard previously made sense in the deleted sourced library (`scripts/lib/github-api-csma.sh`), but after inlining into an executable script it should not `return` from the file. The test harness shows `scripts/post-prioritize.sh` (and the related test script) are run via `bash ...`, not `source ...`, so any guard must be compatible with execution semantics and should not short-circuit the entire script incorrectly.
## Fix Focus Areas
- scripts/post-prioritize.sh[30-33]
- scripts/post-prioritize-test.sh[284-287]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| # shellcheck source=lib/github-api-csma.sh | ||
| source "${SCRIPT_DIR}/lib/github-api-csma.sh" | ||
| # --- Inlined from lib/github-api-csma.sh --- | ||
| # CSMA/CD-style resilience for GitHub API calls via gh/fullsend. | ||
| # Carrier sense: check rate_limit before transmitting. |
There was a problem hiding this comment.
2. post-retro.sh references deleted file 📜 Skill insight ≡ Correctness
The PR removes the lib/github-api-csma.sh identifier from prioritize scripts, but scripts/post-retro.sh still references github-api-csma.sh, leaving a stale identifier in the repository. This violates the requirement to eliminate stale references after an identifier is removed/retired.
Agent Prompt
## Issue description
A remaining repository reference to `github-api-csma.sh` exists after the PR removes the prior `source "${SCRIPT_DIR}/lib/github-api-csma.sh"` usage, leaving stale guidance in another script.
## Issue Context
Even though the remaining reference is in a comment, the compliance rule requires removing all stale references to removed/renamed identifiers across the repo.
## Fix Focus Areas
- scripts/post-retro.sh[141-145]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| # --- Inlined from lib/github-api-csma.sh --- | ||
| # CSMA/CD-style resilience for GitHub API calls via gh/fullsend. | ||
| # Carrier sense: check rate_limit before transmitting. | ||
| # Slot time: random jitter between calls to desynchronize parallel runners. | ||
| # Collision detection: retry on 429 / secondary rate limit errors with exponential backoff. | ||
| # |
There was a problem hiding this comment.
| [[ -n "${GITHUB_API_CSMA_SH_LOADED:-}" ]] && return 0 | ||
| GITHUB_API_CSMA_SH_LOADED=1 |
There was a problem hiding this comment.
4. Top-level return in tests 🐞 Bug ☼ Reliability
scripts/post-prioritize-test.sh also includes the inlined CSMA guard `[[ -n
"${GITHUB_API_CSMA_SH_LOADED:-}" ]] && return 0` at top level, so the test runner will crash if that
env var is set before invocation. This makes the test suite brittle to ambient environment
variables.
Agent Prompt
## Issue description
`post-prioritize-test.sh` is an executable script, but it inlines a library-style guard that uses `return 0` at top level. If `GITHUB_API_CSMA_SH_LOADED` is set in the environment, the test script will error immediately.
## Issue Context
This guard was originally designed for a sourced library file. In an executable test script, it should only prevent re-definition of functions, not attempt to return from the script.
## Fix Focus Areas
- scripts/post-prioritize-test.sh[286-287]
### Suggested change
Use the same pattern as the production script fix:
```bash
if [[ -z "${GITHUB_API_CSMA_SH_LOADED:-}" ]]; then
GITHUB_API_CSMA_SH_LOADED=1
# define CSMA functions...
fi
```
(or remove the guard entirely).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
ReviewVerdict: Approve This PR fixes a production breakage by inlining the CSMA/CD rate-limit resilience library ( The approach is sound: base composition fetches scripts as individual content-addressed blobs, and Findings1. Dead source-guard in inlined code · The inlined code carries over Remediation: Remove both the guard line and the 2. Code duplication drift risk · ~315 lines of identical CSMA library code now exist in two files with no mechanism to detect divergence. Future bug fixes or enhancements must be applied to both copies. Remediation: Consider a CI check (e.g., extract the inlined blocks and 3. Stale comment reference to deleted library · Comment reads: "Note: we handle 401/403 inline rather than relying on github-api-csma.sh" — the library no longer exists after this PR. The comment is explanatory (not functional) so this is cosmetic. Remediation: Update the comment to reference "the CSMA pattern used in post-prioritize.sh" or similar.
Labels: PR fixes a bug in script composition affecting the prioritize agent. |
Summary
scripts/lib/github-api-csma.shdirectly intopost-prioritize.shandpost-prioritize-test.shscripts/lib/directoryProblem
Base composition (
base:in harness YAML) fetches each script as an individual content-addressed blob. The post-prioritize script doessource "${SCRIPT_DIR}/lib/github-api-csma.sh", butlib/doesn't exist next to the cached blob. This has broken every prioritize run since the July 1 migration to base composition.Fix
Inline the CSMA functions, matching the pattern used by other post-scripts (e.g., post-retro.sh handles retries inline rather than sourcing a library).
Companion PR: fullsend-ai/fullsend#3182
Test plan
bash scripts/post-prioritize-test.sh— all 7 tests passshellcheck— clean