Repository navigation
mcpx testing: format.yaml -- auto-format after a green run; PR head vs main #1997
Description
Relocated from dezren39/nix#142 on 2026-10-28 (placement audit): mcpx's single source lives in developing-today/code (pkgs/mcpx); re-home tracked at #309. Original body below; issue references inside it are historical (dezren39/nix numbering).
Parent: #95
Summary
The notes ask for a separate format.yaml that auto-commits formatting to main after the formatters succeed and the test workflow is green. This issue works through what that actually requires (trigger, token, loop guard, branch protection, the race with newer commits) and finds a logical conflict with the check-mode formatters in the test matrix (#141). Plain conclusion: auto-committing to main is worse than formatting the PR head; the recommendation is a blocking format check on PRs plus an autofix push to the PR branch, and a main-side workflow that opens a PR rather than committing directly. If the direct-to-main version is still wanted, the safe way to build it is written out below.
Original notes
- testing
- separately, format.yaml which auto-commits to main when format has changes and successfully completes all formatters, receives a green test workflow
Current state
- No formatting automation. The only bot push today is the vendorHash repair in
mcpx.yml(:67-95at 05c78b2), which commits torenovate/*/dependabot/*PR branches withGITHUB_TOKEN. Its comment states the repository's policy on rewriting branches (mcpx.yml:68-70): "A human changing dependencies should see the failure and fix it deliberately rather than have it rewritten under them." The step ran on a real Renovate PR for the first time in Phase 5: compaction for tag clears #184 (run 36693225089) but took the no-op path ("builds as-is; nothing to repair"), so the push-back itself has still never run. mainhas no branch protection and no rulesets (.../branches/main/protection-> 404;.../rulesets->[]), so a direct push from Actions would succeed today, and would start failing the day protection is turned on.- Token behaviour (trigger docs): events caused by
GITHUB_TOKENdo not start new workflow runs -- so a push tomainwith it is never tested. The exception ispull_requestopened/synchronize/reopened, which now create runs in an approval-required state. A GitHub App installation token or PAT does trigger runs normally. workflow_run(events docs): only fires for a workflow file on the default branch; typescompleted/requested/in_progress; exposesworkflow_run.conclusion,head_branch,head_sha,event; chains at most three levels deep.
The conflict to resolve first
#141 puts formatters in check mode in the test matrix. If an unformatted commit reaches main, the fmt leg fails, the test workflow is red, and a format.yaml gated on "a green test workflow" never runs. Conversely, whenever the test workflow is green, formatting is already clean and there is nothing to commit. As written, the two notes cancel out. Ways out:
- Make the
fmtleg non-blocking onmainpushes (continue-on-error: ${{ github.event_name == 'push' }}) and haveformat.yamlcheck the individual job results through the API (workflow_run.conclusionis for the whole run). Works, but main can then be "green" while unformatted. - Make formatting blocking on PRs, so main is always formatted by construction, and use
format.yamlonly for drift that does not come through a PR: a formatter version bump inflake.lock, a direct push. That is the recommended framing below.
Approaches
A -- auto-commit to main after a green run (the notes' design), least recommended
How to build it safely if chosen:
on: workflow_run: { workflows: [mcpx-test], types: [completed] };if: conclusion == 'success' && event == 'push' && head_branch == 'main'(ignorecancelled, PR runs, and dispatches).- Check out
workflow_run.head_sha; run the same pinned formatters (Bump dezren39/nix flake input to 3499ed3 — opencode now defaults to v2 #141:nix fmt); ifgit diff --quiet, exit. - Race: if
mainhas moved pasthead_sha, skip (the newer commit's own run will triggerformat.yamlagain) rather than rebase -- rebasing a bot commit onto untested code is the worst of both. - Verify the diff is formatting-only before pushing:
go build ./...andgo vet ./...,deno check,nix evalof the package -- seconds, and it catches a formatter bug or a bad version bump. - Bound what the bot may write (from Phase 2: transfer with no successor drops grants #170): stage the explicit list from
git diff --name-only, nevergit add -A(the repair step already stages one named file,mcpx.yml:93); abort on an unexpectedly large diff (a formatter version bump can legitimately reformat the whole tree -- a threshold such as 500 lines or 50 files, or any file outside a configured set, hands it to a human PR instead); re-run the check after committing and fail if it is still dirty, which catches two formatters fighting over one file. - Add
workflow_dispatchbesideworkflow_run:workflow_runonly fires for the workflow file on the default branch, so without it the workflow cannot be exercised before it merges (from Phase 2: transfer with no successor drops grants #170). - Loop guard: commit as
github-actions[bot]with a marker ([mcpx-format]) and skip when the triggering head commit carries it; withGITHUB_TOKENno run is triggered anyway. With an App token, the push triggers a test run, which triggersformat.yaml, which finds no diff and stops -- "no diff" is the real guard, the marker is belt and braces. - Branch protection: when it is turned on, the pusher must be on the bypass list; that requires a GitHub App (PATs belong to a person and bypassing as a person is worse).
Why it is least recommended:
- With
GITHUB_TOKEN, the formatted commit on main is never tested. For gofmt that is nearly safe (it only rewrites layout); fordeno fmt,shfmt,nixfmtafter a version bump it is "probably fine", which is the wrong standard for main. - Every unformatted merge becomes two commits on main (the change, then "format"), permanently in history and in
git blame. - It needs
contents: writeon the default branch in an automated job, and it fights branch protection. - It only ever fires if the conflict above is resolved by making main's format check non-blocking.
B -- open a formatting PR from main drift
Same trigger as A; instead of pushing to main, peter-evans/create-pull-request (or gh pr create) opens or updates a single branch bot/mcpx-format with the diff. The PR runs the full test workflow (with an App token; with GITHUB_TOKEN it lands in the approval-required state), and a human or auto-merge merges it. Tested, reviewable, protection-compatible, one extra PR when drift happens. Good as the main-side fallback.
C -- blocking check on PRs, autofix pushed to the PR head, B as the main fallback (recommended)
- The
fmtleg in the test workflow is blocking on PRs (check mode, prints the diff and the one command to fix:nix fmt; the diff also goes into$GITHUB_STEP_SUMMARYas agit apply-able patch, from Phase 2: transfer with no successor drops grants #170). - An
autofixjob (same workflow, afterfmtfails, same-repo branches only -- forks cannot be pushed to) runsnix fmtand pushes a commit to the PR branch, exactly like the existing vendorHash repair. Merge it into one "bot push-back" job with the vendorHash repair so a bot PR gets one push, not two. Scope it the way the repair is scoped (changed in the 2026-09-30 pass, from Phase 2: transfer with no successor drops grants #170): always on bot branches (renovate/*,dependabot/*-- where a formatter bump lands, open question 3), and on a human's branch only when the author opts in (e.g. anautofixlabel). The repository has already written down that a human's branch is not rewritten under them (mcpx.yml:68-70); an unconditional autofix contradicts that and rewrites a branch mid-review, invalidating review comments and forcing a pull before the next push. If the user wants autofix on every PR anyway, that is a policy change to make explicitly, together with the repair step's comment. - Token: a GitHub App token, so the push triggers a normal (not approval-gated) run of the full workflow; the superseded run is cancelled by the PR concurrency group (Update Rust crate tokio to v1.38.2 [SECURITY] #135). Loop guard: no diff -> no push; and skip when the head commit author is the App.
format.yamlexists only for drift on main that did not come through a PR (formatter bump, direct push): approach B.- Result: every commit on main is formatted and tested; with squash merges the autofix commit leaves no trace on main.
- Off-the-shelf alternative to a custom job: autofix.ci (a GitHub App, free for OSS) does the PR-head push with its own loop/fork handling; it requires trusting a third-party app with write access. Worth considering; decide with the App-token question.
Recommended because it keeps the notes' intent (nobody hand-formats; formatting never blocks a human for long) without an untested commit on main, and without overriding the branch-rewriting policy the repository already has.
Open questions
- Is the user OK replacing "auto-commit to main" with "autofix the PR head + a formatting PR for drift"? If not, build A with the safeguards above.
- GitHub App (new, owned by the user) vs autofix.ci vs a fine-grained PAT for the push-back? The same token would fix the vendorHash repair's approval-required runs (build(deps): bump build/buck/preludes/buck2-prelude from
d956689to3cfc766#49). - Should Renovate/Dependabot formatter bumps (a new nixfmt in
flake.lock) reformat inside the bump PR via the same autofix job? That is the cleanest place for format drift to land. - Does the user merge by squash? (If merge commits are used, autofix commits appear in main's history.)
- A pre-commit hook (and which framework), or is
nix fmt/just fmtplus the blocking check enough? A hook makes the blocking check nearly unreachable andformat.yamlalmost dead code -- which is fine; it is a backstop (from Phase 2: transfer with no successor drops grants #170). - Bot identity: reuse
mcpx-hash-bot(mcpx.yml:91-92) or a separatemcpx-format-botsogit logsays which bot did what? Cosmetic; decide once (from Phase 2: transfer with no successor drops grants #170). - Should the same push-back machinery regenerate derived artefacts CI checks (man page, OpenAPI, settings table)? That is Bump golang.org/x/crypto from 0.16.0 to 0.17.0 in /sources/import #96/⚡ Bolt: Optimize cursor color generation and position grouping #145's drift problem; build the mechanism once (from Phase 2: transfer with no successor drops grants #170).
Ease · Impact · Related
Ease M (needs a token decision) · Impact low-med (convenience; quality comes from the blocking check) · Depends on / related: #95, #141 (the formatters and their pinning), #135 (concurrency cancels superseded runs; the vendorHash push-back job this merges with), #49 (bot push-back for every package, token choice), #99 (agents run nix fmt before pushing; CI is the backstop).
Security/auth deferred; see tracker.
Acceptance criteria
- format.yaml auto-formats only after a green run, distinguishes PR-head vs main behaviour as specified, and its CI run is green.
- The work described in the Summary/Scope above is delivered via a branch + PR (no direct push to main); verification runs in GitHub Actions CI, not on the sandbox.
- Evidence (CI links, timings, or probe output as applicable) is recorded on this issue before it is closed.
Activity
developing-today commentedon Oct 28, 2026
CODE-922
dezren39 commentedon Oct 28, 2026
History carried over from dezren39/nix#142 (verbatim, oldest first):
@dezren39 · 2026-09-30T06:51:40Z
Overlaps #170, written concurrently by a second worker on the same slice. Both are kept until reconciled; see the tracker (#95) for the differences and which one to keep.
@dezren39 · 2026-09-30T07:21:02Z
Integration pass. Placed in the DAG rooted at #181.
d956689toe7457f6#52 blocks sharding@dezren39 · 2026-09-30T12:03:18Z
Issues pass (2026-09-30, at 05c78b2).
mcpx.yml:68-70: "A human changing dependencies should see the failure and fix it deliberately rather than have it rewritten under them"). C's autofix is now always-on for bot branches and opt-in (label) for human PRs; an unconditional autofix would contradict that policy and should be an explicit policy change if wanted.workflow_dispatchbesideworkflow_run, the step-summary patch; open questions on a pre-commit hook, bot identity, and regenerating derived artefacts.dezren39 commentedon Oct 28, 2026
Intake review
🤖 Agent note · state: filed
dezren39 commentedon Oct 29, 2026
Triage verdict — scope corrected: the blocking-check half already exists, and a newer fleet issue now owns the autofix vehicle
Triaged against
developing-today/codemain@4d157755(2026-10-29). Queue task2906(kind triage, workerrepo-worker-code, originopenchamber-scheduled-lane). No implementation here, as scoped.Verdict: not a duplicate, but do not build a mcpx-specific
format.yaml. Formatting is already a blocking check, and the autofix workflow this issue recommends is now the subject of the fleet program (#2014 design, #2015 proof-of-concept). Keep this issue open, re-scoped to the mcpx-side input those issues need — and reconcile one real conflict, below.Current state (the body's "no formatting automation" has half moved)
treefmt checkjob exists and runs on both PRs and pushes tomain, checking the changed files of the diff (git diff --name-only base...HEAD→nix run .#format-check)..github/workflows/nix-ci.yml:4-12,29-60format.yaml… auto-commits".github/workflows/at all. So a formatting diff reachingmaintoday turnstreefmt checkred and nothing repairs it.ls .github/workflows(no format/fix)/branches/main/protectionis not set andrulesetsreturns[]— a workflow push tomainwould succeed today and break the day protection arrives.gh api …/branches/main/protection,…/rulesets→ empty,0#141: a green run can never need formatting"mainpushes too (on.push.branches: [main]), so main can be red on formatting with nothing fixing it, and any fix gated on "a green test workflow" can still never fire.A newer issue supersedes this one's implementation vehicle
These come from the 2026-10-28 fleet directive and direct the opposite of this body's recommendation: they commit the formatter diff to
main; this issue recommends formatting the PR head and opening a PR instead. That divergence must be settled in #2014 — and the sequencing consequence is the body's own option 1: ifmainis auto-formatted after the fact, thetreefmt checkonmainpushes must not block (or must be ordered so it cannot cancel the repair), otherwise the repair job can never be reached. Two-way pointers added.Re-scope: this issue = the mcpx-side input to #2014/#2015 (what the check does today, the changed-files-only behaviour, the main-push red case, and the fact that PRs already get a blocking gate so the autofix only earns its keep for drift that bypasses a PR:
flake.lockformatter bumps, direct pushes, merges that skip the hook). It should not grow a second workflow.Dedupe — no duplicates; neighbours named
nix-ci.yml's treefmt job.nix-ci.yml:66.Proposed next steps (small, mostly elsewhere)
pkgs/mcpx/**participates in the same treefmt job (the mcpxformat-checkis repo-wide, not a separate leg).treefmt checkmust stop being able to block the repair it needs (the body's option 1), or the repair must not be gated on the check's own result.developing-today/code, this issue closes as delivered by it, with the acceptance criteria (loop guard, no untestedGITHUB_TOKENpush, main-side ordering) evidenced there rather than duplicated here.Corrections to carry
mcpx.yml:67-95vendorHash citation is stale (that step is nowmcpx.yml:151-190ish and gated to bot branches); the policy quote it carries still stands.#141→#1996,#142→#1997(this),#143→#1998,#95→#1901,#130→#1973,#127/#166/#168not relocated as issues (history only).Why no follow-up packet was enqueued: the implementation now belongs to #2014/#2015 (owned elsewhere), and
repo-task/v1has no implementation kind for the mcpx-side remainder — same pilot observation as #1983/#1990/#1994/#1944. Enqueueing asmall_fixfor step 2 would collide with #2014's decision.Labels left truthful:
work/assigned,queue/claimed,status/in-progressremoved.— worker
repo-worker-code, openchamber scheduled lane, queue task2906dezren39 commentedon Oct 29, 2026
🤖 Agent note · triage done (scope corrected)
repo-worker-code(openchamber scheduled lane), queue task2906(kind triage, originopenchamber-scheduled-lane)format.yaml— a blockingtreefmt checkalready runs on PRs and main pushes (nix-ci.yml:29-60, changed-files only), no autofix workflow exists, no branch protection/rulesets, and the autofix vehicle is now the fleet program investigate: fleet-wide format auto-fix workflow — push token, loop guard, concurrency #2014 (design) / design+PoC: format auto-fix workflow in one repo, ready to replicate fleet-wide #2015 (PoC).work/assigned,queue/claimed,status/in-progressremoved.dezren39 commentedon Nov 3, 2026
Triage — task 7816 (queue repo-health-code-pilot, lane code-d)
Verdict: superseded — closing with evidence. The auto-format function this issue designs was delivered repo-wide under Drew's own 2026-11-02 CI-policy redesign, with different mechanics than this issue recommended. No further work is owed on issue #1997.
Evidence at current main (verified fresh 2026-11-03 via
gh):.github/ci-policy.md(10,757 bytes on main) sets the policy: formatting is mechanical and NEVER gates a merge; no treefmt/format context is in the required checks..github/workflows/auto-format.ymlimplements it: treefmt on a 15-minute schedule, formats files changed since the last successful pass, commits fixes directly to main as github-actions[bot], staging an explicit changed-file list (nevergit add -A); no diff means no commit. Workflow state: active; latest run 2026-11-03T09:47Z success; repo variableAUTO_FORMAT_LAST_SUCCESS_SHAadvancing (updated 2026-11-03T09:48:30Z).pkgs/mcpx— the mcpx-scoped ask here is subsumed.How this issue's open questions were answered by that directive:
style(treefmt): auto-format on main.Stale premises in the issue body, corrected: main now HAS branch protection (the protection endpoint returns a config; the automation PAT is the sanctioned write path); the mcpx test matrix now lives in
mcpx-test.yml. Separately, most mcpx workflows are currentlydisabled_manually— a fleet-known infra state, not part of this issue;auto-format.ymlitself is active.The literal acceptance line "auto-formats only after a green run" was deliberately not adopted: the delivered design formats on schedule and the PAT push is checked afterwards like any human push. That deviation is Drew's recorded directive in the workflow header (2026-11-02), so this issue closes as superseded rather than escalated.
Labels reconciled to terminal state (no queue/status/work labels remain).
dezren39 commentedon Nov 3, 2026
agent-note:v1
state: triaged (coverage pass)
Coverage pass 2026-11-03: reviewed in the fleet-wide per-issue pass (drift / overlap / already-handled / stale / blockers / ordering). Verdict: still live; queued in repo-health-code-pilot as triage at priority 40 (priority/normal). If this is already handled at current main, the triage closes it with evidence; if it needs splitting, the triage files the split.
Found via: coverage pass 2026-11-03