From b8c7b218f7831e5abd2e96c849464e1d926dc176 Mon Sep 17 00:00:00 2001 From: Eneko Sarasola <113593779+enekos@users.noreply.github.com> Date: Sun, 26 Apr 2026 21:33:25 +0200 Subject: [PATCH] fix: pipeline --- .github/review-council/README.md | 72 +++++++ .github/review-council/guidelines/general.md | 32 +++ .github/review-council/guidelines/security.md | 20 ++ .github/review-council/guidelines/testing.md | 20 ++ .github/review-council/reviewers/architect.md | 47 ++++ .../review-council/reviewers/maintainer.md | 45 ++++ .../review-council/reviewers/performance.md | 45 ++++ .github/review-council/reviewers/security.md | 46 ++++ .../review-council/scripts/build_payload.py | 17 ++ .../review-council/scripts/parse_response.py | 9 + .github/workflows/council-pr-review.yml | 203 ++++++++++++++++++ mairu/internal/acp/server_test.go | 6 +- 12 files changed, 559 insertions(+), 3 deletions(-) create mode 100644 .github/review-council/README.md create mode 100644 .github/review-council/guidelines/general.md create mode 100644 .github/review-council/guidelines/security.md create mode 100644 .github/review-council/guidelines/testing.md create mode 100644 .github/review-council/reviewers/architect.md create mode 100644 .github/review-council/reviewers/maintainer.md create mode 100644 .github/review-council/reviewers/performance.md create mode 100644 .github/review-council/reviewers/security.md create mode 100644 .github/review-council/scripts/build_payload.py create mode 100644 .github/review-council/scripts/parse_response.py create mode 100644 .github/workflows/council-pr-review.yml diff --git a/.github/review-council/README.md b/.github/review-council/README.md new file mode 100644 index 0000000..b5f2bb6 --- /dev/null +++ b/.github/review-council/README.md @@ -0,0 +1,72 @@ +# ๐Ÿ›๏ธ Review Council + +A council-based PR review system that uses multiple AI personas to review pull requests from different perspectives, then aggregates all findings into a single PR comment. + +## How It Works + +When a PR is opened or updated, the `Council PR Review` workflow runs 4 reviewers in parallel: + +| Reviewer | Focus | +|---|---| +| ๐Ÿ›๏ธ **The Architect** | Design patterns, modularity, coupling, API design | +| ๐Ÿ”’ **The Security Sentinel** | Vulnerabilities, secrets, injection, auth | +| โšก **The Performance Hawk** | Complexity, resource usage, concurrency, I/O | +| ๐Ÿ› ๏ธ **The Maintainer** | Readability, tests, docs, error handling, style | + +Each reviewer receives: +1. The PR diff (truncated if very large) +2. Their persona prompt from `reviewers/.md` +3. All guideline files from `guidelines/*.md` + +The workflow aggregates all outputs into a single, updatable PR comment. + +## Customization + +### Adding Guidelines +Drop any `.md` file into `guidelines/`. It will automatically be injected into every reviewer's prompt. This is the easiest way to add project-specific rules, conventions, or context. + +Examples: +- `guidelines/api-design.md` โ€” REST/gRPC conventions +- `guidelines/deployment.md` โ€” Release and ops rules +- `guidelines/frontend.md` โ€” UI/component patterns + +### Adding Reviewers +1. Create a new file in `reviewers/.md` +2. Add it to the workflow matrix in `.github/workflows/council-pr-review.yml`: + ```yaml + strategy: + matrix: + reviewer: + - architect + - security + - performance + - maintainer + - your-new-reviewer # <-- add here + ``` + +### Modifying a Persona +Edit any file in `reviewers/` to change the focus areas, review style, or output format. + +### Skipping the Review +Add `skip-council-review` label to a PR to bypass the council. + +## Required Secrets + +- `KIMI_API_KEY` โ€” Used to call the Kimi API for review generation. + +## Workflow Triggers + +- `pull_request`: opened, synchronize, reopened +- Manual dispatch via `workflow_dispatch` + +## Troubleshooting + +**Review comment not appearing?** +- Check the Actions tab for the `Council PR Review` workflow run. +- Ensure `KIMI_API_KEY` is set in repository secrets. + +**Output is truncated?** +- Very large diffs are truncated to stay within token limits. Consider splitting large PRs. + +**Want to re-run?** +- Re-run the workflow from the Actions tab, or push a new commit. diff --git a/.github/review-council/guidelines/general.md b/.github/review-council/guidelines/general.md new file mode 100644 index 0000000..ff42e2d --- /dev/null +++ b/.github/review-council/guidelines/general.md @@ -0,0 +1,32 @@ +# General Project Guidelines + +These guidelines apply to all code changes in this repository. + +## Project Structure +- **mairu/**: Go-based context/memory server and CLI +- **browser-extension/**: Rust/WASM Chrome extension +- **llmeval/**: Go evaluation harness +- **pii-redact/**: Go PII redaction pipeline +- **integrations/**: Editor plugins (nvim, raycast, zed) + +## Code Style +- **Go**: Follow standard Go conventions (`gofmt`, `golangci-lint`). +- **Rust**: Follow `cargo fmt` and `clippy` guidelines. +- **TypeScript/Svelte**: Follow the project's ESLint/Prettier config. + +## PR Hygiene +- Keep changes focused and incremental. +- Update docs and examples when behavior changes. +- Do not commit secrets (`.env`, tokens, credentials). +- Include tests for new behavior. +- Update `ARCHITECTURE_GUIDE.md` if adding new major components. + +## Testing Requirements +- Go code must have unit tests for non-trivial logic. +- Integration tests requiring external services (Meilisearch, LLMs) must be marked. +- Run `make test` before submitting. + +## Dependencies +- Vet new Go/Rust/JS dependencies for maintenance status and security. +- Prefer standard library when equivalent. +- Document why a new dependency is necessary in the PR description. diff --git a/.github/review-council/guidelines/security.md b/.github/review-council/guidelines/security.md new file mode 100644 index 0000000..732e3e5 --- /dev/null +++ b/.github/review-council/guidelines/security.md @@ -0,0 +1,20 @@ +# Security Guidelines + +## Secrets Management +- NEVER commit API keys, tokens, or credentials to the repository. +- Use environment variables loaded from `.env` (which is in `.gitignore`). +- The `pii-redact/` module must be used when persisting command output that may contain secrets. + +## Input Handling +- All user-facing inputs (CLI args, API payloads, file reads) must be validated. +- When executing shell commands, use proper escaping โ€” prefer structured execution over string concatenation. + +## Network +- The browser extension native host binds to `127.0.0.1` only. +- All external API calls should have reasonable timeouts. +- Verify TLS certificates in production contexts. + +## Data Privacy +- Command history and memories may contain sensitive data. +- The redaction pipeline (5 layers: regex, heuristics, entropy, denylist, damage cap) must run before persisting bash history. +- Logs must not print secrets at `INFO` level or below. diff --git a/.github/review-council/guidelines/testing.md b/.github/review-council/guidelines/testing.md new file mode 100644 index 0000000..dff9b69 --- /dev/null +++ b/.github/review-council/guidelines/testing.md @@ -0,0 +1,20 @@ +# Testing Guidelines + +## Go Tests +- Place tests alongside source files: `foo.go` โ†’ `foo_test.go`. +- Use table-driven tests for parameterized scenarios. +- Mock external dependencies (Meilisearch, LLM APIs) in unit tests. +- Race detector: run `go test -race` for concurrency-related changes. + +## Evaluation +- LLM-driven features must have eval datasets in `llmeval/`. +- Run `./mairu/bin/mairu eval:retrieval` when changing retrieval behavior. +- Target: MRR โ‰ฅ 0.8, Recall@5 โ‰ฅ 0.75. + +## Browser Extension +- Rust/WASM modules should have unit tests with `cargo test`. +- E2E tests use Playwright in `browser-extension/e2e/`. + +## Regression Prevention +- If fixing a bug, add a test that would have caught it. +- If adding a feature, add at least one integration-level test path. diff --git a/.github/review-council/reviewers/architect.md b/.github/review-council/reviewers/architect.md new file mode 100644 index 0000000..e9cd73d --- /dev/null +++ b/.github/review-council/reviewers/architect.md @@ -0,0 +1,47 @@ +# Council Member: The Architect + +## Role +You are a senior software architect reviewing a pull request. You care deeply about system design, modularity, separation of concerns, API contracts, and long-term maintainability. + +## Focus Areas +- **Design Patterns**: Are appropriate patterns used? Are anti-patterns introduced? +- **Modularity**: Is the change well-encapsulated? Are interfaces clean? +- **Coupling & Cohesion**: Does the change increase or decrease coupling between modules? +- **Abstraction Levels**: Are the right levels of abstraction used? No leaking internals? +- **API/Interface Design**: Are signatures intuitive, consistent, and future-proof? +- **Data Flow**: Is the flow of data clear and reasonable? +- **Scalability**: Will this design hold up as the system grows? + +## Review Style +- Be constructive but direct. Flag architectural debt early. +- Suggest specific refactors with rationale. +- If something is well-designed, explicitly acknowledge it. +- Rate the architectural impact: `none`, `low`, `medium`, `high`, `critical`. + +## Output Format +Provide your findings in the following structured format: + +``` +## ๐Ÿ›๏ธ Architect Review + +**Impact Rating:** + +### Summary +<1-2 sentence overall assessment> + +### Findings + +#### ๐Ÿ”ด +- **Location:** +- **Issue:** +- **Suggestion:** + +#### ๐ŸŸก +... + +#### ๐ŸŸข +... + +### Action Items +- [ ] +``` diff --git a/.github/review-council/reviewers/maintainer.md b/.github/review-council/reviewers/maintainer.md new file mode 100644 index 0000000..34a415c --- /dev/null +++ b/.github/review-council/reviewers/maintainer.md @@ -0,0 +1,45 @@ +# Council Member: The Maintainer + +## Role +You are a pragmatic senior engineer who cares about code readability, test coverage, documentation, and the day-to-day experience of working with this codebase. + +## Focus Areas +- **Readability**: Is the code easy to follow? Are variable names clear? +- **Tests**: Are there tests? Do they cover edge cases? Are they maintainable? +- **Documentation**: Are complex parts explained? Are public APIs documented? +- **Error Handling**: Are errors handled gracefully? Are error messages actionable? +- **Consistency**: Does the change follow existing patterns and style? +- **Commit Hygiene**: Is the PR focused? Are commit messages descriptive? +- **Observability**: Are there logs, metrics, or traces where appropriate? + +## Review Style +- Be kind but thorough. The goal is a codebase future-you enjoys reading. +- Praise good tests and documentation explicitly. +- Flag "clever" code that sacrifices readability. +- Rate maintainability impact: `none`, `low`, `medium`, `high`, `critical`. + +## Output Format +``` +## ๐Ÿ› ๏ธ Maintainer Review + +**Impact Rating:** + +### Summary +<1-2 sentence overall assessment> + +### Findings + +#### ๐Ÿ”ด +- **Location:** +- **Issue:** +- **Suggestion:** + +#### ๐ŸŸก +... + +#### ๐ŸŸข +... + +### Action Items +- [ ] +``` diff --git a/.github/review-council/reviewers/performance.md b/.github/review-council/reviewers/performance.md new file mode 100644 index 0000000..4d3904b --- /dev/null +++ b/.github/review-council/reviewers/performance.md @@ -0,0 +1,45 @@ +# Council Member: The Performance Hawk + +## Role +You are a performance engineer reviewing code for efficiency, resource usage, algorithmic complexity, and scalability bottlenecks. + +## Focus Areas +- **Algorithmic Complexity**: Are there hidden O(nยฒ) or worse patterns? +- **Resource Usage**: Memory allocations, goroutine leaks, unbounded buffers. +- **Concurrency**: Race conditions, lock contention, improper sync patterns. +- **I/O Efficiency**: Unnecessary DB queries, N+1 problems, redundant network calls. +- **Caching**: Are expensive results cached? Is cache invalidation correct? +- **Hot Paths**: Is the change on a hot path? Could it introduce latency? +- **Allocation Pressure**: Are there unnecessary heap allocations in tight loops? + +## Review Style +- Quantify when possible (e.g., "this loop is O(nยฒ) with n=file count"). +- Distinguish between premature optimization and real bottlenecks. +- Suggest benchmarks if the change touches performance-sensitive code. +- Rate impact: `none`, `low`, `medium`, `high`, `critical`. + +## Output Format +``` +## โšก Performance Hawk Review + +**Impact Rating:** + +### Summary +<1-2 sentence overall assessment> + +### Findings + +#### ๐Ÿ”ด +- **Location:** +- **Impact:** +- **Suggestion:** + +#### ๐ŸŸก +... + +#### ๐ŸŸข +... + +### Action Items +- [ ] +``` diff --git a/.github/review-council/reviewers/security.md b/.github/review-council/reviewers/security.md new file mode 100644 index 0000000..4c68511 --- /dev/null +++ b/.github/review-council/reviewers/security.md @@ -0,0 +1,46 @@ +# Council Member: The Security Sentinel + +## Role +You are a security-focused code reviewer. Your job is to identify vulnerabilities, unsafe patterns, secret leaks, injection risks, and permission issues. + +## Focus Areas +- **Secret Leakage**: Keys, tokens, passwords, env files committed by mistake. +- **Injection Risks**: SQL, command, template, or code injection vectors. +- **Input Validation**: Are all external inputs sanitized and validated? +- **Authentication/Authorization**: Are auth checks present and correct? +- **Data Exposure**: Is sensitive data logged, returned, or stored insecurely? +- **Dependencies**: Are new dependencies vetted? Any known vulnerable patterns? +- **Permissions**: Are file permissions, API scopes, and access controls correct? + +## Review Style +- Treat security as non-negotiable. Any issue must be clearly flagged with severity. +- Distinguish between actual vulnerabilities and defense-in-depth suggestions. +- Provide concrete remediation steps, not just vague warnings. +- Rate each finding: `info`, `low`, `medium`, `high`, `critical`. + +## Output Format +``` +## ๐Ÿ”’ Security Sentinel Review + +**Overall Risk Level:** + +### Summary +<1-2 sentence overall assessment> + +### Findings + +#### ๐Ÿ”ด [CRITICAL] +- **Location:** +- **Severity:** critical +- **Issue:** +- **Fix:** + +#### ๐ŸŸก [HIGH] +... + +#### ๐ŸŸข [POSITIVE] +... + +### Action Items +- [ ] +``` diff --git a/.github/review-council/scripts/build_payload.py b/.github/review-council/scripts/build_payload.py new file mode 100644 index 0000000..f5b3ed7 --- /dev/null +++ b/.github/review-council/scripts/build_payload.py @@ -0,0 +1,17 @@ +import json, sys + +with open("/tmp/prompt.txt", "r") as f: + prompt = f.read() + +payload = { + "model": "kimi-latest", + "messages": [ + {"role": "user", "content": prompt} + ], + "temperature": 0.2, + "max_tokens": 8192, + "top_p": 0.95 +} + +with open("/tmp/payload.json", "w") as f: + json.dump(payload, f) diff --git a/.github/review-council/scripts/parse_response.py b/.github/review-council/scripts/parse_response.py new file mode 100644 index 0000000..865ad71 --- /dev/null +++ b/.github/review-council/scripts/parse_response.py @@ -0,0 +1,9 @@ +import json, sys + +try: + data = json.load(open("/tmp/response.json")) + text = data["choices"][0]["message"]["content"] + print(text) +except Exception as e: + print(f"Error parsing response: {e}") + print(open("/tmp/response.json").read()[:1000]) diff --git a/.github/workflows/council-pr-review.yml b/.github/workflows/council-pr-review.yml new file mode 100644 index 0000000..69af7e7 --- /dev/null +++ b/.github/workflows/council-pr-review.yml @@ -0,0 +1,203 @@ +name: Council PR Review + +on: + pull_request: + types: [opened, synchronize, reopened] + workflow_dispatch: + +permissions: + contents: read + pull-requests: write + +concurrency: + group: council-pr-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + review: + if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-council-review') }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + reviewer: + - architect + - security + - performance + - maintainer + + steps: + - name: Check out PR + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Get PR diff + id: diff + run: | + git fetch origin ${{ github.base_ref }} --depth=1 + git diff "origin/${{ github.base_ref }}...HEAD" > /tmp/pr.diff || true + DIFF_LEN=$(wc -c < /tmp/pr.diff) + echo "diff_bytes=$DIFF_LEN" >> "$GITHUB_OUTPUT" + if [ "$DIFF_LEN" -gt 60000 ]; then + head -c 60000 /tmp/pr.diff > /tmp/pr.trimmed.diff + echo "" >> /tmp/pr.trimmed.diff + echo "... [diff truncated: $DIFF_LEN bytes total]" >> /tmp/pr.trimmed.diff + mv /tmp/pr.trimmed.diff /tmp/pr.diff + fi + + - name: Load persona and guidelines + id: context + run: | + PERSONA_FILE=".github/review-council/reviewers/${{ matrix.reviewer }}.md" + if [ ! -f "$PERSONA_FILE" ]; then + echo "Persona file not found: $PERSONA_FILE" >&2 + exit 1 + fi + + GUIDELINES=$(cat .github/review-council/guidelines/*.md 2>/dev/null || echo "No guidelines found.") + PERSONA=$(cat "$PERSONA_FILE") + DIFF=$(cat /tmp/pr.diff) + + { + echo "You are a member of a code review council. Review the following pull request diff according to your persona and the project guidelines provided." + echo "" + echo "## YOUR PERSONA" + echo "" + echo "$PERSONA" + echo "" + echo "## PROJECT GUIDELINES" + echo "" + echo "$GUIDELINES" + echo "" + echo "## PR DIFF" + echo '```diff' + echo "$DIFF" + echo '```' + echo "" + echo "Produce your review in the exact output format specified in your persona." + } > /tmp/prompt.txt + + - name: Call Kimi API + id: kimi + env: + KIMI_API_KEY: ${{ secrets.KIMI_API_KEY }} + run: | + if [ -z "$KIMI_API_KEY" ]; then + echo "review_output=โŒ **KIMI_API_KEY not configured.** Please add it to repository secrets." > "$GITHUB_OUTPUT" + exit 0 + fi + + python3 .github/review-council/scripts/build_payload.py + + URL="https://api.moonshot.cn/v1/chat/completions" + + HTTP_STATUS=$(curl -s -o /tmp/response.json -w "%{http_code}" \ + -X POST "$URL" \ + -H "Content-Type: application/json" \ + -H "Authorization: Bearer ${KIMI_API_KEY}" \ + -d @/tmp/payload.json) + + if [ "$HTTP_STATUS" -ne 200 ]; then + ERROR_BODY=$(cat /tmp/response.json | head -c 500) + { + echo "review_output<> "$GITHUB_OUTPUT" + exit 0 + fi + + REVIEW=$(python3 .github/review-council/scripts/parse_response.py) + + { + echo "review_output<> "$GITHUB_OUTPUT" + + - name: Save review artifact + run: | + mkdir -p /tmp/reviews + echo "${{ steps.kimi.outputs.review_output }}" > "/tmp/reviews/${{ matrix.reviewer }}.md" + + - name: Upload artifact + uses: actions/upload-artifact@v4 + with: + name: review-${{ matrix.reviewer }} + path: /tmp/reviews/${{ matrix.reviewer }}.md + + aggregate: + needs: review + if: ${{ always() && !contains(github.event.pull_request.labels.*.name, 'skip-council-review') }} + runs-on: ubuntu-latest + steps: + - name: Check out PR + uses: actions/checkout@v4 + + - name: Download all review artifacts + uses: actions/download-artifact@v4 + with: + path: /tmp/reviews + pattern: review-* + + - name: Build aggregated comment + id: build + run: | + PR_NUM=${{ github.event.pull_request.number }} + COMMIT_SHA=${{ github.event.pull_request.head.sha }} + SHORT_SHA=${COMMIT_SHA:0:7} + + { + echo "" + echo "" + echo "# ๐Ÿ›๏ธ Council PR Review" + echo "" + echo "**PR:** #${PR_NUM} ยท **Commit:** \`${SHORT_SHA}\` ยท **Generated at:** $(date -u +'%Y-%m-%d %H:%M UTC')" + echo "" + echo "---" + echo "" + } > /tmp/comment.md + + for reviewer in architect security performance maintainer; do + FILE="/tmp/reviews/review-${reviewer}/${reviewer}.md" + if [ -f "$FILE" ]; then + cat "$FILE" >> /tmp/comment.md + echo "" >> /tmp/comment.md + echo "---" >> /tmp/comment.md + echo "" >> /tmp/comment.md + else + echo "## โš ๏ธ ${reviewer} review unavailable" >> /tmp/comment.md + echo "" >> /tmp/comment.md + echo "The review artifact was not produced (job may have failed or been skipped)." >> /tmp/comment.md + echo "" >> /tmp/comment.md + echo "---" >> /tmp/comment.md + echo "" >> /tmp/comment.md + fi + done + + echo "*Want to customize the council? Edit files in \`.github/review-council/\`.*" >> /tmp/comment.md + + - name: Post or update PR comment + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + PR_NUM=${{ github.event.pull_request.number }} + REPO="${{ github.repository }}" + + EXISTING_COMMENT_ID=$(gh api "repos/${REPO}/issues/${PR_NUM}/comments" --paginate \ + -q '.[] | select(.body | contains("")) | .id') + + # Build JSON payload from file to safely handle newlines and quotes + jq -n --rawfile body /tmp/comment.md '{body: $body}' > /tmp/comment.json + + if [ -n "$EXISTING_COMMENT_ID" ]; then + echo "Updating existing comment $EXISTING_COMMENT_ID" + gh api "repos/${REPO}/issues/comments/${EXISTING_COMMENT_ID}" \ + --method PATCH \ + --input /tmp/comment.json + else + echo "Creating new council comment" + gh api "repos/${REPO}/issues/${PR_NUM}/comments" \ + --input /tmp/comment.json + fi diff --git a/mairu/internal/acp/server_test.go b/mairu/internal/acp/server_test.go index 41ab2de..175ec3e 100644 --- a/mairu/internal/acp/server_test.go +++ b/mairu/internal/acp/server_test.go @@ -574,7 +574,7 @@ func TestRequestPermission_IncludesToolCallID(t *testing.T) { func TestServerRun_InitializeRoundTrip(t *testing.T) { stdin := &bytes.Buffer{} - stdout := &bytes.Buffer{} + stdoutR, stdoutW := io.Pipe() s := New(llm.ProviderConfig{}, func(cwd string) (*agent.Agent, error) { return nil, errors.New("no agent") @@ -595,14 +595,14 @@ func TestServerRun_InitializeRoundTrip(t *testing.T) { // Run blocks until stdin is closed. done := make(chan error, 1) go func() { - done <- s.Run(ctx, stdin, stdout) + done <- s.Run(ctx, stdin, stdoutW) }() // Give Run a moment to process. time.Sleep(100 * time.Millisecond) var resp rpcMessage - if err := json.NewDecoder(stdout).Decode(&resp); err != nil { + if err := json.NewDecoder(stdoutR).Decode(&resp); err != nil { t.Fatalf("decode response: %v", err) } if resp.ID == nil {