Skip to content

fix(ci): split ci by cadence so coverage uploads on every main commit - #3608

Merged
max-sixty merged 6 commits into
mainfrom
codecov-config-audit
Jul 27, 2026
Merged

fix(ci): split ci by cadence so coverage uploads on every main commit#3608
max-sixty merged 6 commits into
mainfrom
codecov-config-audit

Conversation

@max-sixty

@max-sixty max-sixty commented Jul 25, 2026

Copy link
Copy Markdown
Owner

What prompted this

Getting #3605 green ran into codecov reporting a base_commit three commits
older than the real merge-base. This audits whether our config causes that.

The cause

Codecov picks a PR's base by walking back to the newest ancestor that has a
coverage report. It used the real merge-base for PRs #3480, #3532 and #3602,
and a stale one for #3603 and #3605. The difference is whether the merge-base
uploaded a report. 29 of the last 40 main commits did not.

ci had one concurrency group for main pushes, and GitHub cancels the
pending run in a group whenever a newer one joins, even with
cancel-in-progress: false. So the question is how long a run holds the group,
and a run isn't done until its slowest job is:

job duration on main
fast-checks 2 min
code-coverage 3-4 min
test (windows) 11 min
collect affected coverage (windows) 110-129 min

Each main run held the group for ~2 hours, so nearly every subsequent main push
was cancelled while queued, taking the 4-minute coverage job with it. Every
cancelled main run's updated_at lands within a second of the next push's
created_at.

The 2 hours is real work, not queue: 2-5s from created_at to started_at,
then 108 minutes inside cargo affected collect — 4181 tests under
-C instrument-coverage with a per-test LLVM profile, ~5 GB of profraw.

The fix: one workflow per cadence

The three groups of jobs have incompatible needs, and one group was serving all
of them.

workflow cadence on main why
ci every commit, ~11 min required gate + fast checks
coverage every commit, keyed per-sha a skipped upload leaves later PRs on a stale base
affected sampled, ~2 h a DB a few commits old still anchors a correct superset

affected keeps exactly the grouping it has today, so its sampling is
unchanged and deliberate. It just no longer drags the other two along.

Scope of the impact

The posted codecov/patch check scopes to the PR's own GitHub diff, so a stale
base did not score PRs against other people's lines. On #3605 the posted
91.66% is exactly github.rs's 11/12, while the stale-base compare object
reported 64/65 across 13 files. What a stale base costs:

  • codecov/project reports "compared to <stale sha>"
  • the patch auto target is the stale base's project coverage (0.02pp here)
  • the compare API object widens to base..head, which is what made the
    investigation look like silence

Separately, test/lint/fast-checks also stopped completing on main. Nothing
load-bearing rode on that (they already ran on the PR), but it left
tend-ci-fix with nothing to watch, since it doesn't fire on cancelled runs.

Two smaller fixes

  • ignore: "**/tests/**" compiles to .*/tests/.* (confirmed against
    codecov's validator), which needs a leading directory and so never matched
    tests/ itself. Inert today since cargo llvm-cov reports only src/
    (verified against a downloaded cobertura.xml), but now correct if that
    changes. Now tests/**.
  • fail_ci_if_error gated on github.repository_owner, which is the base
    repo's owner on a fork PR too, so the soft-fail its comment describes never
    applied. It keys off the head repo now.

Docs

The API behaviour was ours to misuse, not codecov's to explain. Three traps,
all confirmed against the live API:

  • file_report/<path>/ 404s with coverage info not found because the route
    swallows the trailing slash into the path. Without it the endpoint returns
    line_coverage.
  • ?pullid=N always compares the PR's current head. ?base=&head= asks
    about an earlier commit.
  • the compare response has no patch_totals key, and .name is
    {base, head} rather than a string, so a filename lookup silently matches
    nothing.

A working recipe already existed in running-tend, but that skill is scoped to
CI. tests/CLAUDE.md owns coverage investigation, so the queries go there and
running-tend points at them instead of keeping a second copy.

Re-running the corrected query against #3605's failing commit reproduces the
miss exactly: src/git/remote_ref/github.rs:164, the gh repo set-default
hint, matching what the session eventually found by hand.

This PR demonstrates it

It changes no Rust at all, only YAML and markdown. Codecov still reported a
10-file, 111-line patch on its first commit, because it based the
comparison on 203603909 rather than the real merge-base 32f380a27. Every
main commit in between has no report:

commit ci run report
32f380a27 queued no
9645e3e13 cancelled no
bcd1ffdfd cancelled no
8865f20ab cancelled no

Every one of those 111 patch lines belongs to somebody else's merged commit. It
passed at 100% only because those commits are well covered.

This was written by Claude Code on behalf of @max-sixty

max-sixty and others added 2 commits July 25, 2026 15:21
Codecov picks a PR's base by walking back to the newest ancestor that has
a coverage report, so a main commit whose upload never ran leaves later
PRs comparing against a stale base. 29 of the last 40 main commits had no
report: `code-coverage` finishes in ~4 minutes, but it shared `ci`'s
concurrency group with `collect affected coverage (windows)`, which holds
a run open for ~2 hours, and GitHub cancels the pending run in a group
whenever a newer one joins it. Nearly every main push was cancelled while
queued, taking the coverage job with it.

Coverage now runs from its own workflow, keyed per-commit on main, so a
merge train can't cancel a queued upload. `collect-affected` keeps today's
sampling behaviour, which its cache design already assumes.

Also:

- `ignore: "**/tests/**"` compiles to `.*/tests/.*`, which needs a leading
  directory and so never matched `tests/` itself. `cargo llvm-cov` reports
  only `src/`, so nothing changes today, but the guard now works if that
  ever stops being true.
- `fail_ci_if_error` gated on `github.repository_owner`, which is the base
  repo's owner on a fork PR too, so the soft-fail the comment describes
  never applied. It keys off the head repo now.
- `coverage` joins tend's watched workflows, so a red coverage run on main
  still reaches `tend-ci-fix`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…home

Three endpoint traps cost a session an afternoon: `file_report/<path>/`
404s because the route swallows the trailing slash into the path,
`?pullid=N` always compares the PR's current head, and the compare
response has no `patch_totals` key while `.name` is an object rather than
a string. None of that is guessable from the response bodies, which just
answer `{"detail": ...}` or an array that looks empty.

`tests/CLAUDE.md` owns coverage investigation, so the working queries go
there, and `running-tend` points at them instead of carrying a second copy
that had already drifted from the prose version.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.

The concurrency split reads correctly: coverage-<sha> with cancel-in-progress: false on main means a merge train can't cancel a queued upload, and coverage-<pr#> with cancel-on-PR preserves supersede-your-own-pushes. coverage.yaml's on: block mirrors ci.yaml's exactly, the code-coverage job name is unchanged (so any check-name-keyed branch protection still matches), and no job in ci.yaml had a needs: code-coverage. The fail_ci_if_error fix keying off head.repo.full_name correctly soft-fails fork PRs where the old repository_owner guard never did.

One stale doc reference the move leaves behind, outside the diff: .claude/skills/running-tend/references/nightly-cleaner.md still uses code-coverage as its example of a job whose failure "belongs to tend-ci-fix, which fires on any ci-workflow failure". After this PR code-coverage lives in the coverage workflow, not ci — the handoff still works because you added coverage to tend-ci-fix's watched_workflows, but the parenthetical example now misattributes the job to ci. Worth updating the example (or generalizing "any ci-workflow failure" to the watched set) so the two references stay consistent with SKILL.md, which you did update. Happy to push that one-line doc fix if useful.

code-coverage moved to the coverage workflow in this PR and tend-ci-fix
now watches ci, coverage, publish-docs, and nightly. The nightly-cleaner
reference still described the handoff as ci-only and cited code-coverage
as a ci job. Point at the watched_workflows list instead so the two
references stay consistent with SKILL.md.
@worktrunk-bot

Copy link
Copy Markdown
Collaborator

Pushed the one-line fix in 8478521 — the nightly-cleaner.md example now points at tend-ci-fix's workflow_run.workflows list (ci, coverage, publish-docs, nightly) and notes code-coverage lives in coverage, rather than describing the handoff as ci-only. Generalizing off ci keeps it from drifting again the next time the watched set changes, consistent with the SKILL.md updates in this PR.

max-sixty and others added 2 commits July 26, 2026 08:51
`collect-affected` spends ~2h in `cargo affected collect` (108 min of it in
one step on Windows: 4181 tests run under `-C instrument-coverage` with a
per-test LLVM profile, ~5 GB of profraw). Queue time is 2-5 seconds, so
that is all real work.

Holding `ci`'s concurrency group for two hours meant every main push
behind it was cancelled while pending, so `test`, `lint` and `fast-checks`
also stopped completing on main. Nothing there is load-bearing, since the
same jobs already ran on the PR, but it left `tend-ci-fix` with nothing to
watch: it does not fire on cancelled runs.

The affected jobs keep the sampling group they have today, which suits
them. A DB a few commits old still anchors a correct superset, so
collecting per-commit would buy nothing for three ~2h matrices per merge.
What changes is that the sampling no longer reaches jobs that need to run
every time.

`ci` is now the required gate plus the fast checks, and finishes in ~11
minutes on main.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The enumeration went stale in the same PR that added it, once `affected`
joined the list. One home for it: `.config/tend.yaml`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@max-sixty max-sixty changed the title fix(ci): give coverage its own workflow so main keeps uploading fix(ci): split ci by cadence so coverage uploads on every main commit Jul 26, 2026
@max-sixty
max-sixty marked this pull request as ready for review July 27, 2026 02:42
@max-sixty
max-sixty merged commit 4202cff into main Jul 27, 2026
39 of 40 checks passed
@max-sixty
max-sixty deleted the codecov-config-audit branch July 27, 2026 03:19
@max-sixty max-sixty mentioned this pull request Jul 29, 2026
max-sixty added a commit that referenced this pull request Jul 29, 2026
Cuts 0.70.0 — version bump plus the changelog for the 45 commits since
v0.69.2.

**Minor bump, not patch.** `cargo semver-checks` reports 6 breaking
library changes (`set_command_timeout` and `ListConfig::task_timeout_ms`
removed, `WorkingTree::stage` arity changed, three enum variants added
to exhaustive enums). Worktrunk ships breaking library changes freely,
but semver still puts a break at minor while pre-1.0.

## Release validation

- Local gate green on the release commit: 4631 tests, lints, doctests.
- `nightly` dispatched on the cut-from tip (`a27cbd42`) for the full
cross-platform suite — `full-tests` green on linux, macOS, and Windows,
plus minimal-versions, nix-flake, crate-build, and the release targets.

## Data-loss surface review

The cumulative diff was audited against the deletion surface. One
deliberate widening, signed off for this release with follow-ups to
file:

- **#3602** removes the content check from `wt config shell install`'s
legacy cleanup, so `conf.d/{cmd}.fish` and the stranded nushell
`{cmd}.nu` are now deleted by path, unread. Only that exact filename is
touched and each removal is reported, but the deletion is absent from
both `--dry-run` and the confirmation prompt, and the already-configured
path skips the prompt entirely.

Also noted, none blocking:

- The `!path.exists()` check precedes the lock guard on the
`Path`/`Current` removal arm, so a locked worktree whose directory is
absent loses its branch. This already governed the branch-targeted route
in v0.69.2; #3533 unified the other arms onto it. The FAQ's "Neither
`git worktree remove` nor `wt remove` (even with `--force`) will delete
them" is absolute where the behavior isn't.
- On the default background removal path, `ensure_clean` and
`stop_fsmonitor_daemon` swapped order, so the safety gate is now
answered by the live fsmonitor daemon rather than a full re-stat.
Bounded by trash staging with 24-hour retention, and the foreground and
picker paths were already daemon-served.

Net *improvements* to the same surface: shared-branch retention across
remove/prune/merge (#3533), outcome-accurate removal reporting (#3633,
#3637), and the removal of the thread-local command timeout that could
kill in-flight git commands on worker threads (#3615).

## Changelog accuracy

Entries were verified against the actual diffs rather than commit
messages, which corrected several drafts: the prune figures were one PR
stale (~2.9 s → the real ~0.6 s), `wt merge` takes no worktree argument
so it only gained the retention half of #3533, the Azure DevOps report
is behind `--full`, #3608 never touched `nightly.yaml`, and #3601
inverted what the FAQ change actually said. Two omissions were added —
the `install-statusline` foreign-statusline fix (#3595) and the shipped
`/wt-switch-create` skill change (#3636).

> _This was written by Claude Code on behalf of Maximilian_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants