Skip to content

Commit 1aa62f4

Browse files
authored
Remove the silent two-library default from trigger-add-submodules.sh (#73)
* remove the silent two-library default from trigger-add-submodules.sh * fix the coderabbitai review * fix the first reviewer comments * fix the second reviewer comments
1 parent 01fa10d commit 1aa62f4

13 files changed

Lines changed: 488 additions & 65 deletions

.github/workflows/assets/lib.sh

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22
# Shared shell library for add-submodules and start-translation workflows.
33
# Source env.sh before lib.sh so ORG, MODULE_ORG, BOT_NAME, BOT_EMAIL, BOOST_ORG, MASTER_BRANCH,
44
# LOCAL_BRANCH_PREFIX, TRANSLATION_BRANCH_PREFIX, WEBLATE_ENDPOINT_PATH, and TRANSLATIONS_REPO
5-
# are set. Workflows also set GITHUB_TOKEN, LANG_CODES, and (for start-translation)
6-
# WEBLATE_URL / WEBLATE_TOKEN in the step env before sourcing.
5+
# are set. Workflows also set GITHUB_TOKEN (from secrets.SYNC_TOKEN — broad PAT),
6+
# LANG_CODES, and (for start-translation) WEBLATE_URL / WEBLATE_TOKEN in the step
7+
# env before sourcing. Some workflows set GH_TOKEN to the ephemeral Actions token
8+
# (e.g. heartbeat.yml for gh CLI); that is not the same privilege level as GITHUB_TOKEN.
79
# Call validate_secrets (or validate_secrets weblate) after sourcing env.sh and lib.sh.
810
#
911
# Per-submodule batch return convention (see docs/ARCHITECTURE.md §6):
@@ -576,7 +578,7 @@ validate_secrets() {
576578
local require_weblate=0
577579
[[ "${1:-}" == "weblate" ]] && require_weblate=1
578580

579-
_require_nonempty GITHUB_TOKEN "SYNC_TOKEN secret is not set."
581+
_require_nonempty GITHUB_TOKEN "SYNC_TOKEN secret is not set (mapped to GITHUB_TOKEN in workflow env)."
580582
if [[ -n "${LANG_CODE:-}" ]]; then
581583
validate_lang_codes "$LANG_CODE"
582584
elif [[ -n "${LANG_CODES:-}" ]]; then

.github/workflows/heartbeat.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ jobs:
3030

3131
- name: Check the last successful scheduled sync
3232
env:
33+
# Ephemeral Actions token for gh CLI — not the SYNC_TOKEN PAT in GITHUB_TOKEN elsewhere.
3334
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
3435
run: |
3536
set -euo pipefail

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,17 @@ are a separate namespace — see [README](README.md#releases) and
3636
per-step verification and create-tag coverage.
3737
- [endpoint-contract.md](docs/endpoint-contract.md) Outbound Weblate section:
3838
request schema is now the source of truth for payload fields.
39+
- `scripts/trigger-add-submodules.sh` now requires `--submodules`; the
40+
`unordered, json` script default is removed so the operator script cannot
41+
silently diverge from raw API auto-discovery.
42+
- Bats coverage for `scripts/trigger-dispatch-common.sh`, `trigger-add-submodules.sh`,
43+
and `trigger-start-translation.sh`.
3944

4045
### Fixed
4146

47+
- `build_dispatch_json` in `trigger-dispatch-common.sh`: pass key/value pairs
48+
positionally via jq `--args` and `$ARGS.positional` instead of building an
49+
intermediate array.
4250
- Clone and finalize steps now fail fast: **`clone_repo`**, **`sync_translations_branch`**,
4351
and **`finalize_translations_*`** propagate non-zero status instead of continuing with
4452
incomplete state.

README.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,10 @@ These wrappers use only **`exit 0`** (success, including **`--help`**) and **`ex
192192
Copy **`.env.example`** to **`.env`** and set **`GH_TOKEN`** (or **`GITHUB_TOKEN`**)
193193
with permission to call **`POST /repos/{owner}/{repo}/dispatches`** on the target
194194
repo. The workflows still use GitHub **secrets** and **variables** on the server as
195-
documented below.
195+
documented below. **Workflow jobs** map tokens differently: **`SYNC_TOKEN`** (broad
196+
PAT) is set as **`GITHUB_TOKEN`**; the ephemeral Actions token used by **`gh`** (e.g.
197+
in **`heartbeat.yml`**) is set as **`GH_TOKEN`**. Those names are interchangeable
198+
only in the local trigger scripts, not across client and workflow contexts.
196199

197200
---
198201

docs/GETTING-STARTED.md

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ linked README sections for scope, format, and which workflows consume each value
6666

6767
## 1. Local trigger setup (optional)
6868

69-
To fire dispatches from a clone of this repo instead of the GitHub API or UI:
69+
To fire dispatches from a clone of this repo instead of the GitHub API
70+
(`POST …/dispatches` or `gh api`):
7071

7172
```bash
7273
cp .env.example .env # set GH_TOKEN (GITHUB_TOKEN is also accepted)
@@ -75,6 +76,10 @@ cp .env.example .env # set GH_TOKEN (GITHUB_TOKEN is also accepted)
7576
- **`GH_TOKEN`** is **client-side only** — permission to call
7677
`POST /repos/{owner}/{repo}/dispatches`. Workflows still use the GitHub
7778
**secrets** from step 0 on the server.
79+
- In **workflow jobs**, naming is inverted relative to the client scripts:
80+
the broad **`SYNC_TOKEN`** PAT is exported as **`GITHUB_TOKEN`**, while the
81+
ephemeral Actions token (e.g. for `gh` in **`heartbeat.yml`**) is exported as
82+
**`GH_TOKEN`**. The two names are interchangeable only on the client side.
7883
- Requires **curl** and **jq** or Python 3.
7984

8085
See [README § Scripts](../README.md#scripts-local-repository_dispatch) for script
@@ -117,14 +122,12 @@ scripts/trigger-add-submodules.sh \
117122
```
118123

119124
- Omit **`--lang-codes`** to use repository variable **`LANG_CODES`**.
120-
- Omit **`--submodules`** to use the script default **`DEFAULT_SUBMODULES`**
121-
(`unordered, json`; see
122-
[trigger-add-submodules.sh](../scripts/trigger-add-submodules.sh#L40)) — **not**
123-
auto-discovery from **`boostorg/boost`**. The script substitutes that default
124-
before building the payload
125-
([`SUBMODULES` assignment](../scripts/trigger-add-submodules.sh#L99)), so `client_payload.submodules`
126-
is never omitted. Full discovery runs only when the workflow receives a dispatch
127-
**without** a `submodules` field (raw API / GitHub UI).
125+
- **`--submodules` is required** when using this script (comma-separated library
126+
names). To process all Boost libraries, omit `submodules` from a **raw**
127+
`repository_dispatch` payload sent via the GitHub API (for example
128+
`POST …/dispatches` or `gh api repos/{owner}/{repo}/dispatches`) — the workflow
129+
then auto-discovers the full list from **`boostorg/boost`** `.gitmodules` via
130+
**`resolve_add_submodules_names`**.
128131

129132
### Expected outcome
130133

@@ -238,7 +241,7 @@ Runs automatically **daily at 00:00 UTC** (`0 0 * * *`) or on manual dispatch.
238241
| Item | Value |
239242
| -------- | --------------------------------------------------------------------- |
240243
| Workflow | [`.github/workflows/sync-translation.yml`](../.github/workflows/sync-translation.yml) |
241-
| Script | **None** — use the dispatches API, GitHub UI, or the curl example below |
244+
| Script | **None** — use the dispatches API or the curl example below |
242245
| Trigger | `repository_dispatch` with `event_type: sync-translation` (no `client_payload`) |
243246

244247
### Trigger (JSON)

docs/endpoint-contract.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ submodule pointer updates, and Boost release refs in `client_payload.version`.
101101
| Workflow | `.github/workflows/add-submodules.yml` |
102102
| Body shape | `{"event_type":"add-submodules","client_payload":{...}}` |
103103
| `client_payload` | All optional: `version`, `submodules` (list-like string), `lang_codes` (comma-separated). See [README](../README.md). |
104-
| Script | `scripts/trigger-add-submodules.sh` builds JSON with `jq` or Python; omits empty optional fields. Both trigger scripts source shared [`scripts/trigger-dispatch-common.sh`](../scripts/trigger-dispatch-common.sh). |
104+
| Script | `scripts/trigger-add-submodules.sh` builds JSON with `jq` or Python; omits empty optional fields; **`--submodules` is required** on the script even though the API field is optional. Both trigger scripts source shared [`scripts/trigger-dispatch-common.sh`](../scripts/trigger-dispatch-common.sh). |
105105

106106
### `event_type: start-translation`
107107

scripts/trigger-add-submodules.sh

100644100755
Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
#
1313
# Usage:
1414
# scripts/trigger-add-submodules.sh [--repo OWNER/NAME] [--token PAT] \
15-
# [--version REF] [--submodules 'a, b'] [--lang-codes zh_Hans,ja]
15+
# --submodules 'a, b' [--version REF] [--lang-codes zh_Hans,ja]
1616
#
1717
# If --repo is omitted: GITHUB_REPOSITORY, then git origin, then DEFAULT_REPO below.
1818

@@ -34,18 +34,13 @@ source "$_ASSETS_DIR/lib.sh"
3434
source "$_REPO_ROOT/scripts/trigger-dispatch-common.sh"
3535
unset _REPO_ROOT _ASSETS_DIR
3636

37-
# ---------------------------------------------------------------------------
38-
# Typical run — edit these. CLI flags override (except --token uses env/PAT).
39-
# ---------------------------------------------------------------------------
40-
DEFAULT_SUBMODULES="unordered, json"
41-
4237
usage() {
4338
cat <<'EOF'
4439
Trigger add-submodules.yml via repository_dispatch (POST .../dispatches).
4540
4641
Usage:
4742
scripts/trigger-add-submodules.sh [--repo OWNER/NAME] [--token PAT] \
48-
[--version REF] [--submodules 'a, b'] [--lang-codes zh_Hans,ja]
43+
--submodules 'a, b' [--version REF] [--lang-codes zh_Hans,ja]
4944
5045
Requires: curl; jq or Python 3 (python3 / python)
5146
Auth: .env (GH_TOKEN), GH_TOKEN / GITHUB_TOKEN in env, or --token (needs repo scope on the target).
@@ -54,7 +49,7 @@ Options:
5449
--repo OWNER/REPO Target repository (default: GITHUB_REPOSITORY, then origin, then DEFAULT_REPO)
5550
--token PAT GitHub token
5651
--version REF Boost ref; default DEFAULT_VERSION in script
57-
--submodules LIST default DEFAULT_SUBMODULES in script
52+
--submodules LIST comma-separated library names (required)
5853
--lang-codes CSV optional; omit → workflow uses repo vars.LANG_CODES
5954
EOF
6055
}
@@ -65,18 +60,22 @@ VERSION=""
6560
SUBMODULES=""
6661
LANG_CODES=""
6762

63+
shift_pair_into() {
64+
printf -v "$1" '%s' "${2:-}"
65+
}
66+
6867
while [[ $# -gt 0 ]]; do
6968
case "$1" in
7069
--repo)
71-
REPO="${2:-}"; shift 2 || exit 1 ;;
70+
shift_pair_into REPO "$2"; shift 2 || exit 1 ;;
7271
--token)
73-
TOKEN="${2:-}"; shift 2 || exit 1 ;;
72+
shift_pair_into TOKEN "$2"; shift 2 || exit 1 ;;
7473
--version)
75-
VERSION="${2:-}"; shift 2 || exit 1 ;;
74+
shift_pair_into VERSION "$2"; shift 2 || exit 1 ;;
7675
--submodules)
77-
SUBMODULES="${2:-}"; shift 2 || exit 1 ;;
76+
shift_pair_into SUBMODULES "$2"; shift 2 || exit 1 ;;
7877
--lang-codes)
79-
LANG_CODES="${2:-}"; shift 2 || exit 1 ;;
78+
shift_pair_into LANG_CODES "$2"; shift 2 || exit 1 ;;
8079
-h|--help)
8180
usage; exit 0 ;;
8281
*)
@@ -86,6 +85,16 @@ while [[ $# -gt 0 ]]; do
8685
esac
8786
done
8887

88+
# Trim leading/trailing whitespace; reject empty or whitespace-only values.
89+
SUBMODULES="${SUBMODULES#"${SUBMODULES%%[![:space:]]*}"}"
90+
SUBMODULES="${SUBMODULES%"${SUBMODULES##*[![:space:]]}"}"
91+
92+
if [[ -z "$SUBMODULES" ]]; then
93+
echo "error: --submodules is required (comma-separated library names)" >&2
94+
usage >&2
95+
exit 1
96+
fi
97+
8998
require_curl || exit 1
9099

91100
TOKEN="$(resolve_trigger_token "$TOKEN")" || exit 1
@@ -96,7 +105,6 @@ REPO="$(resolve_trigger_repo "$REPO")" || {
96105
}
97106

98107
VERSION="${VERSION:-$DEFAULT_VERSION}"
99-
SUBMODULES="${SUBMODULES:-$DEFAULT_SUBMODULES}"
100108

101109
validate_event_type "$EVENT_ADD_SUBMODULES"
102110

scripts/trigger-dispatch-common.sh

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ DEFAULT_REPO="cppalliance/boost-docs-translation"
88
DEFAULT_VERSION="boost-1.90.0"
99

1010
infer_repo_from_git() {
11-
local url root o r
12-
root="$(git rev-parse --show-toplevel 2>/dev/null)" || return 1
13-
url="$(git -C "$root" remote get-url origin 2>/dev/null)" || return 1
11+
local url="${1:-}" root o r
12+
if [[ -z "$url" ]]; then
13+
root="$(git rev-parse --show-toplevel 2>/dev/null)" || return 1
14+
url="$(git -C "$root" remote get-url origin 2>/dev/null)" || return 1
15+
fi
1416
if [[ "$url" =~ github\.com[:/]([^/]+)/([^[:space:]]+) ]]; then
1517
o="${BASH_REMATCH[1]}"
1618
r="${BASH_REMATCH[2]}"
@@ -44,6 +46,9 @@ resolve_trigger_repo() {
4446
return 1
4547
}
4648

49+
# Client-side dispatch auth: prefers GH_TOKEN, then GITHUB_TOKEN. Workflow jobs use a
50+
# different mapping — SYNC_TOKEN PAT in GITHUB_TOKEN; ephemeral Actions token in GH_TOKEN
51+
# (e.g. heartbeat.yml). The names are not interchangeable across those two contexts.
4752
resolve_trigger_token() {
4853
local explicit="${1:-}"
4954
local token="${explicit:-${GH_TOKEN:-${GITHUB_TOKEN:-}}}"
@@ -67,26 +72,21 @@ build_dispatch_json() {
6772
local event_type="$1"
6873
shift
6974
if command -v jq >/dev/null 2>&1; then
70-
local json_pairs='[]' key val
71-
while [[ $# -gt 0 ]]; do
72-
key="$1"
73-
val="$2"
74-
shift 2
75-
json_pairs="$(jq -n --argjson arr "$json_pairs" --arg k "$key" --arg v "$val" \
76-
'$arr + [{key: $k, value: $v}]')"
77-
done
78-
jq -n --arg event_type "$event_type" --argjson pairs "$json_pairs" \
75+
jq -n --arg event_type "$event_type" --args \
7976
'{
8077
event_type: $event_type,
8178
client_payload: (
82-
{}
83-
| reduce pairs[] as $p (
84-
.;
85-
if ($p.value | length) > 0 then . + {($p.key): $p.value} else . end
86-
)
79+
reduce range(0; $ARGS.positional | length; 2) as $i (
80+
{};
81+
if ($ARGS.positional[$i + 1] | length) > 0
82+
then . + {($ARGS.positional[$i]): $ARGS.positional[$i + 1]}
83+
else .
84+
end
85+
)
8786
)
88-
}'
89-
return 0
87+
}' \
88+
"$@"
89+
return $?
9090
fi
9191
local py=""
9292
command -v python3 >/dev/null 2>&1 && py="python3"
@@ -102,7 +102,7 @@ for i in range(0,len(pairs),2):
102102
d[k]=v
103103
print(json.dumps({"event_type":et,"client_payload":d}))' \
104104
"$event_type" "$@"
105-
return 0
105+
return $?
106106
fi
107107
return 1
108108
}

0 commit comments

Comments
 (0)