From 624869c99b6b243eecb639516447b3d3c70dcf4b Mon Sep 17 00:00:00 2001 From: Justin <159104807+Justin24506@users.noreply.github.com> Date: Wed, 11 Nov 2026 22:14:49 +0530 Subject: [PATCH 1/2] feat(ci): add a pre-release workflow --- .github/workflows/pre-release.yml | 198 ++++++++++++++++++++++++++++++ .github/workflows/release.yml | 13 +- AGENTS.md | 4 +- 3 files changed, 212 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/pre-release.yml diff --git a/.github/workflows/pre-release.yml b/.github/workflows/pre-release.yml new file mode 100644 index 0000000..d784a99 --- /dev/null +++ b/.github/workflows/pre-release.yml @@ -0,0 +1,198 @@ +# Pre-release: pushing a pre-release v* tag issues a pre-release +# automatically. +# +# git tag -a v0.2.0-rc.1 -m "TetherKit 0.2.0 rc 1" && git push origin v0.2.0-rc.1 +# +# Semver puts the pre-release part after a hyphen, so the trigger below is +# "tag contains a hyphen" (v*-*). release.yml excludes exactly those tags, +# so a tag push starts one of the two workflows, never both. +# +# Two deliberate differences from release.yml: +# +# 1. The Release is created with --prerelease. GitHub never serves a +# marked pre-release from the releases/latest endpoint, and that +# endpoint is what UpdateChecker.swift queries, so the app's Check +# for Updates will not offer a pre-release, and the Latest badge +# stays on the stable release. +# +# 2. There is no update-tap job. The tap is what users `brew install` +# from, and it must point at stable versions only; a pre-release +# tag never drives it. The tap moves when the stable tag is cut +# from the same base version, via release.yml's update-tap job. +# +# Like release.yml, this workflow builds and unit-tests on its own (ctest +# plus the GUI packaging), so a tag cut after a main-branch regression is +# still caught at release time. + +name: Pre-release + +on: + push: + # A hyphen is semver's pre-release marker (v0.2.0-rc.1, v0.2.0-beta.2). + # release.yml excludes hyphenated tags, so exactly one of the two + # release workflows runs per tag push. + tags: ["v*-*"] + +# Creating the Release and uploading assets need write access. +permissions: + contents: write + +jobs: + prerelease: + name: Build and publish ${{ github.ref_name }} (pre-release) + runs-on: macos-26 + + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + # Same check as release.yml: the tag and CMakeLists' VERSION must + # agree, otherwise `tetherkit-cli --version` will not match the + # release. A pre-release tag carries a suffix (v0.2.0-rc.1) that + # CMake's project() VERSION cannot hold (components are numeric + # only), so the comparison uses the base version: everything before + # the first hyphen. The suffix stays tag metadata only; the binaries + # it produces still report the base version. + - name: Verify the tag matches the CMakeLists VERSION + run: | + tag_ver="${GITHUB_REF_NAME#v}" + base_ver="${tag_ver%%-*}" + cmake_ver=$(grep -E '^[[:space:]]+VERSION[[:space:]]+[0-9]' CMakeLists.txt \ + | head -1 | awk '{print $2}') + echo "tag=${tag_ver} base=${base_ver} CMakeLists.txt=${cmake_ver}" + if [ "$base_ver" != "$cmake_ver" ]; then + echo "::error::pre-release tag (${tag_ver}) base version (${base_ver}) does not match CMakeLists.txt VERSION (${cmake_ver})" + exit 1 + fi + + - name: Install libusb + run: brew install libusb + + - name: Build and test + run: | + cmake -S . -B build \ + -DCMAKE_BUILD_TYPE=Release \ + -DTETHERKIT_WARNINGS_AS_ERRORS=ON + cmake --build build -j"$(sysctl -n hw.ncpu)" + ctest --test-dir build --output-on-failure + + - name: Build the GUI + run: cmake --build build --target gui + + - name: Package the command-line binary + id: pkg + run: | + name="tetherkit-cli-${GITHUB_REF_NAME}-macos-arm64.tar.gz" + tar -C build/bin -czf "$name" tetherkit-cli + { + echo "name=${name}" + echo "sha256=$(shasum -a 256 "$name" | awk '{print $1}')" + } >> "$GITHUB_OUTPUT" + + - name: Package TetherKit.app + id: app + run: | + name="TetherKit-${GITHUB_REF_NAME}-macos-arm64.zip" + # ditto preserves the bundle structure and extended attributes; it is + # the standard way to package a .app. + ditto -c -k --keepParent dist/TetherKit.app "$name" + { + echo "name=${name}" + echo "sha256=$(shasum -a 256 "$name" | awk '{print $1}')" + } >> "$GITHUB_OUTPUT" + + # Unlike release.yml, no formula consumes this sha256: the tap is not + # touched by pre-releases. It still goes into the notes so anyone + # building the same tag from source can verify their tarball. + - name: Compute the source tarball sha256 + id: src + run: | + url="https://github.com/${GITHUB_REPOSITORY}/archive/refs/tags/${GITHUB_REF_NAME}.tar.gz" + # A freshly pushed tag can take a few seconds before GitHub generates + # the archive; retry a few times. + for i in 1 2 3 4 5; do + if curl -fsSL "$url" -o src.tar.gz; then break; fi + echo "attempt ${i}: source archive not fetched yet, retrying in 5s…" + sleep 5 + done + test -s src.tar.gz || { echo "::error::could not fetch the source archive ${url}"; exit 1; } + echo "sha256=$(shasum -a 256 src.tar.gz | awk '{print $1}')" >> "$GITHUB_OUTPUT" + + # The quoted heredoc keeps the shell from expanding anything; ${{ }} is + # substituted by Actions before the shell runs, so the step outputs above + # still resolve. + # + # No Homebrew install section: the tap does not carry this version, so + # pointing the reader at it would be a dead end. The assets and a source + # build of the same tag are the two ways to get the binaries. + - name: Assemble the release notes + run: | + cat > notes.md <<'EOF' + ## Pre-release + + This is a **pre-release**, cut from a development tag for testing + ahead of the stable release. Expect rough edges; when you report + one, include the tag name you are running. + + The Homebrew tap is **not** updated for pre-releases. It only ever + points at stable versions, so `brew install` / `brew upgrade` will + not offer this build; the tap moves when the stable release is cut + from this same base version. + + ## Install + + There is no Homebrew route (see above), so the choices are the + assets below or a source build of the same tag. The assets are + **unsigned**, so Gatekeeper quarantines them after a browser + download; remove the quarantine manually to run them (locally + built artifacts do not carry the flag): + + ```bash + xattr -d com.apple.quarantine tetherkit-cli # CLI + xattr -dr com.apple.quarantine TetherKit.app # GUI (recursive) + ``` + + ## Command line + + ```bash + tetherkit-cli --list # list recognized devices, no root needed + sudo tetherkit-cli # start the driver, needs root + ``` + + ## Requirements + + - CLI: macOS 13.3+; GUI: macOS 14+ (all assets are arm64-only) + - The CLI needs libusb at runtime: `brew install libusb`. The .app + embeds its own copy, so no extra install for the GUI. + + ## Checksums + + | File | SHA-256 | + |---|---| + | Source tarball (for a source build of this tag) | `${{ steps.src.outputs.sha256 }}` | + | `${{ steps.pkg.outputs.name }}` | `${{ steps.pkg.outputs.sha256 }}` | + | `${{ steps.app.outputs.name }}` | `${{ steps.app.outputs.sha256 }}` | + EOF + + # --prerelease: GitHub marks the Release as a pre-release. A marked + # pre-release is never served by releases/latest (the endpoint + # UpdateChecker.swift queries), so the app's Check for Updates will + # not offer this build; it also keeps the Latest badge on the stable + # release. + - name: Create the Release + env: + GH_TOKEN: ${{ github.token }} + run: | + gh release create "${GITHUB_REF_NAME}" \ + --title "TetherKit ${GITHUB_REF_NAME}" \ + --prerelease \ + --notes-file notes.md \ + --generate-notes \ + --verify-tag \ + "${{ steps.pkg.outputs.name }}" \ + "${{ steps.app.outputs.name }}" + + # No update-tap job, deliberately: pre-releases never touch the tap. The + # tap is what users `brew install` from, and it must point at stable + # versions only -- that is also what keeps a pre-release out of every + # user's `brew upgrade`. release.yml's update-tap job moves the tap when + # the stable tag cut from the same base version lands. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4d44bf6..9550a85 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,7 +1,11 @@ -# Release: pushing a v* tag issues a release automatically. +# Release: pushing a stable v* tag issues a release automatically. # # git tag -a v0.2.0 -m "TetherKit 0.2.0" && git push origin v0.2.0 # +# Pre-release tags (a hyphen marks the semver pre-release part, e.g. +# v0.2.0-rc.1) are excluded below and handled by pre-release.yml, which +# publishes a marked pre-release and never touches the Homebrew tap. +# # This workflow builds and unit-tests on its own (ctest plus the GUI # packaging), so a tag cut after a main-branch regression is still caught at # release time. It deliberately does NOT run the whole matrix - no Swift @@ -12,7 +16,12 @@ name: Release on: push: - tags: ["v*"] + # Stable tags only. The exclusion is what keeps a pre-release tag + # (v*-*: the hyphen is semver's pre-release marker) from starting this + # workflow -- otherwise it would publish a stable Release AND bump the + # tap. Hyphenated tags belong to pre-release.yml, so exactly one of the + # two workflows runs per tag push. + tags: ["v*", "!v*-*"] # Creating the Release and uploading assets need write access. permissions: diff --git a/AGENTS.md b/AGENTS.md index c1d71a6..9564d2d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -268,11 +268,13 @@ Legend: `Done` `In progress` `Not started` | 62 | `docs(ci): document the Dependabot PR merge procedure` (this branch) | Done | The SHA pinning made Dependabot a weekly event, so the procedure for merging its PR is now part of the repo's operating instructions rather than tribal knowledge. It lives in .github/dependabot.yml, where a maintainer opening a bot PR will look, and it covers: merge-commit rather than squash or rebase (both leave the tip authored by Dependabot), because a squash or rebase commit is authored by Dependabot and loses write scope (the merge commit is authored by the human, so results keep publishing); the REST bypass command for the review gate, `gh api repos/justsrc/TetherKit/pulls//merge -X PUT -f merge_method=merge`, because `gh pr merge` refuses at its own pre-flight when the PR is BLOCKED (cli/cli#13388, still open) and suggests --auto or --admin; --admin only suppresses that client-side refusal -- no admin flag travels to the server (gh's mergePayload has no such field) and the 'Protect main' ruleset has no bypass actor at all, so nothing can override the required status checks. The merge succeeds because the 'Require review' ruleset lists the admin role as a bypass actor with bypass_mode pull_request, which engages server-side on any PR merge by the admin, REST or gh; and the reminder that the version-update PR is the ONLY human checkpoint a compromised action release would meet, since SHA pinning deliberately kills the Dependabot alert channel (row 60). **The review requirement needed no settings change**, and the reason is the non-obvious part: the 'Require review' ruleset already lists the admin role as a bypass actor with bypass_mode pull_request, so the human who merges the bot PR bypasses the review exactly as for their own PRs. Adding Dependabot to the bypass list would have been actively wrong -- it would let the bot merge its own PR unreviewed, defeating the SHA pinning. That warning is carried in both dependabot.yml's step 4 and codeql.yml's header, so a maintainer meets it whichever of the two files they open. Also confirmed while writing it: no PR-triggered workflow (tests, lint, codeql) references a secret, so Dependabot's no-secrets rule is a non-issue for them. Committed and pushed as feat/ci-hardening | | 63 | `docs(ci): apply the second-review text corrections` (this branch) | Done | A second, line-by-line review of rows 60-62 and the three CI files. Verdict: no defect in the workflows themselves -- every change is comment, prose, or a PR body -- but nine places where the ledger described the mechanism inaccurately or claimed something untrue, one addition pointing at where a warning now lives, and one check that turned out to be correct only by accident. **Every claim in the review was verified against upstream source before being applied, not taken on trust**, and one of them turned out to be overstated: the review says `gh pr merge --admin` "only suppresses that client-side refusal", but gh's own help and source show it also skips the merge queue client-side (the help text: "To bypass a merge queue and merge directly, pass the `--admin` flag"); the wording keeps the server-side half, which is the part that matters, and the load-bearing finding stands either way -- gh's `mergePayload` carries no admin field, so nothing about privileges travels to the server, and the 'Protect main' ruleset has `bypass_actors: []` with `current_user_can_bypass: never`, so no admin can override the required status checks. What merges the PR is the 'Require review' ruleset, which lists the admin role as a bypass actor with `bypass_mode: pull_request`. Corrections applied: row 38's open-loop deploy-key instruction became a closed loop (the tap repo is since done, the formula copy is still outstanding, and the deploy key is superseded by row 60's one-token model, so an onboarding agent can no longer act on a credential that is no longer used); row 60's claim that SARIF post-filtering "requires an Enterprise-owned repository" was **wrong on the facts** (the action is public) and was rewritten as a decision rather than a constraint -- declined, because it is one more third-party action to trust and pin on the analysis path; row 61's three clause fixes were corrected to the two-layer failure path (upload-lib.ts warns and rethrows, the action's outer catch then calls `core.setFailed`, read at analyze-action.ts:451 at the pinned v4.39.3), to `getUploadValue`'s real contract (exactly always/failure-only/never, with legacy `true`/`false` inputs mapped onto them and an unknown value warning and defaulting to always), and to actionlint's real rule (a `branches`+`branches-ignore` **pair** on one event is rejected, exit 1, while paths filters combine with branches fine, exit 0 -- both probed on actionlint 1.7.12); rows 61 and 62 ended with "Nothing pushed" although their commits are on origin/feat/ci-hardening (350eb0e and c34783e), so both now read "Committed and pushed as feat/ci-hardening"; row 62's merge strategy now says merge-commit rather than squash **or rebase** (both leave the tip authored by Dependabot) and its `--admin` paragraph was replaced with the verified mechanism. The one behavior change: update-tap.yml's existing-PR check piped `gh pr list --jq '.[0].number'` into `grep -q .`, and gh prints a bare newline when nothing matches -- **the old form was not actually broken** (probed: BSD grep, which the runner uses, does not match `.` against a bare newline, so the old check took the right branch in both cases), but it worked only by that accident, so the check now tests for a non-empty number explicitly and is independent of the grep implementation; probed live against a branch with an open PR (yields the number) and one without (yields empty). The rewrite also changes one failure mode: under `bash -e` (GitHub's default for `run:` blocks) a failing `gh` call now ends the step at the check, because the assignment takes the non-zero status, where the old pipeline fell through to `gh pr create` -- the error surfaces at the check instead of at a later command. Delegation: two disjoint scopes went to iceman-free subagents (codeql.yml, dependabot.yml), each verified independently afterwards by diffing the comment-stripped payload against HEAD (byte-identical), so no YAML key, expression, or action pin moved; update-tap.yml was edited directly, because it is the secret-bearing file and row 52's workspace-guard rule keeps non-primary providers out of it. Also documented, not fixed: the tap token's `contents:write` also reaches the tap's tags and releases, which no ruleset governs (pitfall 26). Committed, not pushed | +| 64 | `feat(ci): add a pre-release workflow` (this branch) | Done | New `.github/workflows/pre-release.yml`: pushing a pre-release tag (`git tag -a v0.2.0-rc.1 ...`) publishes a marked pre-release. A step-for-step mirror of release.yml on purpose: same checkout pin (3d3c42e # v7.0.1), same verify/libusb/build+ctest/GUI/package/checksum steps, same GITHUB_REF_NAME env-var doctrine, no environment gate (release.yml's release job has none either -- the `release` environment guards only the tap) and no timeout. The ruling that a pre-release never touches the tap required TWO changes, not one: (a) the new workflow simply has no update-tap job and holds no tap secret (a closing comment block marks the deliberate absence where the job would sit), but that alone would have been worthless; (b) release.yml's `on.push.tags: ["v*"]` ALSO matched `v0.2.0-rc.1` (`*` crosses the hyphen), so pushing an rc tag would have fired BOTH workflows, and the stable one would have published a full Release AND bumped the tap anyway -- release.yml is now `tags: ["v*", "!v*-*"]`, the documented include-then-exclude form for tags (a negative pattern after a positive match excludes the ref, and a negative must be accompanied by at least one positive pattern), so exactly one of the two workflows runs per tag push: the hyphen is semver's pre-release marker and doubles as the routing bit, verified with an fnmatch probe (`v0.2.0-rc.1` matches `v*` and `v*-*`; `v0.2.0` matches only `v*`). Four deliberate differences from release.yml, each commented in the file: (1) `gh release create --prerelease`, with the reason recorded there: GitHub never serves a marked pre-release from `releases/latest`, the endpoint UpdateChecker.swift queries, so the app's Check for Updates can never offer an rc build and the Latest badge stays on the stable release; (2) the tag/version verify step compares only the BASE version (`base_ver="${tag_ver%%-*}"`, everything before the first hyphen), because CMake's `project(VERSION)` rejects pre-release suffixes -- probed locally: `project(Probe VERSION 0.2.0-rc.1)` errors with `VERSION "0.2.0-rc.1" format invalid.` -- so `v0.2.0-rc.1` requires CMakeLists.txt to say `0.2.0` and fails fast otherwise (operational consequence: bump VERSION before cutting an rc; the suffix stays tag metadata, the binaries still report the base version); (3) the release notes drop the Homebrew section entirely (the tap does not carry this version, so pointing readers at it is a dead end) and state plainly that the tap is not updated, keeping the quarantine/xattr instructions, CLI, requirements, and the checksum table; (4) one accuracy improvement over the stable notes: only the CLI asset needs `brew install libusb` -- the .app embeds its own copy via build-gui.sh's `embed_libusb` (`@loader_path`, verified in gui/Scripts/build-gui.sh), so the asset route needs no extra install for the GUI. Verification, all local: actionlint clean on both files (its shellcheck integration active), YAML parse of both, all 9 `run:` blocks per file pass `bash -n`, and comments ASCII-only (the single non-ASCII char is the ellipsis in the retry echo copied verbatim from release.yml's operational line, not a comment). Committed and pushed as feat/pre-release-workflow. | + ### Current state - **Tests**: all 23 ctest cases pass; on the GUI side `swift test` has 30 cases passing (TetherKitIPCTests 17 and TetherKitUIModelTests 13, including LocalizationTests and HelperConstantsTests). - **Build**: zero warnings under `-Werror`; the CLI, shared library, and GUI products all build. -- **CI**: legacy ci.yml split into tests.yml (five jobs migrated with their steps and checks unchanged, triggers PR + push main + Monday cron + ASan/UBSan), build.yml (manual trigger only, full build + artifacts) and lint.yml (clang-format gate + advisory clang-tidy); new codeql.yml (weekly + push main + every-PR C++ scanning), bench.yml (manual benchmarks) and dependabot.yml (weekly grouped action bumps); permissions: contents: read across tests/lint/build; Xcode selection filters beta; every external action is pinned to a full commit SHA with the release tag in a trailing comment (see ledger row 60 for why SHA rather than a major tag). +- **CI**: legacy ci.yml split into tests.yml (five jobs migrated with their steps and checks unchanged, triggers PR + push main + Monday cron + ASan/UBSan), build.yml (manual trigger only, full build + artifacts) and lint.yml (clang-format gate + advisory clang-tidy); new codeql.yml (weekly + push main + every-PR C++ scanning), bench.yml (manual benchmarks) and dependabot.yml (weekly grouped action bumps); permissions: contents: read across tests/lint/build; Xcode selection filters beta; every external action is pinned to a full commit SHA with the release tag in a trailing comment (see ledger row 60 for why SHA rather than a major tag); releases split by tag shape: release.yml takes stable `v*` tags only, and pre-release.yml takes hyphenated `v*-*` tags and never touches the tap (row 64). - **Runnable**: `--version` / `--help` / `--list` all work; starting without root prints a clear hint and returns exit code 1; `TetherKit.app` and `tetherkit-helper` both start after packaging; the privileged helper can be installed / uninstalled inside the App with one click (the install path has been exercised on real hardware); when the helper version does not match the App, the management row lights up "Update the helper" (it compares the **library** version, not Info.plist -- `brew upgrade` only replaces the .app, the copy installed under /Library does not follow; when the protocol revision did not change there was previously no prompt at all, and the only thing the user could notice was "the issue described as fixed in the release notes is still there"; the windows were screenshot-checked); English-only behavior is verified: CLI `--help` / `--list` / argument errors are correct, and switching the language in the GUI menu updates the main window and the App menu **immediately** (no restart required, screenshot-checked). - **Verified**: the USB side works on a real RNDIS device (enumeration, interface claim, RNDIS handshake), and this USB side **does not require root**; the feth private ABI, the two private BPF ioctls, and the core assumption that "a BPF `write()` lets the peer IP stack receive a frame" have all been confirmed on real hardware. See section 6. - **Benchmarked** (2026-07-28, Android USB tethering): end-to-end iperf3 measured **RX 324-327 Mbps / TX 234-241 Mbps** (USB 2.0 High Speed, theoretical ceiling 426 Mbps); the full GUI-to-real-device link works (plug in -> connect -> obtain a DHCP address -> bidirectional ping). See section 6.4 and [docs/BENCHMARKS.md](docs/BENCHMARKS.md). From f0cdbde2283ed34cf1ca068aa49b779d8ff265f9 Mon Sep 17 00:00:00 2001 From: Justin <159104807+Justin24506@users.noreply.github.com> Date: Wed, 11 Nov 2026 22:59:30 +0530 Subject: [PATCH 2/2] fix(ci): unbreak the ledger table, pass step outputs as env --- .github/workflows/pre-release.yml | 35 +++++++++++++++++++++++-------- .github/workflows/release.yml | 27 +++++++++++++++++------- AGENTS.md | 4 +++- 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/.github/workflows/pre-release.yml b/.github/workflows/pre-release.yml index d784a99..21976c6 100644 --- a/.github/workflows/pre-release.yml +++ b/.github/workflows/pre-release.yml @@ -3,11 +3,17 @@ # # git tag -a v0.2.0-rc.1 -m "TetherKit 0.2.0 rc 1" && git push origin v0.2.0-rc.1 # +# Cut rc tags only from current main: tag-filtered workflows are resolved +# from the TAGGED commit, not from main, so an rc tag on a commit older +# than this file runs the OLD release.yml -- its tags: ["v*"] matches every +# v* tag -- and would publish a stable Release and bump the tap. The v*-* / +# !v*-* split only guards tags cut from commits that carry it. +# # Semver puts the pre-release part after a hyphen, so the trigger below is # "tag contains a hyphen" (v*-*). release.yml excludes exactly those tags, # so a tag push starts one of the two workflows, never both. # -# Two deliberate differences from release.yml: +# Deliberate differences from release.yml: # # 1. The Release is created with --prerelease. GitHub never serves a # marked pre-release from the releases/latest endpoint, and that @@ -117,14 +123,21 @@ jobs: test -s src.tar.gz || { echo "::error::could not fetch the source archive ${url}"; exit 1; } echo "sha256=$(shasum -a 256 src.tar.gz | awk '{print $1}')" >> "$GITHUB_OUTPUT" - # The quoted heredoc keeps the shell from expanding anything; ${{ }} is - # substituted by Actions before the shell runs, so the step outputs above - # still resolve. + # The quoted heredoc keeps the shell from expanding anything, and + # nothing is interpolated into the script itself: the checksum values + # and asset names travel as env and are appended with printf, so their + # content is data, never shell text. # # No Homebrew install section: the tap does not carry this version, so # pointing the reader at it would be a dead end. The assets and a source # build of the same tag are the two ways to get the binaries. - name: Assemble the release notes + env: + SRC_SHA256: ${{ steps.src.outputs.sha256 }} + PKG_NAME: ${{ steps.pkg.outputs.name }} + PKG_SHA256: ${{ steps.pkg.outputs.sha256 }} + APP_NAME: ${{ steps.app.outputs.name }} + APP_SHA256: ${{ steps.app.outputs.sha256 }} run: | cat > notes.md <<'EOF' ## Pre-release @@ -168,10 +181,12 @@ jobs: | File | SHA-256 | |---|---| - | Source tarball (for a source build of this tag) | `${{ steps.src.outputs.sha256 }}` | - | `${{ steps.pkg.outputs.name }}` | `${{ steps.pkg.outputs.sha256 }}` | - | `${{ steps.app.outputs.name }}` | `${{ steps.app.outputs.sha256 }}` | EOF + { + printf "| %s | \`%s\` |\n" "Source tarball (for a source build of this tag)" "${SRC_SHA256}" + printf "| \`%s\` | \`%s\` |\n" "${PKG_NAME}" "${PKG_SHA256}" + printf "| \`%s\` | \`%s\` |\n" "${APP_NAME}" "${APP_SHA256}" + } >> notes.md # --prerelease: GitHub marks the Release as a pre-release. A marked # pre-release is never served by releases/latest (the endpoint @@ -181,6 +196,8 @@ jobs: - name: Create the Release env: GH_TOKEN: ${{ github.token }} + PKG_NAME: ${{ steps.pkg.outputs.name }} + APP_NAME: ${{ steps.app.outputs.name }} run: | gh release create "${GITHUB_REF_NAME}" \ --title "TetherKit ${GITHUB_REF_NAME}" \ @@ -188,8 +205,8 @@ jobs: --notes-file notes.md \ --generate-notes \ --verify-tag \ - "${{ steps.pkg.outputs.name }}" \ - "${{ steps.app.outputs.name }}" + "${PKG_NAME}" \ + "${APP_NAME}" # No update-tap job, deliberately: pre-releases never touch the tap. The # tap is what users `brew install` from, and it must point at stable diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9550a85..1b5ec06 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -102,10 +102,17 @@ jobs: test -s src.tar.gz || { echo "::error::could not fetch the source archive ${url}"; exit 1; } echo "sha256=$(shasum -a 256 src.tar.gz | awk '{print $1}')" >> "$GITHUB_OUTPUT" - # The quoted heredoc keeps the shell from expanding anything; ${{ }} is - # substituted by Actions before the shell runs, so the step outputs above - # still resolve. + # The quoted heredoc keeps the shell from expanding anything, and + # nothing is interpolated into the script itself: the checksum values + # and asset names travel as env and are appended with printf, so their + # content is data, never shell text. - name: Assemble the release notes + env: + SRC_SHA256: ${{ steps.src.outputs.sha256 }} + PKG_NAME: ${{ steps.pkg.outputs.name }} + PKG_SHA256: ${{ steps.pkg.outputs.sha256 }} + APP_NAME: ${{ steps.app.outputs.name }} + APP_SHA256: ${{ steps.app.outputs.sha256 }} run: | cat > notes.md <<'EOF' ## Install @@ -158,22 +165,26 @@ jobs: | File | SHA-256 | |---|---| - | Source tarball (used by the Homebrew formula) | `${{ steps.src.outputs.sha256 }}` | - | `${{ steps.pkg.outputs.name }}` | `${{ steps.pkg.outputs.sha256 }}` | - | `${{ steps.app.outputs.name }}` | `${{ steps.app.outputs.sha256 }}` | EOF + { + printf "| %s | \`%s\` |\n" "Source tarball (used by the Homebrew formula)" "${SRC_SHA256}" + printf "| \`%s\` | \`%s\` |\n" "${PKG_NAME}" "${PKG_SHA256}" + printf "| \`%s\` | \`%s\` |\n" "${APP_NAME}" "${APP_SHA256}" + } >> notes.md - name: Create the Release env: GH_TOKEN: ${{ github.token }} + PKG_NAME: ${{ steps.pkg.outputs.name }} + APP_NAME: ${{ steps.app.outputs.name }} run: | gh release create "${GITHUB_REF_NAME}" \ --title "TetherKit ${GITHUB_REF_NAME}" \ --notes-file notes.md \ --generate-notes \ --verify-tag \ - "${{ steps.pkg.outputs.name }}" \ - "${{ steps.app.outputs.name }}" + "${PKG_NAME}" \ + "${APP_NAME}" # After the Release is created, sync the Homebrew tap to the new version. # diff --git a/AGENTS.md b/AGENTS.md index 9564d2d..dc56468 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -267,8 +267,8 @@ Legend: `Done` `In progress` `Not started` | 61 | `fix(ci): keep the CodeQL upload alive on Dependabot merges` (this branch) | Done | A Dependabot PR merged with SQUASH produces a commit on main that is AUTHORED BY DEPENDABOT, and any workflow on such a commit gets a read-only GITHUB_TOKEN, so the SARIF upload 403s ("Resource not accessible by integration"). The upload library logs a warning and rethrows (upload-lib.ts's 403 handler); the action's outer catch then fails the step via `core.setFailed`, so the run goes red. The analyze step now sets the action's own `upload` input to `never` for a Dependabot-authored push and `always` otherwise, so the run stays green while the analysis still runs; only the publish is skipped. The action's `getUploadValue` returns exactly always/failure-only/never (legacy `true`/`false` inputs map onto them; unknown values warn and default to always), and the 'never' branch skips the upload call and returns normally (read out of the action source at the pinned v4.39.3, not just the docs). **The pull_request path is deliberately NOT guarded**, which corrects an earlier assumption in this session: code scanning always allows uploading results when the pull_request event triggers the run, because the results are scoped to that one PR, and upload-lib.ts carries an explicit pull_request branch for it. Guarding it would be a regression, and a regression with teeth: no upload means no result, and a required check that never reports blocks the PR forever. **Two wrong turns worth recording**: the input is `upload`, not `upload-sarif` -- that boolean belongs to OSV-Scanner, and copying it here would have been silently ignored; and both `paths-ignore` and a `branches`+`branches-ignore` pair were wrong -- actionlint rejects a `branches`+`branches-ignore` PAIR on one event (paths filters combine with branches fine -- probe: exit 0 -- but paths-ignore is inert for C++ manual builds), and a squash-merged bot commit lands on main, so a dependabot/** branch filter cannot match it anyway. Verification, local only: the expression's four event/author combinations evaluated by hand, the workflow parses, actionlint is clean, the input name and its accepted values read out of the action.yml at the pinned SHA, and getUploadValue plus the 'never' branch read in the action source. Committed and pushed as feat/ci-hardening | | 62 | `docs(ci): document the Dependabot PR merge procedure` (this branch) | Done | The SHA pinning made Dependabot a weekly event, so the procedure for merging its PR is now part of the repo's operating instructions rather than tribal knowledge. It lives in .github/dependabot.yml, where a maintainer opening a bot PR will look, and it covers: merge-commit rather than squash or rebase (both leave the tip authored by Dependabot), because a squash or rebase commit is authored by Dependabot and loses write scope (the merge commit is authored by the human, so results keep publishing); the REST bypass command for the review gate, `gh api repos/justsrc/TetherKit/pulls//merge -X PUT -f merge_method=merge`, because `gh pr merge` refuses at its own pre-flight when the PR is BLOCKED (cli/cli#13388, still open) and suggests --auto or --admin; --admin only suppresses that client-side refusal -- no admin flag travels to the server (gh's mergePayload has no such field) and the 'Protect main' ruleset has no bypass actor at all, so nothing can override the required status checks. The merge succeeds because the 'Require review' ruleset lists the admin role as a bypass actor with bypass_mode pull_request, which engages server-side on any PR merge by the admin, REST or gh; and the reminder that the version-update PR is the ONLY human checkpoint a compromised action release would meet, since SHA pinning deliberately kills the Dependabot alert channel (row 60). **The review requirement needed no settings change**, and the reason is the non-obvious part: the 'Require review' ruleset already lists the admin role as a bypass actor with bypass_mode pull_request, so the human who merges the bot PR bypasses the review exactly as for their own PRs. Adding Dependabot to the bypass list would have been actively wrong -- it would let the bot merge its own PR unreviewed, defeating the SHA pinning. That warning is carried in both dependabot.yml's step 4 and codeql.yml's header, so a maintainer meets it whichever of the two files they open. Also confirmed while writing it: no PR-triggered workflow (tests, lint, codeql) references a secret, so Dependabot's no-secrets rule is a non-issue for them. Committed and pushed as feat/ci-hardening | | 63 | `docs(ci): apply the second-review text corrections` (this branch) | Done | A second, line-by-line review of rows 60-62 and the three CI files. Verdict: no defect in the workflows themselves -- every change is comment, prose, or a PR body -- but nine places where the ledger described the mechanism inaccurately or claimed something untrue, one addition pointing at where a warning now lives, and one check that turned out to be correct only by accident. **Every claim in the review was verified against upstream source before being applied, not taken on trust**, and one of them turned out to be overstated: the review says `gh pr merge --admin` "only suppresses that client-side refusal", but gh's own help and source show it also skips the merge queue client-side (the help text: "To bypass a merge queue and merge directly, pass the `--admin` flag"); the wording keeps the server-side half, which is the part that matters, and the load-bearing finding stands either way -- gh's `mergePayload` carries no admin field, so nothing about privileges travels to the server, and the 'Protect main' ruleset has `bypass_actors: []` with `current_user_can_bypass: never`, so no admin can override the required status checks. What merges the PR is the 'Require review' ruleset, which lists the admin role as a bypass actor with `bypass_mode: pull_request`. Corrections applied: row 38's open-loop deploy-key instruction became a closed loop (the tap repo is since done, the formula copy is still outstanding, and the deploy key is superseded by row 60's one-token model, so an onboarding agent can no longer act on a credential that is no longer used); row 60's claim that SARIF post-filtering "requires an Enterprise-owned repository" was **wrong on the facts** (the action is public) and was rewritten as a decision rather than a constraint -- declined, because it is one more third-party action to trust and pin on the analysis path; row 61's three clause fixes were corrected to the two-layer failure path (upload-lib.ts warns and rethrows, the action's outer catch then calls `core.setFailed`, read at analyze-action.ts:451 at the pinned v4.39.3), to `getUploadValue`'s real contract (exactly always/failure-only/never, with legacy `true`/`false` inputs mapped onto them and an unknown value warning and defaulting to always), and to actionlint's real rule (a `branches`+`branches-ignore` **pair** on one event is rejected, exit 1, while paths filters combine with branches fine, exit 0 -- both probed on actionlint 1.7.12); rows 61 and 62 ended with "Nothing pushed" although their commits are on origin/feat/ci-hardening (350eb0e and c34783e), so both now read "Committed and pushed as feat/ci-hardening"; row 62's merge strategy now says merge-commit rather than squash **or rebase** (both leave the tip authored by Dependabot) and its `--admin` paragraph was replaced with the verified mechanism. The one behavior change: update-tap.yml's existing-PR check piped `gh pr list --jq '.[0].number'` into `grep -q .`, and gh prints a bare newline when nothing matches -- **the old form was not actually broken** (probed: BSD grep, which the runner uses, does not match `.` against a bare newline, so the old check took the right branch in both cases), but it worked only by that accident, so the check now tests for a non-empty number explicitly and is independent of the grep implementation; probed live against a branch with an open PR (yields the number) and one without (yields empty). The rewrite also changes one failure mode: under `bash -e` (GitHub's default for `run:` blocks) a failing `gh` call now ends the step at the check, because the assignment takes the non-zero status, where the old pipeline fell through to `gh pr create` -- the error surfaces at the check instead of at a later command. Delegation: two disjoint scopes went to iceman-free subagents (codeql.yml, dependabot.yml), each verified independently afterwards by diffing the comment-stripped payload against HEAD (byte-identical), so no YAML key, expression, or action pin moved; update-tap.yml was edited directly, because it is the secret-bearing file and row 52's workspace-guard rule keeps non-primary providers out of it. Also documented, not fixed: the tap token's `contents:write` also reaches the tap's tags and releases, which no ruleset governs (pitfall 26). Committed, not pushed | - | 64 | `feat(ci): add a pre-release workflow` (this branch) | Done | New `.github/workflows/pre-release.yml`: pushing a pre-release tag (`git tag -a v0.2.0-rc.1 ...`) publishes a marked pre-release. A step-for-step mirror of release.yml on purpose: same checkout pin (3d3c42e # v7.0.1), same verify/libusb/build+ctest/GUI/package/checksum steps, same GITHUB_REF_NAME env-var doctrine, no environment gate (release.yml's release job has none either -- the `release` environment guards only the tap) and no timeout. The ruling that a pre-release never touches the tap required TWO changes, not one: (a) the new workflow simply has no update-tap job and holds no tap secret (a closing comment block marks the deliberate absence where the job would sit), but that alone would have been worthless; (b) release.yml's `on.push.tags: ["v*"]` ALSO matched `v0.2.0-rc.1` (`*` crosses the hyphen), so pushing an rc tag would have fired BOTH workflows, and the stable one would have published a full Release AND bumped the tap anyway -- release.yml is now `tags: ["v*", "!v*-*"]`, the documented include-then-exclude form for tags (a negative pattern after a positive match excludes the ref, and a negative must be accompanied by at least one positive pattern), so exactly one of the two workflows runs per tag push: the hyphen is semver's pre-release marker and doubles as the routing bit, verified with an fnmatch probe (`v0.2.0-rc.1` matches `v*` and `v*-*`; `v0.2.0` matches only `v*`). Four deliberate differences from release.yml, each commented in the file: (1) `gh release create --prerelease`, with the reason recorded there: GitHub never serves a marked pre-release from `releases/latest`, the endpoint UpdateChecker.swift queries, so the app's Check for Updates can never offer an rc build and the Latest badge stays on the stable release; (2) the tag/version verify step compares only the BASE version (`base_ver="${tag_ver%%-*}"`, everything before the first hyphen), because CMake's `project(VERSION)` rejects pre-release suffixes -- probed locally: `project(Probe VERSION 0.2.0-rc.1)` errors with `VERSION "0.2.0-rc.1" format invalid.` -- so `v0.2.0-rc.1` requires CMakeLists.txt to say `0.2.0` and fails fast otherwise (operational consequence: bump VERSION before cutting an rc; the suffix stays tag metadata, the binaries still report the base version); (3) the release notes drop the Homebrew section entirely (the tap does not carry this version, so pointing readers at it is a dead end) and state plainly that the tap is not updated, keeping the quarantine/xattr instructions, CLI, requirements, and the checksum table; (4) one accuracy improvement over the stable notes: only the CLI asset needs `brew install libusb` -- the .app embeds its own copy via build-gui.sh's `embed_libusb` (`@loader_path`, verified in gui/Scripts/build-gui.sh), so the asset route needs no extra install for the GUI. Verification, all local: actionlint clean on both files (its shellcheck integration active), YAML parse of both, all 9 `run:` blocks per file pass `bash -n`, and comments ASCII-only (the single non-ASCII char is the ellipsis in the retry echo copied verbatim from release.yml's operational line, not a comment). Committed and pushed as feat/pre-release-workflow. | +| 65 | `fix(ci): unbreak the ledger table, pass step outputs as env` (this branch) | Done | Four review findings on row 64's work, all fixed in one commit. (1, must fix) Row 64's insertion anchored on `### Current state` and landed BELOW the blank line that already separated row 63 from that heading, so a blank line sat between rows 63 and 64 -- in GFM a blank line terminates a table, and row 64 rendered as a paragraph of pipes (the reviewer verified by rendering the region: one table of 64 rows, row 64 outside it). The blank line is deleted; rows 63 and 64 are contiguous and the blank line after row 64 still separates the table from the heading. Recorded as pitfall 29; the check is a structural scan of the table region (no blank line between the table header and the next heading, every row line starting with the pipe character). (2) Tagging an OLD commit bypasses the guard: on.push.tags workflows are resolved from the TAGGED commit, not from main, so an rc tag on a commit older than this change runs the OLD release.yml -- its tags: ["v*"] matches every v* tag -- and would publish a stable Release AND bump the tap anyway. Inherent to Actions and unfixable from the new file (old commits do not carry it), so it is an operating rule, stated next to the tag-cutting example in pre-release.yml's header: cut rc tags only from current main after this merges. (3) The new workflow cannot be exercised by PR CI (tag-only trigger; this fork has never run release.yml either -- zero runs, zero releases), so row 64's verification stays structural until a tag exists: after merge, cut one throwaway rc whose base matches CMakeLists.txt VERSION (v0.1.5-rc.1, or bump to 0.1.6 first) and confirm the Pre-release badge, no tap PR, Latest unchanged, assets and checksums present, and no stable Release run. (4) Step outputs were interpolated into run: text in BOTH files -- a row-44 doctrine violation, pre-existing in release.yml and copied verbatim into pre-release.yml. Both notes heredocs and both gh release create steps now take the five outputs through env (SRC_SHA256, PKG_NAME, PKG_SHA256, APP_NAME, APP_SHA256 -- the update-tap.yml FORMULA_URL/FORMULA_SHA256 pattern), the heredocs stay quoted, and the checksum rows are appended with printf so the values are arguments -- data, never shell text. Two wrong turns worth recording: the printf format was first single-quoted, and shellcheck's SC2016 (info) fired on the markdown backticks inside it -- actionlint exits 1 even on info findings, so that is not a cosmetic grade; the format is now double-quoted with escaped backticks (a literal backtick inside double quotes), which keeps the table's code spans. And the first printf attempt backticked BOTH columns; executing the notes step locally against the old heredoc caught that the Source-tarball row's label is plain text in the published table -- its format now leaves the label column plain while the asset-name rows keep both columns in code spans, and the assembled notes are byte-identical to the old output under identical inputs. The unquoted-heredoc alternative was rejected: the notes contain markdown backticks, which an unquoted heredoc evaluates as command substitution. Also fixed the count mismatch the reviewer flagged as a nit: pre-release.yml's header no longer says "two deliberate differences" (the step-level ones are commented at their steps). Verification, all local: actionlint clean on both files (exit 0, zero findings), YAML parse, all 9 run: blocks per file pass bash -n, no ${{ }} remains inside any run block of either file, and each notes step was executed locally with dummy env values and byte-compared against the old heredoc output. Committed on feat/pre-release-workflow, not pushed. | ### Current state @@ -503,6 +503,8 @@ Warning: two statistics miscounts were fixed along with this: oversized frames a Any script that pushes with a credential must do the same: the safe default is to have no script anywhere embed a token in a URL. +29. **A blank line inside the AGENTS.md ledger table terminates it.** In GFM everything after the blank line renders as plain paragraphs, so a row added below a stray blank line silently drops out of the table while looking fine in the raw file. It happened to row 64: the insertion anchored on the next heading and landed BELOW the blank line that already separated the last row from that heading. When appending a row, anchor on the previous row's text, or delete the separating blank line in the same edit; afterwards scan the table -- between the `| # |` header and the next `###` heading, no blank line and every line starts with `| `. + --- ## 8. Common commands