refactor(ci): remove orchestrator; reviewer-assign in housekeeping - #115
Conversation
…ousekeeping
The project-orchestrator workflow was doing two conceptually separate
jobs: assigning a non-author admin as reviewer, and mirroring PR / issue
state into the Task Board's Status field via a bespoke state machine
with opposite-semantics linked-issue moves. It accounted for 3-4 of the
~8 workflow runs every PR push fired and was the largest source of
cross-trigger complexity in this repo (workflow_run chains, GraphQL
status-rollup quirks, multi-event re-evals).
For a 4-person team most of what it does is either already provided by
GitHub natively or one-click manual:
- Adding a PR to the project, setting initial Status, marking it
Done on merge → GitHub Project v2 native workflows ("Auto-add to
project", "Item added to project", "Pull request merged") already
enabled on project #7's Workflows tab.
- Moving the PR card on `changes_requested` / `re_requested` →
one-click in the board UI.
- Auto-flipping draft → ready on bot-clean → drop; drafts being
drafts is informative signal.
- Mirroring linked issues with opposite-semantics → drop; the PR
list itself shows what needs review.
- The only orchestrator behaviour worth automating per push:
assigning the non-author admin as reviewer on first open.
Net of changes:
- Delete `.github/workflows/project-orchestrator.yml` (546 lines).
- Delete the two composite actions used only by it
(`board-upsert-status`, `set-linked-issues-status`) and the shared
helper script (`board-fetch-item.sh`).
- Keep `.github/actions/assign-and-request-review/` — still used
by `dependabot-automerge.yml` and now also by `housekeeping.yml`.
- Add reviewer-assign to `housekeeping.yml`. Fires only on
`pull_request_target: opened` / `ready_for_review`, skips drafts
and Dependabot, picks the non-author admin via the same parity
logic the orchestrator used. Step is `continue-on-error: true`
so the title-lint required check stays the source of truth for
PR-housekeeping's pass/fail.
- Trim `dependabot-automerge.yml`: drop the board-upsert step
(native workflows handle the placement now), drop the
PROJECT_BOARD_TOKEN guard (no longer needed in this workflow),
drop the board-config.env load (no consumers left here), and
apply the marker-comment upsert pattern to the major-bump
comment so it edits in place across Dependabot's repeated
synchronize cycles instead of duplicating.
- Trim `.github/board-config.env`: STATUS_* options and ADMINS
had only orchestrator-side consumers; remove. PROJECT_OWNER /
NUMBER / ID and PRIORITY_* stay for triage.yml.
Behavioural trade-offs accepted (per the discussion that landed this):
- Reviewer gets pinged at PR-open time, not after CI + Claude have
gone green. Reviewer can ignore the ping until checks settle.
- Status=Ready is whatever the project's native "Item added" rule
sets it to (configured in the project UI). The orchestrator's
bot-clean-gated promotion no longer applies.
- `changes_requested` doesn't auto-move the board card. Manual.
- Linked issues don't auto-mirror PR state. Manual.
Workflow run count per non-Dependabot PR push goes from ~8 to ~5
(CI, Claude PR review, PR housekeeping, Admin approval, Dependabot
auto-merge skipped). The three workflow_run-triggered orchestrator
fires per push are gone.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the repository's CI infrastructure by removing the custom project-orchestrator workflow in favor of native GitHub Projects-v2 features. By offloading board state management to native automation, the change reduces technical debt, simplifies CI logic, and improves overall performance by decreasing the number of workflow runs per pull request. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
github-actions
Bot
added
github_actions
|
Claude finished @EricAndrechek's task in 2m 56s —— View job PR Review:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes custom GitHub Action workflows and scripts, including board-fetch-item.sh and composite actions for board item management, in favor of native GitHub Project workflows. Corresponding status and admin configurations were removed from .github/board-config.env. Review feedback indicates that AGENTS.md must be updated to reflect this architectural shift as required by the style guide. Additionally, it is recommended to retain the central ADMINS configuration to prevent maintenance overhead and potential configuration drift in other workflows.
EricAndrechek
changed the title
Addresses Gemini and Claude review feedback on orchestrator removal:
- Restore ADMINS to .github/board-config.env. The initial commit
dropped it and inlined into housekeeping.yml on the rationale that
admin-approval.yml already inlines it — but both bot reviewers
correctly flagged that this creates three drift-prone copies
(admin-approval.yml + housekeeping.yml + a hardcoded literal in
dependabot-automerge.yml). board-config.env is the single source
of truth again; housekeeping.yml and dependabot-automerge.yml
both load it. admin-approval.yml keeps its own inline copy with
the documented latency reason; the file's comment already notes
"update both places," which now correctly means "update this file
and board-config.env."
- dependabot-automerge.yml's reviewer list is now
`${{ replace(env.ADMINS, ',', ' ') }}` so the comma-separated
board-config representation converts inline to the
space-separated form the composite expects.
- AGENTS.md sections updated to match the new arrangement:
§"Review tooling reference" (Human admins row) — reviewer is
requested by housekeeping.yml at PR open / ready_for_review
(not bot-clean), board placement is via native Projects v2
workflows. Dropped the orchestrator's multi-event re-eval
and check_suite/workflow_run discussion.
§"Governance Files" — replaces the project-orchestrator
reference with housekeeping.yml + native project workflows.
§"Task Board state machine" — heavily trimmed. Drops the
orchestrator-transition list, the bidirectional linked-issue
mirror, and the auto-flip-on-bot-clean behavior. Adds an
explicit "things we used to automate but don't anymore" list
reflecting the trade-offs accepted in the removal PR.
- CHANGELOG.md gains a `[Unreleased] / Removed` entry describing
the deletion + the replacement arrangement.
- Stale orchestrator references cleaned up in workflow comments:
admin-approval.yml header + bypass-block comment.
claude-review.yml gate-skip comment + job-name comment.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-project-automation
Bot
moved this from In progress
to Done
in WaveHouse Task Board
#115 introduced `${{ replace(env.ADMINS, ',', ' ') }}` to convert the comma-separated ADMINS env var into the space-separated form the assign-and-request-review composite expects. Problem: GitHub Actions doesn't have a `replace()` expression function — the valid list is `contains`, `startsWith`, `endsWith`, `format`, `join`, `toJSON`, `fromJSON`, `hashFiles`, plus status checks. An unknown function in a `with:` expression fails workflow validation, which is why every branch push has been producing a ghost `.github/workflows/dependabot-automerge.yml push failure` run with no jobs and the file path (not the workflow's `name:`) as the display name — GitHub couldn't load the workflow definition cleanly for the push event. Replaced with a bash step that uses parameter expansion (`${ADMINS//,/ }`) to do the conversion, then the composite reads `env.ADMINS_SPACE`. Both steps gated on the major-bump path so they only run when actually needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Two related noise sources Both produce red workflow runs on PR pushes that don't reflect actual problems and were flooding the inbox. ### 1. `claude-review.yml` failing on PRs that edit Claude's own files The Anthropic action runs a self-validation step that fails when its own workflow file (`.github/workflows/claude-review.yml`) or the prompt template (`.github/prompts/pr-review.md`) is in the PR's diff. Every PR touching Claude's wiring (#105, #108, #109, #110, #111, #113, the merge commit of #115) has logged a noisy red Claude check that resolves itself once the change merges. **Fix:** in `claude-review.yml`'s gate step, query the PR's changed files; if either path is in the diff, set `skip=true` and exit 0. The check shows success, the action never runs, the change still takes effect on the next PR's review. ### 2. `dependabot-automerge.yml push failure` ghost runs on every branch push #115 added `reviewers: ${{ replace(env.ADMINS, ',', ' ') }}` to convert the comma-separated `ADMINS` env var to the space-separated form the composite expects. Problem: `replace()` isn't a GitHub Actions expression function. The valid list is `contains`, `startsWith`, `endsWith`, `format`, `join`, `toJSON`, `fromJSON`, `hashFiles`, plus status checks. An unknown function fails workflow validation, and GitHub records a failed run with the file path (not the workflow's `name:`) as the display name and no jobs — every push to any branch since #115 merged. **Fix:** replace the bad expression with a real bash step that uses parameter expansion (`${ADMINS//,/ }`) to write `ADMINS_SPACE` to `$GITHUB_ENV`. The composite's `with:` then references `env.ADMINS_SPACE`. Both steps gated on `update-type == version-update:semver-major` so they only run when actually needed. ## Test plan - [ ] This PR's own Claude check: success (skip-gate catches the self-modification). - [ ] After merge, branch pushes no longer trigger the ghost `.github/workflows/dependabot-automerge.yml push failure` runs. - [ ] Next major-version Dependabot PR: both admins assigned correctly via `ADMINS_SPACE`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The contributor docs (development.md §CI & review automation, CONTRIBUTING.md) still described the pre-consolidation pipeline: pr-title.yml and a `Validate` check, `Check`/`Build` required contexts, project-orchestrator.yml card movement, and label.yml — all replaced by housekeeping.yml / the single `CI` job / native Projects v2 board workflows (May 2026 consolidation, #115). Rewritten against the live config: required checks are CI, PR housekeeping (Conventional Commits title + 72-char cap, labeler step, reviewer assignment on open/ready), and Admin approval; the inverted "Lint/Test not required (#57)" note is dropped. Also fixes a Chromium misattribution carried in prose and two Makefile comments — starlight-links-validator needs no browser; only rehype-mermaid does — and adds docs/.dev-dist/ to make clean's help text and the development.md targets table. Found by the docs-reviewer pre-push gate (5 MUST / 1 SHOULD / 1 MAY, all addressed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#277) ## Summary Four change sets, one theme: the docs workflow now behaves like production, fast. **1. Production-faithful `make dev-docs`** (`docs/scripts/dev.mjs`, new): `astro dev` skips everything the Cloudflare Worker adds in production — `cloudflare-md-router` content negotiation (`.md` twins), the pagefind search index, `starlight-llm-tools` outputs — so the de-facto workflow had become manually cycling `make build-docs && make preview-docs`. The dev loop now rebuilds on save (debounced; mid-build saves coalesce) into a `.dev-dist/` staging dir, syncs into `dist/` only on green builds (plain node fs — no rsync or other external tools), and serves through `wrangler dev --live-reload` on :4321 (`DOCS_PORT` overrides): the browser refreshes itself per build, and a failed build keeps serving the last good site (red banner + terminal bell). `starlight-links-validator` is skipped in watch builds so mid-edit dangling links don't block previewing (`DOCS_WATCH_STRICT=1` keeps it on; CI and `make build-docs` enforce links unchanged). The raw HMR server stays available as `pnpm run start`. **2. Per-diagram Mermaid render cache**: every build re-rendered all 17 diagrams through headless Chromium, diagrams changed or not. `docs/astro.config.mjs` now uses [`astro-themed-mermaid` v0.2.0](Wave-RF/astro-themed-mermaid@938f765)'s `rehypeMermaid` export (released as part of this work) — `rehype-mermaid` behind a content-addressed per-diagram disk cache. Warm rebuilds drop 6–7s → ~3.7s, a single-diagram edit re-renders only its page, and output is byte-identical modulo SVG ids. Save → refreshed browser is ~4s end to end. **3. Theme-adaptive favicon that actually displays** (commit 19e7a10): four stacked causes peeled — #193 dropped the `sizes="any"` SVG link (Chromium then picks the `.ico`); Chromium rasterizes an SVG favicon once and never re-renders on theme flip (crbug.com/1208277); in-place `href` mutation is honored only transiently; and Chromium's scorer commits an exact-size `.ico` over a `sizes="any"` SVG (verified against the profile `Favicons` SQLite, contradicting the standard blog advice). Fixed via the restored icon-link pair, statically-colored `favicon-{light,dark}.svg` brand-kit variants, and a `Head.astro` live-swap script (fresh link node per repaint, `.ico` removed from live candidates). No-JS and Safari ≤18 keep working fallbacks. **4. CI/review-automation docs refresh** (commit 0b54c50): `development.md` §CI & review automation and `CONTRIBUTING.md` still described `pr-title.yml`/`Validate`, `Check`/`Build` checks, `project-orchestrator.yml`, and `label.yml` — all replaced by `housekeeping.yml` / the single `CI` job / native Projects v2 board workflows (May consolidation, #115). Rewritten against the live config, the 72-char title cap documented, and a Chromium misattribution corrected (`starlight-links-validator` needs no browser; only `rehype-mermaid` does). ## Test plan - Dev loop verified end-to-end in-session: wrangler hot-pickup of modified *and new* assets, `Accept: text/markdown` twin negotiation, `/llms.txt`, pagefind, failed builds keep serving the previous dist, file↔directory type-flip recovery in the sync, `DOCS_WATCH_STRICT=1` fails loudly on a planted broken link, SIGINT/SIGTERM/`pkill` teardown leaves no orphans - Mermaid cache profiled cold (5.8s, 17 entries harvested) and warm (~3.5s); cached output diffed byte-identical to uncached modulo ids; `astro:build:done` theme patch counts unchanged; v0.2.0 ships 12 unit tests covering hit/miss/harvest/re-id/corruption paths - Favicon: Chromium profile `Favicons` DB ground truth, Playwright DOM swap checks both directions + across view transitions, manual Brave/Firefox/Safari - Docs rewrite verified claim-by-claim against the live ruleset (`gh api`), `housekeeping.yml`, `ci.yml`, and `admin-approval.yml` by the pre-push review gate - `make ci` green for every pushed tree (tree-keyed markers); both pre-push reviewers (code + docs) at ship_it for HEAD ## Related issues - Builds on the brand kit (#142) and head cleanup (#193); favicon entry supersedes the wiring described in the branding-pipeline CHANGELOG entry - Docs refresh closes out staleness introduced by the workflow consolidation (#115) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
Delete
project-orchestrator.yml(and the two composite actions + helper script it owned), fold the only behaviour worth keeping (reviewer-assign on first open) intohousekeeping.yml, and rely on GitHub's native Projects-v2 workflows for everything else the orchestrator was doing on the board.Net diff: −887 lines. Workflow runs per non-Dependabot PR push go from ~8 to ~5.
Why now
The orchestrator was doing two conceptually separate jobs squeezed into one workflow:
For a 4-person team, #1 is genuinely useful as automation and #2 is mostly bookkeeping that GitHub already provides natively (or that's a one-click manual operation on the rare event it doesn't). Each PR push was triggering 3-4 orchestrator runs through
workflow_runchains, and the workflow has been the largest single source of cross-trigger complexity (GraphQLstatusCheckRollupperms, integration-tokenNONEfor private members, etc. — every recent CI fix has touched it).What the native Project workflows already handle
Project #7 has these enabled (verified via
gh api graphql ... { projectV2 { workflows } }):Closes #Nkeywords.That covers placement, initial Status, and Done-on-merge. The orchestrator's remaining custom behaviours are dropped (see "trade-offs" below).
Files
Deleted:
.github/workflows/project-orchestrator.yml(546 lines).github/actions/board-upsert-status/(composite, only orchestrator + the now-trimmed dependabot-automerge step used it).github/actions/set-linked-issues-status/(composite, only orchestrator used it).github/scripts/board-fetch-item.sh(helper, only the above used it)Kept:
.github/actions/assign-and-request-review/— still used bydependabot-automerge.ymland now alsohousekeeping.yml.Modified:
housekeeping.yml— added a reviewer-assign step that fires onpull_request_target: opened/ready_for_review(NOT onsynchronize— composite is idempotent but firing per-push would re-spam reviewers afterdismiss_stale_reviews_on_pushclears a request following CHANGES_REQUESTED). Picks the non-author admin by the same parity logic the orchestrator used.continue-on-error: trueso a flaky review-request can't mask the title-lint required check's exit code.ready_for_reviewadded to the workflow's trigger types so the draft → ready flip pings the reviewer.dependabot-automerge.yml— drop the board-upsert step (native handles it), drop thePROJECT_BOARD_TOKENguard (no longer needed in this workflow), drop theboard-config.envload (no consumers left), and apply the marker-comment upsert pattern to the major-bump comment (same fix as fix(ci): dependabot major-bump comment uses marker-based upsert #114 — folded in here since they touch the same step)..github/board-config.env— dropSTATUS_*(no consumers left) andADMINS(was only read by orchestrator;housekeeping.ymlandadmin-approval.ymlboth inline it). KeepPROJECT_OWNER/NUMBER/IDandPRIORITY_*fortriage.yml.Trade-offs you're explicitly accepting
changes_requestedmoves PR card to "In review"re_requestedre-fires review requestConflict with #114
#114 (the standalone dependabot major-bump comment upsert fix) and this PR both edit the same step. Whichever lands first, the other needs a trivial rebase. The upsert pattern in this PR matches #114 exactly, so if #114 lands first the conflict resolution is "take theirs"; if this lands first, #114 closes as already-incorporated.
Test plan
housekeepingruns, assigns the non-author admin, requests their review. No orchestrator runs fire.🤖 Generated with Claude Code