diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml new file mode 100644 index 0000000000..bba7bdd1b5 --- /dev/null +++ b/.github/workflows/claude-review.yml @@ -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": , "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: ``. + 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 # () + + + + ### 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/ -F body=@pr-review-body.txt + ``` diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml deleted file mode 100644 index 79762a5d7c..0000000000 --- a/.github/workflows/claude.yml +++ /dev/null @@ -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 diff --git a/.gitignore b/.gitignore index 5c5f1ed586..a6aec32496 100644 --- a/.gitignore +++ b/.gitignore @@ -36,3 +36,4 @@ __pycache__/ .vscode/ /pkg/ .aider* +/worktrees diff --git a/AGENTS.md b/AGENTS.md index 6d57417555..e69b25a076 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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 ` 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 ` 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 - -# 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 ` -- **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 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-.so) - ↑ -src// → 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