Skip to content

fix: avoid associative arrays in authorize-partner-secret.sh for bash 3.2#33

Open
Snakefin wants to merge 2 commits into
mainfrom
fix/authorize-partner-secret-bash32
Open

fix: avoid associative arrays in authorize-partner-secret.sh for bash 3.2#33
Snakefin wants to merge 2 commits into
mainfrom
fix/authorize-partner-secret-bash32

Conversation

@Snakefin

Copy link
Copy Markdown

Summary

  • macOS ships bash 3.2 (last GPLv2 release), which doesn't support declare -A associative arrays
  • Under set -u, the [nl]=... array keys were arithmetic-evaluated as unset variables, causing line 21: nl: unbound variable before the script could even run
  • Replaced the associative array with a case statement for the market → repo lookup — same behavior, portable across bash 3.2 (macOS default) and bash 4+ (CI runners)

Test plan

  • bash -n scripts/authorize-partner-secret.sh — syntax OK
  • Ran the script locally on macOS's stock bash 3.2 with a test partner/market — confirmed it resolves nlsilverfin/nl_market and proceeds past the previous failure point

… 3.2

macOS ships bash 3.2 (last GPLv2 release), which lacks declare -A. Under
set -u, the [nl]=... array keys were arithmetic-evaluated as unset
variables, failing with "nl: unbound variable" before the script could
run. Swap the associative array for a case statement so the script
works on macOS's default bash as well as bash 4+ CI runners.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0392aa4d-e257-4514-bf49-4296ab9f408e

📥 Commits

Reviewing files that changed from the base of the PR and between f801bfd and e5590cc.

📒 Files selected for processing (2)
  • README.md
  • scripts/authorize-partner-secret.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/authorize-partner-secret.sh

Walkthrough

The authorization script now resolves market codes to repositories and checks GitHub CLI authentication before requesting the partner API key. README guidance documents the preflight check, market mapping, and early-exit behavior.

Changes

Partner secret authorization

Layer / File(s) Summary
Resolve market repository and validate authentication
scripts/authorize-partner-secret.sh, README.md
Adds market-to-repository resolution, runs gh auth status before sensitive prompts, and documents the updated authorization workflow.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description omits required template sections like Fixes #, Type of change, and Checklist, so it is largely incomplete. Add the missing template sections, including the issue link, change type, and checklist items, and keep the summary/test plan.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: replacing associative arrays in the partner secret script for Bash 3.2 compatibility.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/authorize-partner-secret-bash32

Comment @coderabbitai help to get the list of available commands.

Two real hiccups came up authorizing partners on staging: macOS's bash
3.2 (fixed in the parent commit) and a stale/expired gh session that
only surfaced as a 401 at the very last step, after already going
through the staging login flow. Check gh auth status up front instead
and fail fast with the fix steps, and document checking it before
starting in the README.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants