SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Two claude fixes/improvements (#40971)

Browse Source
This commit is contained in:
Daan De Meyer
2026-03-06 15:18:26 +01:00
committed by GitHub
parent dfb1fa0197 5d223ff468
commit d2fec49b55
4 changed files with 164 additions and 209 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/claude-review.yml
+151
View File
@@ -0,0 +1,151 @@
# Integrates Claude Code as an AI assistant for reviewing pull requests.
# Mention @claude in any PR review to request a review. Claude authenticates
# via AWS Bedrock using OIDC — no long-lived API keys required.
name: Claude Review
on:
pull_request_review_comment:
types: [created]
pull_request_review:
types: [submitted]
concurrency:
group: claude-review-${{ github.event.pull_request.number }}
cancel-in-progress: true
jobs:
claude-review:
runs-on: ubuntu-latest
if: |
github.repository_owner == 'systemd' &&
(github.event_name == 'pull_request_review_comment' &&
contains(github.event.comment.body, '@claude') &&
contains(fromJSON('["MEMBER","OWNER","COLLABORATOR"]'), github.event.comment.author_association)) ||
(github.event_name == 'pull_request_review' &&
contains(github.event.review.body, '@claude') &&
contains(fromJSON('["MEMBER","OWNER","COLLABORATOR"]'), github.event.review.author_association))
permissions:
contents: read # Read repository contents
pull-requests: write # Post comments and reviews on PRs
id-token: write # Authenticate with AWS via OIDC
actions: read # Access workflow run metadata
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd
- name: Configure AWS credentials
uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7
with:
role-to-assume: arn:aws:iam::${{ secrets.AWS_ACCOUNT_ID }}:role/${{ secrets.AWS_ROLE_NAME }}
role-session-name: GitHubActions-Claude-${{ github.run_id }}
aws-region: us-east-1
- name: Run Claude Code
uses: anthropics/claude-code-action@1fc90f3ed982521116d8ff6d85b948c9b12cae3e
with:
use_bedrock: "true"
github_token: ${{ secrets.GITHUB_TOKEN }}
claude_args: |
--model us.anthropic.claude-opus-4-6-v1
--max-turns 100
--allowedTools "
Read,Write,Edit,MultiEdit,LS,Grep,Glob,Task,
Bash(cat:*),Bash(test:*),Bash(printf:*),Bash(jq:*),Bash(head:*),Bash(git:*),Bash(gh:*),
mcp__github_inline_comment__create_inline_comment,
"
prompt: |
REPO: ${{ github.repository }}
PR NUMBER: ${{ github.event.pull_request.number }}
HEAD SHA: ${{ github.event.pull_request.head.sha }}
Review this pull request.
You are in the upstream repo without the patch applied. Do not apply it.
## Phase 1: Gather context
Fetch the patch, PR title/body, and list of existing comments (top-level, inline, and reviews):
- `gh pr diff ${{ github.event.pull_request.number }} --patch`
- `gh pr view ${{ github.event.pull_request.number }} --json title,body`
- `gh api --paginate repos/${{ github.repository }}/issues/${{ github.event.pull_request.number }}/comments`
- `gh api --paginate repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/comments`
- `gh api --paginate repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/reviews`
## Phase 2: Parallel review subagents
Review:
- Code quality, style, and best practices
- Potential bugs, issues, incorrect logic
- Security implications
- CLAUDE.md - compliance
For every category, launch subagents to review them in parallel. Group related sections
as needed — use 2-4 subagents based on PR size and scope.
Give each subagent the PR title, description, full patch, and the list of changed files.
Each subagent must return a JSON array of issues:
`[{"file": "path", "line": <number or null>, "severity": "must-fix|suggestion|nit", "title": "...", "body": "..."}]`
Subagents must ONLY return the JSON array — they must NOT post comments,
call `gh`, or use `mcp__github_inline_comment__create_inline_comment`.
All posting happens in Phase 3.
Each subagent MUST verify its findings before returning them:
- For style/convention claims, check at least 3 existing examples in the codebase to confirm
the pattern actually exists before flagging a violation.
- For "use X instead of Y" suggestions, confirm X actually exists and works for this case.
- If unsure, don't include the issue.
## Phase 3: Collect and post
After ALL subagents complete:
1. Collect all issues. Merge duplicates (same file, lines within 3 of each other, same problem).
2. Drop low-confidence findings.
3. For CLAUDE.md violations that appear in 3+ existing places in the codebase, do NOT post inline comments.
Instead, add them to the 'CLAUDE.md improvements' section of the tracking comment
4. Check existing inline review comments (fetched in Phase 1). Do NOT post an inline comment if
one already exists on the same file+line about the same problem.
5. Check for author replies that dismiss or reject a previous comment. Do NOT re-raise an issue
the PR author has already responded to disagreeing with.
6. Post new inline comments with `mcp__github_inline_comment__create_inline_comment`.
Prefix ALL comments with "Claude: ".
Link format: https://github.com/${{ github.repository }}/blob/${{ github.event.pull_request.head.sha }}/README.md#L10-L15
Then maintain a single top-level "tracking comment" listing ALL issues as checkboxes.
Use a hidden HTML marker to find it: `<!-- claude-pr-review -->`.
Look through the top-level comments fetched in Phase 1 for one containing that marker.
**If no tracking comment exists (first run):**
Create one with `gh pr comment ${{ github.event.pull_request.number }} --body "..."` using this format:
```
Claude: review of <REPO> #<PR NUMBER> (<HEAD SHA>)
<!-- claude-pr-review -->
### Must fix
- [ ] **title** — `file:line` — short explanation
### Suggestions
- [ ] **title** — `file:line` — short explanation
### Nits
- [ ] **title** — `file:line` — short explanation
### CLAUDE.md improvements
- improvement suggestion
```
Omit empty sections.
**If a tracking comment already exists (subsequent run):**
1. Parse the existing checkboxes. For each old issue, check if the current patch still has
that problem (re-check the relevant lines in the new diff). If fixed, mark it `- [x]`.
If the author dismissed it, mark it `- [x] ~~title~~ (dismissed)`.
2. Append any NEW issues found in this run that aren't already listed.
3. Update the HEAD SHA in the header line.
4. Edit the comment in-place.
```
printf '%s' "$BODY" > pr-review-body.txt
gh api --method PATCH repos/${{ github.repository }}/issues/comments/<comment-id> -F body=@pr-review-body.txt
```
.github/workflows/claude.yml
-62
View File
@@ -1,62 +0,0 @@
# Integrates Claude Code as an AI assistant for issues and pull requests.
# Mention @claude in any issue comment, PR review comment, or PR review to
# interact with it, or assign the "claude" user to an issue. Claude
# authenticates via AWS Bedrock using OIDC — no long-lived API keys required.
name: Claude Code
on:
issue_comment:
types: [created]
pull_request_review_comment:
types: [created]
issues:
types: [opened, assigned]
pull_request_review:
types: [submitted]
jobs:
claude:
runs-on: ubuntu-latest
if: |
github.repository_owner == 'systemd' &&
((github.event_name == 'issue_comment' &&
contains(github.event.comment.body, '@claude') &&
contains(fromJSON('["MEMBER","OWNER","COLLABORATOR"]'), github.event.comment.author_association)) ||
(github.event_name == 'pull_request_review_comment' &&
contains(github.event.comment.body, '@claude') &&
contains(fromJSON('["MEMBER","OWNER","COLLABORATOR"]'), github.event.comment.author_association)) ||
(github.event_name == 'pull_request_review' &&
contains(github.event.review.body, '@claude') &&
contains(fromJSON('["MEMBER","OWNER","COLLABORATOR"]'), github.event.review.author_association)) ||
(github.event_name == 'issues' &&
github.event.action == 'assigned' &&
github.event.assignee.login == 'claude'))
permissions:
contents: read # Read repository contents
issues: write # Post comments on issues
pull-requests: write # Post comments and reviews on PRs
id-token: write # Authenticate with AWS via OIDC
actions: read # Access workflow run metadata
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd
with:
fetch-depth: 1
- name: Configure AWS credentials
uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7
with:
role-to-assume: arn:aws:iam::${{ secrets.AWS_ACCOUNT_ID }}:role/${{ secrets.AWS_ROLE_NAME }}
role-session-name: GitHubActions-Claude-${{ github.run_id }}
aws-region: us-east-1
- name: Run Claude Code
uses: anthropics/claude-code-action@1fc90f3ed982521116d8ff6d85b948c9b12cae3e
with:
use_bedrock: "true"
github_token: ${{ secrets.GITHUB_TOKEN }}
claude_args: |
--model us.anthropic.claude-opus-4-6-v1
.gitignore
+1
View File
@@ -36,3 +36,4 @@ __pycache__/
.vscode/
/pkg/
.aider*
/worktrees
AGENTS.md
+12 -147
View File
@@ -1,10 +1,7 @@
# AGENTS.md
This file provides guidance to AI coding agents when working with code in this repository.
## Project Overview
systemd is a system and service manager for Linux, written in C (GNU17 with extensions). The project is built with Meson and consists of ~140 components including PID 1, journald, udevd, networkd, and many other system daemons.
This file provides guidance to AI coding agents when working with code in this repository. Only add
instructions to this file if you've seen an AI agent mess up that particular bit of logic in practice.
## Key Documentation
@@ -17,154 +14,22 @@ Always consult these files as needed:
## Build and Test Commands
**CRITICAL: Read and follow these instructions exactly.**
- Never compile individual files or targets. Always run `mkosi -f box -- meson compile -C build` to build
the entire project. Meson handles incremental compilation automatically.
- Never run `meson compile` followed by `meson test` as separate steps. Always run
`mkosi -f box -- meson test -C build -v <TEST-NAME>` directly. Meson will automatically rebuild any required
targets before running tests.
- Never invent your own build commands or try to optimize the build process.
- Never use `head`, `tail`, or pipe (`|`) the output of build or test commands. Always let the full output
display. This is critical for diagnosing build and test failures.
- **NEVER** compile individual files or targets. **ALWAYS** run `mkosi -f box -- meson compile -C build` to build the entire project. Meson handles incremental compilation automatically.
- **NEVER** run `meson compile` followed by `meson test` as separate steps. **ALWAYS** run `mkosi -f box -- meson test -C build -v <TEST-NAME>` directly. Meson will automatically rebuild any required targets before running tests.
- **NEVER** invent your own build commands or try to optimize the build process.
- **NEVER** use `head`, `tail`, or pipe (`|`) the output of build or test commands. Always let the full output display. This is critical for diagnosing build and test failures.
## Integration Tests
```sh
# Initial setup (one-time)
mkosi -f genkey
mkosi -f box -- meson setup build
# Build everything
mkosi -f box -- meson compile -C build
# Run a specific unit test (also rebuilds as needed)
mkosi -f box -- meson test -C build -v <TEST-NAME>
# Run all unit tests
mkosi -f box -- meson test -C build --print-errorlogs -q
# Build and boot an OS image for integration testing
mkosi -f box -- meson compile -C build mkosi
mkosi boot # nspawn
mkosi vm # qemu
```
- **CORRECT**: `mkosi -f box -- meson test -C build -v <TEST-NAME>`
- **WRONG**: Separate compile then test steps
- **WRONG**: `mkosi -f box -- meson compile -C build src/core/systemd` (individual target)
- **WRONG**: `mkosi -f box -- meson test -C build -v <TEST-NAME> 2>&1 | tail -100` (piped output)
## Code Architecture
### Shared Code Dependency Hierarchy (strict layering)
```
src/fundamental/ → no dependencies (used in EFI + userspace)
↑
src/basic/ → depends only on fundamental (userspace only)
↑
src/libsystemd/ → depends on basic + fundamental (public libsystemd.so)
↑
src/shared/ → depends on all above (libsystemd-shared-<nnn>.so)
↑
src/<component>/ → individual daemons and tools
```
Code should be linked as few times as possible. Place shared code at the lowest possible layer.
### Key Components
- `src/core/` — PID 1 service manager (system and user instances). Uses `systemd-executor` for process spawning via `posix_spawn()` to avoid fork+exec pitfalls.
- `src/udev/` — udev daemon and udevadm tool
- `src/journal/` — journald logging daemon
- `src/network/` — networkd network manager
- `src/resolve/` — resolved DNS resolver
- `src/login/` — logind session manager
- `src/boot/` — systemd-boot EFI bootloader
- `src/systemctl/`, `src/journalctl/`, `src/analyze/` — CLI tools
### Unit Settings Implementation
Adding a new unit setting requires changes in various places:
1. `src/core/load-fragment-gperf.gperf.in` + `src/core/load-fragment.c` — unit file parsing
1. `src/core/dbus-*.c` — D-Bus interface
1. `src/core/varlink-*.c` — Varlink interface
1. `src/shared/bus-unit-util.c` — client-side parsing for systemctl/systemd-run
1. `test/fuzz/fuzz-unit-file/` — add to fuzz corpus
### Tests
- **Unit tests**: `src/test/` — match source files (e.g., `src/test/test-path-util.c` tests `src/basic/path-util.c`). Use assertion macros from `tests.h` (`ASSERT_GE()`, `ASSERT_OK()`, `ASSERT_OK_ERRNO()` for glibc calls).
- **Fuzzers**: `src/fuzz/` for basic/shared code; next to component code otherwise. Input samples in `test/fuzz/`.
- **Integration tests**: `test/TEST-*` directories, run via systemd-nspawn or qemu.
## Coding Style (Essential Rules)
The full style guide is in `docs/CODING_STYLE.md`. Key rules:
### Formatting
- 8-space indent (no tabs) for C; 4-space for shell scripts; 2-space for man pages (XML)
- ~109 character line limit
- Opening brace on same line: `void foo() {`
- Function parameters with double indent (16 spaces) when broken across lines
- Single-line `if` blocks without braces
- `/* comments */` for committed code; `//` reserved for temporary debug comments
- Pointer in return types: `const char* foo()` (star with type)
- Pointer in parameters: `const char *input` (star with name)
- Casts: `(const char*) s` (space before `s`, not after `*`)
### Naming and Structure
- Structs: `PascalCase`; variables/functions: `snake_case`
- Return parameters prefixed `ret_` (success) or `reterr_` (failure)
- Command-line globals prefixed `arg_`
- Enum flags: use `1 << N` expressions, aligned vertically
- Non-flag enums: include `_FOO_MAX` and `_FOO_INVALID = -EINVAL` sentinels
- Commit messages: prefix with component name (e.g., `journal: `, `nspawn: `)
### Error Handling
- Return negative errno values: `return -EINVAL`
- Use `RET_NERRNO()` to convert libc-style returns
- Combined log+return: `return log_error_errno(r, "Failed to ...: %m")`
- Use `SYNTHETIC_ERRNO()` for errors not from a called function
- Cast ignored errors to void: `(void) unlink("/foo/bar")`
- Always check OOM; use `log_oom()` in program code
### Header Files
- Keep headers lean — prefer implementations in `.c` files
- Include the appropriate forward declaration header (`basic-forward.h`, `sd-forward.h`, `shared-forward.h`) instead of pulling in full headers
- Order: external headers (`<>`), then `sd-*` headers, then internal headers, alphabetically within each group
- No circular header dependencies
### Memory and Types
- Use `_cleanup_free_` and friends for automatic cleanup
- Use `alloca_safe()`, `strdupa_safe()` instead of raw `alloca()`, `strdupa()`
- Never use `off_t`; use `uint64_t` instead
- Use `unsigned` not `unsigned int`; `uint8_t` for bytes, not `char`
### Functions to Avoid
- `memset` → use `memzero()` or `zero()`
- `strcmp`/`strncmp` → use `streq()` / `strneq()`
- `strtol`/`atoi` → use `safe_atoli()`, `safe_atou32()`, etc.
- `basename`/`dirname` → use `path_extract_filename()` / `path_extract_directory()`
- `fgets` → use `read_line()`
- `exit()` → use `return` from main or `_exit()` in forked children
- `dup()` → use `fcntl(fd, F_DUPFD_CLOEXEC, 3)`
- `htonl`/`htons` → use `htobe32()` / `htobe16()`
### File Descriptors
- Always use `O_CLOEXEC` / `SOCK_CLOEXEC` / `MSG_CMSG_CLOEXEC` / ...
### Logging
- Library code (`src/basic/`, `src/shared/`): never log (except `LOG_DEBUG`)
- "Logging" functions log errors themselves; "non-logging" functions expect callers to log
- Non-fatal warnings: suffix with `, ignoring: %m"` or similar
### Integration Tests
- Never use `grep -q` in pipelines; use `grep >/dev/null` instead (avoids `SIGPIPE`)
## Pull Request Review Instructions
- Focus on coding style compliance from `docs/CODING_STYLE.md`
- Only leave comments for logic issues if you are very confident in your deduction
- Frame comments as questions
- Always consider you may be wrong
- Do not argue with contributors; assume they are right unless you are very confident
- Be extremely thorough — every single coding style violation should be reported
- Always check out the PR in a git worktree in `worktrees/`, review it locally and remove the worktree when finished.
## AI Contribution Disclosure