Skip to content

fix(#174): remove invalid top-level return 0 load guard from github-api-csma.sh - #298

Closed
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/174-remove-csma-load-guard
Closed

fix(#174): remove invalid top-level return 0 load guard from github-api-csma.sh#298
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/174-remove-csma-load-guard

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown

The library-style load guard ([[ -n GITHUB_API_CSMA_SH_LOADED ]] && return 0 / GITHUB_API_CSMA_SH_LOADED=1) was left behind when PR fullsend-ai#3182 inlined the CSMA library into post-prioritize scripts. Since these scripts are executed directly (not sourced) under set -euo pipefail, the bare return 0 crashes with "return: can only return from a function or sourced script" (exit 2) whenever the env var is set.

Remove the two-line guard (lines 21-22). The file is still sourced by post-prioritize.sh and post-prioritize-test.sh, so the guard is unnecessary — each script sources it exactly once.

All 7 post-prioritize tests pass. Go scaffold tests have pre-existing failures on main unrelated to this change.

Note: pre-commit could not run shellcheck in the sandbox (network restriction). Manual verification required.


Closes #174

Post-script verification

  • Branch is not main/master (agent/174-remove-csma-load-guard)
  • Secret scan passed (gitleaks — a2afa8b179a351845733c680b839a39a82304db6..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

…pi-csma.sh

The library-style load guard ([[ -n GITHUB_API_CSMA_SH_LOADED ]] &&
return 0 / GITHUB_API_CSMA_SH_LOADED=1) was left behind when PR fullsend-ai#3182
inlined the CSMA library into post-prioritize scripts. Since these
scripts are executed directly (not sourced) under set -euo pipefail,
the bare return 0 crashes with "return: can only return from a
function or sourced script" (exit 2) whenever the env var is set.

Remove the two-line guard (lines 21-22). The file is still sourced
by post-prioritize.sh and post-prioritize-test.sh, so the guard is
unnecessary — each script sources it exactly once.

All 7 post-prioritize tests pass. Go scaffold tests have pre-existing
failures on main unrelated to this change.

Note: pre-commit could not run shellcheck in the sandbox (network
restriction). Manual verification required.

Closes #174
@github-actions

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 12, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 3:34 AM UTC · Ended 3:42 AM UTC
Commit: a2afa8b · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review — PR #298

Verdict: Approve ✅

Summary

This PR removes the library-style load guard ([[ -n "${GITHUB_API_CSMA_SH_LOADED:-}" ]] && return 0 / GITHUB_API_CSMA_SH_LOADED=1) from internal/scaffold/fullsend-repo/scripts/lib/github-api-csma.sh. The change is a clean 3-line deletion with zero additions.

Analysis

Correctness: The guard removal is safe and correct. The library is sourced by exactly two consumers (post-prioritize.sh line 17, post-prioritize-test.sh line 269), neither of which sources it more than once. The functions defined in the library are idempotent — re-sourcing would simply redefine them identically, which is harmless. No remaining references to GITHUB_API_CSMA_SH_LOADED exist in the codebase outside the removed lines. The guard itself was the actual hazard: if the environment variable leaked from a parent process, all function definitions would be skipped, causing undefined-function failures at runtime.

Security: No security surface area. Pure deletion of two code lines and a blank line. No injection patterns, permission changes, or sensitive data concerns.

Intent & coherence: The PR traces to authorized work (issue #174, labeled type/bug and ready-to-code). Issue #174 was auto-generated from an upstream repo where the CSMA library was inlined into consuming scripts; in this fork, the guard exists only in the library file, so the PR correctly targets the sole location where the guard resides. The scope is appropriate — a minimal bug fix with no scope creep.

Style: The post-deletion file structure is consistent with other library files in the same directory. No excess blank lines or formatting issues.

Documentation: The removed variable (GITHUB_API_CSMA_SH_LOADED) was an internal implementation detail never referenced in any documentation. No doc staleness introduced.

Reviewed dimensions

Dimension Result
Correctness ✅ No findings
Security ✅ No findings
Intent & coherence ✅ No findings
Style & conventions ✅ No findings
Documentation currency ✅ No findings
Cross-repo contracts ⏭ Skipped (no exported interfaces modified)

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jul 12, 2026
@guyoron1 guyoron1 closed this Jul 12, 2026
@guyoron1
guyoron1 deleted the agent/174-remove-csma-load-guard branch July 12, 2026 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ARCHIVED] Original issue 174

1 participant