This reverts commit 0475fad2ec.
When the downstream/* labels were added, the intent was for a
"downstream/foo" label to mean "this PR is important for foo". Hence we
had downstream/fedora, downstream/debian, …, a some time later
downstream/amutable. But the PR job started adding that label to all PRs,
even for cleanups and other things that are done by a person from the
company, but might not be directly related to the downstream product that
is being built. This dilutes the usefulness of the labels. Also, the
config in the CI job was only added for downstream/amutable, so one
company was more visible than others. If we filled this out for all
companies and projects, many PRs would have a whole list of lables, since
people can be affiliated with both a company and a distro, and some PRs
are done by multiple people in cooperation. But we don't actually care
about affiliations of authors that much. Let's just drop this.
systemd also builds against musl, and musl behaves differently from glibc in
places. The review agents only have the model's knowledge of musl when they
check code that depends on how musl differs from glibc. Let's clone the latest
musl next to the kernel and glibc and tell the agents where it is. They can
then check libc behavior in the musl source as well. The WHATSNEW file in the
musl tree lists the changes in each musl release.
The review agents rely on what the model knows about glibc when they check
systemd code that uses glibc interfaces, the same as for the kernel. Let's
clone the latest glibc next to the kernel and tell the agents where it is. They
can then check system call wrappers, NSS, and errno values in the glibc source.
The abilist files in the glibc tree list the glibc version that added each
symbol. We clone the kernel and glibc in parallel to keep the step short.
The review agents rely on what the model knows about the kernel when they
check systemd code that uses kernel interfaces. That knowledge can be wrong,
and it doesn't include anything merged after the model's training. Let's clone
the latest mainline kernel before the review and tell the agents where it is.
They can then check system call semantics, flags, and error codes in the kernel
source. When the clone fails, the review runs without the kernel tree.
Changes to the service manager got no component label unless they also
matched one of cgroups, creds or mount. The pid1 label already exists
and is used for issues, so apply it to pull requests touching
src/core/ as well.
Labels such as documentation, tests or util-lib are matched by nearly
every pull request that adds a feature, since it comes with a man page,
a test and a helper in src/shared/. They say nothing about how much of
the tree a pull request touches, yet they used up the five labels
allowed before it is relabeled tree-wide, so a pull request touching a
single component regularly ended up as tree-wide. Out of the last 50
pull requests labeled tree-wide, 17 matched at most one component.
List these labels as generic in labeler.json. They no longer count
towards the limit, and since they describe a pull request the same way
however much of the tree it touches, they are applied even when it is
labeled tree-wide.
src/varlink/ never existed, so the varlink label was never applied to
changes of varlinkctl. Match varlinkctl and varlink-monitord by name
instead, like the other tools, so that their man pages, shell
completions, tests and units are covered as well.
actions/labeler did not let "*" match "/", so "ci" only covered the
files directly in .github/ and workflow changes were labeled "tests".
Since 1060fd7472 "*" matches any run of characters, so ".github/*"
covers .github/workflows/ as well and every workflow change gets both
labels, spending two of the five labels allowed before a pull request
is relabeled tree-wide on a single fact.
A workflow change is a CI change, so leave it to "ci".
Every component lists its sources in its own meson.build, so the
"*meson.build" pattern labels nearly every pull request that adds a
file as a build system change. Limit the label to the top-level
meson.build and meson_options.txt and to the meson helpers in tools/,
where the build is actually defined.
The build-system label was added in 79ce5f940e with the meson patterns
plus Makefile and configure. Since 5e12de940a dropped the latter two,
its patterns are identical to those of the meson label, so every pull
request touching a meson.build file gets both. That spends two of the
five labels allowed before a pull request is relabeled tree-wide on a
single fact.
Keep build-system, the label that names the area rather than the tool,
and stop applying meson automatically. The label itself stays around
for manual use.
This reverts commit f1ea185bcf.
This was maybe a reasonable idea, but it's causing annoying usability
regressions. Since this was merged, the github CI hides the merge button
and replaces it by button to rebase, and shows a strange message:
> This branch is out-of-date with the base branch
> Merge the latest changes from main into this branch. This merge commit will
> be associated with $user.
Sometimes, the CI is overloaded, and the job that is usually quick takes
a while to run. This means that the user is permanently blocked from using
the UI to merge a PR.
We actually need patches that are merged to be reviewed. This job just
does a _very_ narrow check that can catch one very particular issue, but
is not enough to really ensure anything. And since it's creating a
usability regression, let's drop it, and go back to old style eyeballing,
which we should have been doing anyway.
This reverts commit 375d42ff3f.
The local mirror was supposed to be temporary workaround. Now that the
official mirror is back up, let's go back to using that.
A later commit cuts bindings.h down to the headers the Rust code uses,
so there's nothing left to check. It fires on main right now anyway:
9c76ca4cec moved the Authenticode code to src/shared/ and bindings.h
doesn't list authenticode.h. Drop the test and the linter step running
it.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Add rustc and bindgen to the build images and tools trees so
-Drust=auto turns Rust support on wherever the packages are new enough.
That's Debian 13 and testing, Fedora, CentOS Stream 10, openSUSE
Tumbleweed and Arch. Ubuntu noble's rustc is too old so Rust stays off
there. Add rustfmt and clippy to the tools trees where the distribution
packages them separately.
The mkosi images of the distributions whose toolchain is known to be
new enough are built with -Drust=enabled so a toolchain that quietly
stops being found fails the build instead of silently dropping Rust.
Debian 13 ships exactly the floors, rustc 1.85.0, bindgen 0.71.1 and
meson 1.7.0, so its image doubles as the check for the oldest supported
toolchain.
rustfmt, the bindings header check and clippy run in the linter
workflow whose tools tree is Fedora rawhide. So that covers the newest
toolchain. Changes to Rust code get the rust label.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
In general, "unit tests" are better then "integration tests" which require
an image to be created. For stuff that can be immediately tested from the
build directory, let's use unit tests.
The subtest ran inside the TEST-74 image and looked the programs up by a
hard-coded list of names. Register it with meson instead, so that it runs
in "meson test" against the freshly built binaries, using the actual list
of installed programs and aliases. A few programs cannot be introspected:
ukify is (still) a Python script, the mount helpers implement the mount(8)
interface, and systemd does not list its init alias among its names.
Aliases are passed as the bare names of the symlinks in the build
directory, so the script turns them into paths before running them, and
checks the invoked name against all names of a command, as the aliases of
a command are listed after its main name. The introspection of each
program is captured once and inspected with jq several times, which keeps
the test at a few seconds. The script is not installed, since the
installed unit tests are run without arguments. jq is added to the
dependencies of the GitHub unit test job.
I'd like to be pinged about work happening on sysupdate, since this is a
hotly active component with possibly overlapping work happening in the
STF project and by Amutable
* ed3eed38f2 Fix key-file path for extension/portable
\
* 43e8b2d35a Handle repart definitions for extension/portable
* 04816c3b82 Add oci-archive output format
* 7a9c94b63a Install ruff on recent Debian
* 4a7b08985e Change contextlb.contextmanager typing to Generator
\
* 5ca453ab6b Make OUTPUT variable available in postoutput scripts
* 90463ff04d Add tests for BuildStream support
\
* d36b138612 Add BuildStream support
* 2ca0d9c8ce ci: Sync claude-review workflow with systemd
* 1d1763f25a codespell: add "prFiles" to ignore-words-list
* 54b2897425 Revert "Symlink man pages on the website"
\
* 0c087181ed tests: fix pytest deprecation warning
* c9bbec224c docs: bump year in footer
* ae665678e2 Symlink man pages on the website
\
* 5895cd7596 Revert "Symlink man pages on the website"
* a09d5348ed docs: symlink man pages and assign them to the documentation heading
\
* 0bd378b7cb man: use YAML front matter
* 922e4a7f43 build(deps): bump aws-actions/configure-aws-credentials
* 05dfd295d3 initrd: add uas to the default list
* 2d52cc80db Pass through TERMINFO and TERMINFO_DIRS to tput (#4455)
* caa3c3235d Revert "dnf: Only read repositories and variables from the sandbox tree"
* 29b5f4d0d1 Add --umask to sandbox and UMask= setting
* d2e27ab62b Renaming remaining postmarketOS to Nura
Since c549d00e9f the tracking comment only lists the findings of the
latest run, so a re-review without new findings reports "No issues
found" even while earlier findings are still unresolved.
Rebuild the summary in the post job from the bot's review threads,
which it already fetches for thread resolution, as a checklist that
is checked once a thread is resolved. The validator additionally
returns the earlier findings it still sees at the head, so resolved
findings whose issue remains in the code are struck through as
dismissed. No extra model invocation is needed.
Signed-off-by: Paul Meyer <katexochen0@gmail.com>
The labeler converts a PR to a draft while README.md has the review
notice. Nothing else stops a merge. If the labeler fails, or somebody
merges before it runs, the PR goes in with the notice.
Let's add a linter job that fails when README.md has the notice. We keep
it out of the main lint job. That way it doesn't wait for the tools tree
build, and it can be made a required status check by itself.
Mechanical cleanup of function definitions under test/units/ and
.github/workflows/unit-tests.sh:
- Drop the 'function' keyword, i.e. 'function foo() { ... }' -> 'foo() { ... }'.
- Simplify functions whose body is a brace-wrapped subshell, i.e.
'foo() {( ... )}' -> 'foo() ( ... )'.
At high effort, Opus 5.5 uses about half as many thinking tokens per review as
Opus 5 did. The lenses return after about five of their twenty minutes. Use
xhigh effort, and tell the lenses to go through every hunk of the diff instead
of stopping at the first findings.
Pull requests from members of downstream organizations such as amutable
get no label that shows which organization they come from. Add a
downstream/<org> label when the author is a member of the organization.
The workflow token is not a member of the downstream organizations.
GitHub only shows public memberships to the token, so a member has to
make their membership public to get the label.
The review runs on Claude Opus 5. In Anthropic's testing, Claude Opus 5.5 scores
higher than Claude Opus 5 on coding evaluations. Switch the review to Claude
Opus 5.5.
With CLAUDE_CODE_SUBPROCESS_ENV_SCRUB set, Claude Code 2.1.220 lets the
sandbox write to /home. Every sandboxed Bash call then fails because bwrap
cannot create a placeholder at /home/.mcp.json. Bump to 2.1.288, which no
longer creates that placeholder. A sandboxed shell can also write to
$RUNNER_TEMP. There it can forge the results of other lenses, or set the
environment of later steps through $GITHUB_ENV. Deny sandbox writes to
$RUNNER_TEMP. Lenses are also killed in their final turn because the timeout
fires three minutes after the deadline. Give them eight minutes instead. The
summary says "No issues found" when lenses have failed. Make it say that the
review is incomplete. A lens also counts as done when all of its Bash calls
failed. Count it as failed instead. Agents cannot run git diff without a
working shell, so write the diff since the last review to a file.
Syncing a fork's main branch triggers the push workflows if actions are
enabled, which then run the full CI matrix and use up the fork owner's
Actions minutes for no benefit. Only skip push events so that PRs within
a fork still run CI, which is useful for testing actions.
Signed-off-by: Paul Meyer <katexochen0@gmail.com>
Let's maintain the list of subsystem maintainers
in CODEOWNERS, which github knows about and can do
useful stuff with like automatically adding those
usernames as reviewers.
* 117d382486 nura: use modern url for Alpine binary repos
* 7872fd58f7 nura: drop x86
* 3533864541 gitignore: add mkosi.pkgcache
* 580c4d94d7 Rename postmarketOS --> Nura
* ba96aba91e Bump version to 28~devel
* ecf7f5702b Release 27.1
* ca08e2ccef docs: add notes on nsresourced in execution flow
* 146b5dd4de docs: add a few notes on package manager specific behaviour
* 344e42ee20 Update NEWS
* dde1bf020f debian: fix debian-security/debug url when using a mirror
* 699aa3b565 mkosi: upgrade debug message about fallback path to info
* cb6da3f3ce mkosi: move fallback environment variable check to bypass nsresourced fully
* 88c716a6b9 config: add delegate_ranges
* f4de9a102a mkosi: use the foreign UID range only when requested when running as root
* d091fed134 mkosi: only delegate UID ranges if we're not already root
* 165bc99675 sandbox: rename delegate to delegate_ranges
* bba53c50d0 sandbox: use equality check instead of boolean not for delegate
* a033d03f59 summary: add foreign uid range
* f13e96eb95 user: drop unused variable SUBRANGE
* 847d113863 mkosi-obs: Only configure MakeScriptsExecutable in the main image
* 568b1bce56 dnf: Only read repositories and variables from the sandbox tree
* 938d6a1c21 obs: use policy signing data when available
* 19d045e5e9 Rename sysupdate configuration files to .transfer
* 9539f1771a action: bump fallback apk version to 3.0.8
* 8de4d25596 sandbox: add force_fallback to acquire_privileges and wire it up via env
* af97d6ffef opensuse: tolerate missing RPM keyring
* 0f2b7e0656 initrd: don't fail when encountering directories among ucode files
* 5240191893 Allow using SignInitrdPCRs with SplitArtifacts=pcrs
* ce90009686 Bump ukify required version from 261.999 to 262~ for SignInitrdPCRs
* 58bbd63aca news: fix v27 `DiskType=` setting typo
* 1da3299aec tests: skip test_fork_and_wait if on rc 1
* 5658e7abc9 Bump version to 28~devel
* 4736cd8361 Release 27
* 97f305bf0a action: make shell style consistent
* dd7140cbae action: only use apk version from API if non-empty
* 2cfe0cefb8 news: update NEWS
* 7bca598fa4 action: support container-based self-hosted runners
* 08c86a25e1 action: bump fallback APK version
* cc0fcef38f action: reduce timeouts and number of API calls
* 211993d5d2 action: download latest version of apk
| * f33cc45db3 Add ToolsTreeSnapshot= for pinning the default tools tree
| * ddfc5a2cb5 tests: join options and their values with f-strings
| * a497ff0251 doc: add missing matchers
* | 589226cfd4 apk: disable grub's update-grub trigger during package installation
/
| * 595451bb4b debian: add new split-out systemd packages
| * 7ce1851901 Revert "mkosi-tools: Install systemd-tpm on Debian"
| * 50a1137be5 ci: pin resolved release for integration tests
| * e30b0a0ba4 tests: drop obsolete @ prefix
| * e7f4d57fac news: update NEWS
|/
* bf2addb2ec build(deps): bump aws-actions/configure-aws-credentials
* a58ded6a2a build(deps): bump actions/checkout from 6.0.2 to 7.0.1
* 9b7d87a170 add support for prebuilt UKIs
* 75fdb9be15 mkosi-tools: Install systemd-tpm on Debian
* d5ff0d0d98 Fall back to unprivileged user namespace on polkit denial
* 0e4d5546ac Bump 261~devel version checks to 261
* 180d4a5329 Use systemd nightly OBS version in signing PCR policies auto detection
* f7401bdc8d Add support for signing PCR policies for the initrd
* 33d17b2b92 sandbox: Don't create shared propagation peer group
* a078088f8e sandbox: Declare MS_PRIVATE as well
* e1e9eafc87 kmod: eliminate duplication in calls to modinfo
* df68b27e8e tools: add mount to Debian tools tree
* 205dcedd45 util: don't drop single-character variable names in read_env_file
* 35a9d38ea0 distribution: report the actual architecture in unsupported-arch errors
* d4f44d212c sandbox: handle case where dev node for tty doesn't exist better
* acf08c5eca build(deps): bump actions/upload-artifact from 7.0.0 to 7.0.1
* b3ec53743a tests: Pass --output-directory to vm() and boot()
* 9bc430a7fa config: Isolate build history per output directory
* fd3d9bfe57 tests: Add regression test for build history lookup via configured output dir
| * 55ab00bb13 Fix tar exclude patterns for apivfs paths
|/
* 699da862f8 sandbox: Split a chroot() context manager out of chase()
* 70ce10d132 sandbox: Default FSOperation.execute oldroot/newroot to /
* 610b38538f Forbid AI attribution in commit messages
* b7425bbe42 ci: Sync claude-review workflow with systemd
* b9d4c06239 action: Make apk installation configurable and reliable again
actions/labeler applies path based labels by reading the pull request's full
label set and writing it back with setLabels(). It re-reads the labels just
before writing, but the window is still open.
Match the configured patterns in the workflow instead and apply them with
addLabels() and removeLabel(), which only ever touch the labels the step
decided on. The configuration moves to .github/labeler.json so parsing needs
nothing github-script does not already provide, and "*" now means any run of
characters, "/" included. That also makes the patterns say what they looked
like they said: "src/home*" matched nothing below src/home/, so the homed
label was dead.
Crossing the too-many-labels limit in either direction takes the other
outcome's labels back off, so a pull request cannot end up carrying tree-wide
and a handful of subsystem labels at the same time.
The pull_request trigger is there to check a configuration change against the
head copy, so it no longer writes anything: a fork only ever got a read-only
token, and pull_request_target runs for the same pull request, so the two would
fight over the labels. Drop its paths-ignore in turn, because it only kept
pull_request_target away when the pull request changed nothing else, and a
configuration change bundled with other files left nobody doing the writing.
The hook that enforces the review deadline used bash's [[ test, but hook
commands run under /bin/sh, which is dash on the runners. Every tool call was
denied with "[[: not found", so all six lenses returned empty findings and
the run reported a clean review in 73 seconds. Use a POSIX test.
An empty result from an agent that never got a tool call through looks exactly
like a clean review, so require at least one successful tool call before a lens
counts as completed. A run like that now fails every lens, which also stops it
from raising the review count.
The "Pull Request Labeler" workflow read the full label set of a pull
request and wrote it back with setLabels(), which is a full replace. Other
workflows label the same pull request at the same time: claude-review.yml
adds the claude-review label on the pull_request_target "opened" event,
which is the very same event that starts the labeler. Whenever that add
landed between the labeler's listLabelsOnIssue() and its setLabels(), the
label was dropped again and the review never ran.
Only ever add or remove the individual labels the labeler owns, so
concurrent changes by other workflows survive. removeLabel() returns 404
when the label is already gone, which is exactly what a racing run causes,
so swallow that status.
Reviewers ignored the tool call budget in their prompt, and the budget was a
poor stand-in for what matters anyway: per-call latency varied from 25 to
85 seconds between lenses, so the same count meant 7 minutes for one and 20
for another. Two lenses hit the timeout and lost everything they had found.
Enforce the deadline itself. run_claude exports it per invocation and a
PreToolUse hook denies further tool calls once it has passed, telling the
model to return what it has. The structured output call stays allowed, so
a slow lens still produces findings. The timeout, three minutes later, only
catches a hang. Give reviewers 22 minutes and the validator 12.
AI agents prepend a notice to README.md that the author removes once they
have reviewed the changes. Until that happens nobody else should look at
the PR: convert it to a draft, don't add the please-review label, and
don't start a Claude review. The labeler and the review workflow check
README.md at the PR head for the notice. Marking the PR ready for review
after removing the notice triggers the normal labeling again.
A triple backtick inside prose made the fence count odd, so a body with an
inline marker plus a valid suggestion block got cut at the block's closing
fence and left it open. Only fences at the start of a line delimit blocks,
so count and strip those alone.
Reviews keep hitting the 50 minute timeout. The coordinator agent that was
supposed to launch reviewers, watch the clock, stop stragglers, validate and
write the result does none of that reliably: reviewers get stopped and lose
their findings, foreground validators can't be stopped, deadlines get
overshot by minutes, and one leftover agent keeps the process alive until
the job fails. Piling more rules into its prompt hasn't helped.
Do the orchestration in the shell instead. Run one bounded claude invocation
per review lens in parallel under timeout(1), each returning its findings as
structured output, merge them with jq and run one bounded validator over the
candidates. A lens that times out is just reported as failed. The post job
builds the tracking comment from what it actually posted, so the model no
longer authors the summary or the review bookkeeping. Agents run from an
empty directory with all inputs outside it, so the sandbox keeps both the
checkout and the inputs read-only.
The Claude review workflow posted long, padded inline comments, and every
force push produced a fresh batch of new findings on code that had not
changed. Both make authors stop reading the reviews.
Add a comment style section to the prompt that limits each comment to the
problem and the obvious fix, and truncate anything over 1000 characters in
the post job. Track the number of completed reviews and the last reviewed
head in a hidden state line in the tracking comment. Each successful review
narrows the severities that may be posted: everything on the first review,
no nits on the second, and only must-fix findings from the third on. The
post job enforces the policy so it holds even if the model ignores the
prompt, and reviewers are told to range-diff against the previous head and
skip non-must-fix findings on unchanged code. Commenting "@claude review
full" resets the count for that run.
Zbigniew Jędrzejewski-Szmek