[claudeleet]

What this changes

1. New slack-on-pull-request.yaml. Posts a pull request that needs action - opened or reopened and not a draft, or marked ready for review - to the caller's channel, and nothing else about it. Forks are skipped, Dependabot's are posted, and each skip leaves a notice saying why. A call from any event but pull_request fails, including pull_request_target.
2. slack-on-failure.yaml posts through a bot token. Both workflows use chat.postMessage with SLACK_BOT_TOKEN and a required channel input, replacing SLACK_WEBHOOK. A refusal fails the job with Slack's error code and a hint, such as inviting the app to the channel.
3. This repository's own callers. announce-pull-requests.yaml is added and listed in the failure watcher; both post to #l337-org.
4. action-pins accepts this repository's version tags. Exactly L337-org/github-workflows/<path>@vN or @vN.N.N. Anything else at a tag or branch still fails.
5. Hygiene check: every Slack call names a channel.
6. README documents the announcer, the token (two secrets, inviting the app, scopes, rotation), and versions and releases, including the moving major tag.
7. Tests run the shipped step scripts against stand-ins for curl and gh, and the pin check over sample references, in a new premerge job.

Breaking

The reporter's interface changes, so this is v2.0.0. Each caller moves to v2 in its own pull request, adding channel and switching the secret at the same time; until then it stays on its current commit and keeps working.

Tested

• 56 tests pass locally. Removing the fork and draft filters fails 5; removing the tag exemption fails 4.
• actionlint, which runs shellcheck over the scripts, is clean.
• The updated pin check passes over this repository.
• The hygiene check over the other local repositories finds only the expected missing channel in docker-mcp and claude-routines from this change.
• A push creating a v* tag from a non-admin account was refused by the tag ruleset.

Not yet verified

• No real Slack post yet. Opening this pull request is the first: this repository's announcer runs from the pull request's branch and should post to #l337-org.
• The reporter change is only exercised after merge, because a workflow_run watcher fires from the default branch.
• Creating v2.0.0 and moving v2 under the tag ruleset's signed-commit rule has not been tried; that happens at release.
• Moving and deleting a tag as a non-admin were not tested, only creating one, because the test would mean force-pushing over a real tag.
• The posting block is duplicated in both workflows; nothing checks the copies stay identical.
• quaffed, apt and send-to-influx call the hygiene check at v1.1.3, which predates the failed-runs check, so their move to v2 also needs a failure watcher each.

[github-actions]

Claude review: approve

Summary

The change does what it sets out to do. The announcer's filters run in the right order: trigger, then action, then fork, then draft, and only after those the token and channel checks, so a fork pull request with no secrets is skipped rather than failed. The token is sent to curl through a header file. Untrusted pull request fields are passed through env: and escaped for Slack. The action-pins regex is anchored on uses:, so evil/L337-org/... and L337-org/other-repo are refused. I found nothing that should block a merge. The findings below are about documentation, one gap in the migration guard, and test coverage.

There are no CLAUDE.md or AGENTS.md files in this repository, so I reviewed against the README's own rules, including its "Recorded state" rule.

Findings

1. Maintainability: the hygiene script still says "the commit your repository pins".
actions/repo-hygiene/check-repo-hygiene.py:15 reads "To run it locally, fetch this file at the commit your repository pins". This change rewrote the lines either side (9–12) for version tags but left this one. It is now untrue for a caller on @v2. It should say "the version your repository's repo-hygiene step names", matching the README's local-run section.

2. Maintainability: the comment on the new premerge job is untrue, and its name is too narrow.
.github/workflows/premerge.yaml:52-53 says "The Slack workflows run only in other repositories' events". In fact this repository calls both of them itself: announce-pull-requests.yaml (added here) and report-failures.yaml. The job also runs tests/test_action_pins.py, which has nothing to do with Slack. Neither the job name "Slack workflows behave" nor the comment mentions that. A failure in the pins tests would show up under a Slack job name.

Suggestion:

• Rename the job to something like "Shared workflows and actions behave".
• Give the real reason the tests are needed: Slack and other repositories' events can't be exercised from a pull request here.

3. Maintainability: the migration guard checks only half of the breaking change.
check-repo-hygiene.py:638-662 (slack_channel_findings) checks for with: channel: but not for the renamed secret. Take a caller that moves slack-on-failure.yaml to @v2 and adds channel, but still passes SLACK_WEBHOOK under secrets::

• The check passes it.
• GitHub rejects a call that passes a secret the called workflow doesn't declare. So would leaving out SLACK_BOT_TOKEN, which fails at run time with "No SLACK_BOT_TOKEN secret".
• The watcher runs only from the default branch, so this shows up only after merge.
• Nothing reports the watcher's own run, so the failure is silent.

That is exactly the outcome the README says the check prevents (README.md "Moving a caller from v1": "a move that leaves a caller unchanged fails its own pull request rather than breaking after merge"). Suggestion: in the same function, also require secrets: SLACK_BOT_TOKEN, or at least refuse SLACK_WEBHOOK, and add rows for this to test_hygiene_requires_a_channel_on_every_slack_call.

4. Maintainability: recorded state in the docs and comments.

• README.md, "Which channel": "#docker-mcp, #quaffed and #send-to-influx serve their repositories; every other repository posts to #l337-org." This describes configuration held in each caller's with: channel:. It goes stale as soon as a repository is added or a channel is renamed. Either drop it or state it as a rule, such as "a repository with its own channel posts there; the rest post to #l337-org".
• README.md "Why a tag and not a commit", and actions/action-pins/action.yml:11-15: "the v* tags are protected by a ruleset that blocks creating, moving and deleting them, with a bypass list no wider than main's". This describes repository settings, which gh api repos/L337-org/github-workflows/rulesets answers. It is also the reason for the exemption, so keep it, but phrase it as a condition to maintain. For example: "the exemption holds only while a ruleset blocks creating, moving and deleting v* tags with a bypass list no wider than main's; loosening either withdraws it." That way it reads as a rule rather than a snapshot.
• README.md "The Slack token": "as the L337-org Slack app". This names an account configured in Slack. It is minor, but it is the same kind of claim.
• tests/test_slack_workflows.py:5: "Slack itself, which is exercised by the first announcement after merge." This is a one-time event. It belongs in the pull request description. The limitation itself ("Slack itself is not tested") can stay.

5. Preference: the posting and refusal handling is duplicated, and only one copy is tested in depth.
The code from umask 077 to the final echo is about 40 lines and is identical in slack-on-failure.yaml:139-175 and slack-on-pull-request.yaml:356-394, apart from the messages. Only the announcer's copy is tested for:

• each error hint
• the non-JSON truncation
• the long acceptance
• curl failing outright

The reporter's copy is tested only for not_in_channel. If the two copies drift, the tests won't catch it. Sharing the code through a composite action is awkward: ./ inside a called workflow resolves to the caller's checkout. So keeping the duplicate is defensible, but then the refusal tests should be parametrised over both workflows.

6. Preference: README line wrapping.
Three places break the ~100-column wrap the rest of the file keeps:

• README.md:88-89: "…so a gate job" runs to about 110 columns.
• The "A post that fails, fails the watcher's run" bullet: "…unreachable after retries. A transient refusal is" runs long.
• "Changing anything here": "(tests/), and reviews each pull request with the" is a short line.

What I checked

• I read the full diff, the commit message, both Slack workflows in full, action-pins/action.yml, the watcher and channel checks in the hygiene script, the premerge and watcher workflows, and the affected README sections.
• I traced the announcer's control flow for each skip and failure path under set -euo pipefail.
• I traced the action-pins regex against the accepted and refused cases in the tests.
• I confirmed that announce-pull-requests.yaml is listed in report-failures.yaml, and that 'Announce pull requests' matches its name:.
• I checked the hygiene script's Slack-call regex for both workflow names, in both the ./ and remote forms.

What I did not check

• The tests: I couldn't run them. Running commands needed approval in this session. Their pass/fail status comes from CI, not from me.
• Tags: I couldn't list the remote tags, so I haven't confirmed that a v1 tag exists to match the README's "Moving a caller from v1".
• The ruleset: I haven't confirmed the ruleset the tag exemption depends on.
• DS.6.1 / DS.6.2: I couldn't verify the renumbering in action-pins/action.yml:5,11. The standard isn't in this repository.
• Slack: I didn't exercise Slack's real responses.

Declined

None. There were no earlier rounds.

---

full review of fd692a603b2f..2783a723e279. Run.

[claudeleet]

Review findings, all addressed in 12e2428 except one point in 4:

1. Fixed. check-repo-hygiene.py now says to fetch it at the version the repository's repo-hygiene step names.
2. Fixed. The job is renamed "Shared workflows and actions behave", and its comment gives the reason: the refusal, fork and draft paths cannot be made to happen from a pull request here, and the pins check needs references this repository does not contain.
3. Fixed. The check (now "Slack calls name a channel and pass the token") also fails a call that passes SLACK_WEBHOOK or does not pass SLACK_BOT_TOKEN; secrets: inherit passes. Tests cover each combination. Run against docker-mcp's current caller, it reports both the missing channel and the old secret.
4. Fixed: the channel list is now a rule, the tag ruleset in the README and action-pins is stated as the condition the exemption depends on, and the test docstring no longer mentions the first announcement. Kept: "as the L337-org Slack app". It names which app's token to rotate and which app to invite to a channel, and the README's rotation steps are unusable without it.
5. Fixed. Every refusal, truncation, long-acceptance, curl-failure and token-handling case runs against both workflows. Breaking the reporter's truncation alone fails one test.
6. Fixed. The three paragraphs are rewrapped to 100 columns.

Not checked by the review, for the record: the v* tag ruleset refused a non-admin push creating v0.0.0-ruleset-test ("Cannot create ref due to creations being restricted"). There is no moving v1 tag; the README now says "a v1 commit pin".

82 tests pass locally; actionlint is clean.

[github-actions]

Claude review: approve

Resolved

• 1. check-repo-hygiene.py:15 now says "the version your repository's repo-hygiene step names".
• 2. The job is renamed "Shared workflows and actions behave" (premerge.yaml). Its comment now gives the real reason, and it covers the pins tests.
• 3. slack_channel_findings (check-repo-hygiene.py, from line 662) now refuses SLACK_WEBHOOK and requires SLACK_BOT_TOKEN. secrets: inherit passes. The test table covers each combination, including a call with a missing channel and the old secret together.
• 4. Mostly resolved:
  • The channel list is now a rule.
  • The ruleset is stated as the condition the exemption depends on, in both the README and action-pins/action.yml.
  • The test docstring no longer mentions the first announcement.
  • Kept, with reasons (the README's rotation steps need the app's name): "as the L337-org Slack app". That is a naming claim, but not a stale-prone one, so I accept it.
• 5. The refusal, truncation, long-acceptance and curl-failure cases, and the token-handling case, now run against both workflows through the BOTH parametrisation. The channel assertion differs per workflow.
• 6. The three README paragraphs are rewrapped.

Still open

None.

Declined

• Author, on finding 4: keep "as the L337-org Slack app". I agree with the reason given.

New

1. Preference: the rewraps missed two lines the commit itself edited.
   • actions/action-pins/action.yml:14 is about 130 columns: "breaking change goes to a new major tag while callers stay on the old one. It is named exactly rather than as the organisation,". The new wording was spliced onto the old line instead of being re-flowed.
   • check-repo-hygiene.py:687 is about 105 columns: "when it moves to the version that requires them. Its watcher only fires from the default branch, so". The file otherwise wraps at 100.
2. Preference: the function name check_slack_calls_match_v2 (and slack_channel_findings) now describe a version rather than a rule. The check name shown to users, "Slack calls name a channel and pass the token", is fine. The test test_every_shipped_caller_here_names_a_channel also still says "channel" only. Neither needs to block the merge.

No bugs found in the delta. In the hygiene change, continue on secrets: inherit runs after the channel finding is appended, so a missing channel is still reported. A non-dict secrets value is treated as no secrets, which is the safe direction.

What I checked

• The full delta diff.
• A search for stale references to the old check or job names: none remain.
• Line lengths in action.yml and the hygiene script.

What I did not check

• I could not run the tests (running commands needed approval). The author reports 82 passing and actionlint clean.
• I did not verify the ruleset, the tags or Slack.

---

delta review of 2783a723e279..12e2428800ee. Run.

[claudeleet]

Both new points fixed in d18c20f: the two spliced lines are rewrapped (action-pins/action.yml, check-repo-hygiene.py), and the check is now check_slack_calls_take_channel_and_token with slack_call_findings, and the shipped-callers test is named for the channel and the token. 82 tests pass locally; actionlint is clean.

[github-actions]

Claude review: approve

Resolved

• Finding 1 (preference, previous round's New): both spliced lines are re-flowed, in actions/action-pins/action.yml:14-16 and in the check_slack_calls_take_channel_and_token docstring in check-repo-hygiene.py.
• Finding 2 (preference, previous round's New): the check function is now check_slack_calls_take_channel_and_token and its helper is slack_call_findings. The shipped-callers test is now test_every_shipped_caller_here_names_a_channel_and_passes_the_token.

Still open

None.

Declined

• The author declined finding 4 of the earlier round, keeping "as the L337-org Slack app", and the previous review accepted that. I don't raise it again.

New

None. The delta is a rename and a rewrap, and I found no bugs, stale references or recorded-state claims in it.

What I checked

• The full delta diff.
• A repo-wide search for slack_channel_findings and check_slack_calls_match_v2: no references remain, in the tests, the CHECKS table or the docs.
• The renamed CHECKS entry still reads correctly and fits the line width.
• The rewrapped comment and docstring lines wrap consistently with the surrounding text.

What I did not check

• I did not run the tests or actionlint, because running commands needed approval. I rely on the author's report of 82 passing and actionlint clean.
• Because the change only renames and rewraps, a missed reference would show up as an import or attribute error. The search above found none.

---

delta review of 12e2428800ee..d18c20f290f2. Run.