diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 0000000000..bd4daaab80 --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,101 @@ +# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json +# CodeRabbit is the first-pass reviewer for every PR, including drafts and +# fork PRs. CircleCI pr-check runs the builds and tests (fork PRs need the +# ci:run label, see .github/workflows/fork-ci-mirror.yml). UI proof on a real +# Windows build stays a human or agent responsibility (AGENTS.md). +language: en-US +reviews: + profile: assertive + # Approve once every comment is resolved and no error-mode check fails; + # request changes otherwise. Maintainers still merge. + request_changes_workflow: true + high_level_summary: true + auto_review: + enabled: true + drafts: true + auto_incremental_review: true + path_instructions: + - path: "rust/src/providers/**" + instructions: >- + Provider-specific logic must stay inside its own provider module. Flag + fetch or parse changes that come without deterministic samples or + fixtures. Flag any provider that reads, shows, or stores identity, + plan, or email belonging to another provider. + - path: "rust/src/core/provider_factory.rs" + instructions: >- + This is the only provider factory. Flag any second factory or + provider construction added in the Tauri shell or the CLI. + - path: "apps/desktop-tauri/src/types/bridge.ts" + instructions: >- + These DTOs must match the Rust command payloads field for field + (names, optionality, enum strings). Check the matching structs under + apps/desktop-tauri/src-tauri/src/commands and flag any drift. + - path: "apps/desktop-tauri/src-tauri/src/surface_target.rs" + instructions: >- + The settings tab whitelist must stay exactly: general, providers, + notifications, menuBar, menu, usageSpend, advanced, about. It must + mirror the frontend SettingsTabId and TAB_META. + - path: "apps/desktop-tauri/src-tauri/src/floatbar/**" + instructions: >- + The float bar window builder must keep + .theme(Some(tauri::Theme::Dark)); an unpinned theme flips other + WebView2 windows under theme auto. + - path: ".github/workflows/**" + instructions: >- + Treat workflow changes as security-sensitive. Flag + pull_request_target jobs that check out or execute PR code, broad + permissions, secrets exposed to fork code, and any automatic trigger + added to the manual Blacksmith reserve workflow. + pre_merge_checks: + docstrings: + mode: "off" + title: + mode: warning + requirements: >- + Short imperative summary of the change, for example "Fix Claude CLI + parser" or "Improve cookie import errors". + description: + mode: warning + custom_checks: + - name: Provider data stays siloed + mode: error + instructions: >- + Fail if the change can show, log, persist, or send identity, plan, + account label, or email from one provider (or one account) in + another provider's or account's UI, payload, or storage, or if it + adds cross-provider branching (matching on specific ProviderId + values) in shared code outside rust/src/providers// and + rust/src/core/provider_factory.rs. Pass otherwise. + - name: Secrets handled safely + mode: error + instructions: >- + Fail if any token, cookie, API key, OAuth credential, or + authorization header can reach a log line (tracing macros, + println, console.*), an error message shown to the user, a test + snapshot, or a plain-text file. Secrets must go through the + existing redaction, secure_file, or keyring helpers. Fail if the + change deletes or overwrites credential files or keyring entries + that belong to another application (for example Claude Code or + Codex CLI logins) without an explicit user action. Pass otherwise. + - name: No unapproved dependencies + mode: error + instructions: >- + Fail if the change adds a dependency to any Cargo.toml or + package.json, adds an npm or yarn lockfile, or changes the pinned + pnpm packageManager version, unless the PR description says a + maintainer approved that addition. Version bumps of existing + dependencies only warn. Pass otherwise. + - name: UI changes include Windows proof + mode: warning + instructions: >- + If the change alters what a user sees (files under + apps/desktop-tauri/src outside *.test.* files, tray rendering in + rust/src/tray, the float bar, settings, or window chrome), the PR + description must include screenshots or a proof note from a fresh + Windows build. Fail if it does not. Pass for changes with no + visible effect. +knowledge_base: + code_guidelines: + enabled: true + filePatterns: + - "**/AGENTS.md" diff --git a/.github/CI.md b/.github/CI.md index bea7035865..2b05fa0687 100644 --- a/.github/CI.md +++ b/.github/CI.md @@ -36,6 +36,34 @@ CircleCI's GitHub App trigger and auto-cancel settings live outside the repository. Keep PR, default-branch, and budget rules there; do not add a second tag trigger. +## Fork pull requests + +CircleCI GitHub App pipelines never fire for pull requests from forks. To +build one: + +1. Read the fork PR's diff. The build runs the contributor's code on CircleCI + with this project's environment variables. +2. Add the `ci:run` label. .github/workflows/fork-ci-mirror.yml pushes + exactly that head commit to ci/pr- in this repository. +3. CircleCI builds the ci/pr- push. The trigger gate in + scripts/circleci-pr-common.ps1 lets those branches through, and the + docs-only gate finds the PR base from the number. The check lands on the + same commit, so it shows on the fork PR. + +Any new push to the fork PR removes the label; re-add it after reviewing the +new commits. Closing the PR removes the mirror branch. GITHUB_TOKEN cannot +push commits that change .github/workflows/**, so the mirror fails for those +PRs; review them by hand and use the manual Blacksmith workflow if needed. + +## Code review + +CodeRabbit reviews every PR, drafts included, with the settings in +.coderabbit.yaml. It uses AGENTS.md as its guidelines and runs blocking +pre-merge checks for provider data siloing, secret handling and new +dependencies. It approves a PR once its comments are resolved; maintainers +still merge. It does not build or test anything, and it does not replace UI +proof on a fresh Windows build. + ## GitHub Actions release path .github/workflows/release.yml runs only for canonical vX.Y.Z tag pushes on a diff --git a/.github/workflows/fork-ci-mirror.yml b/.github/workflows/fork-ci-mirror.yml new file mode 100644 index 0000000000..0a2d705159 --- /dev/null +++ b/.github/workflows/fork-ci-mirror.yml @@ -0,0 +1,107 @@ +name: Fork PR CI mirror + +# CircleCI GitHub App pipelines never fire for pull requests from forks, so +# fork PRs used to merge with no hosted Windows check. A maintainer approves a +# fork PR's current head by adding the `ci:run` label. This workflow then +# copies exactly that commit to ci/pr- in this repository. CircleCI +# builds the push (scripts/circleci-pr-common.ps1 lets ci/pr- through), +# and its check lands on the same commit, so it shows on the fork PR. +# +# Safety: +# - pull_request_target runs this file from the base branch, never the fork's +# copy. No step checks out, installs, or executes fork code here; the job +# only moves a git ref. The fork code runs later on CircleCI, which is the +# reason for the label gate. +# - Only users with triage access or higher can add labels. +# - Any new push to the fork PR removes the label, so every new head needs a +# fresh maintainer approval before CircleCI builds it. +# - The mirror branch is removed when the PR closes. +# - GITHUB_TOKEN cannot push commits that change .github/workflows/**; such +# fork PRs fail the mirror step and need a manual review path. +on: + pull_request_target: + types: [labeled, synchronize, closed] + +permissions: {} + +concurrency: + group: fork-ci-mirror-${{ github.event.pull_request.number }} + cancel-in-progress: false + +jobs: + mirror: + name: Mirror approved fork head + if: >- + github.event.action == 'labeled' && + github.event.label.name == 'ci:run' && + github.event.pull_request.head.repo.full_name != github.repository && + github.event.pull_request.state == 'open' && + vars.CI_BUDGET_MODE != 'off' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: write + steps: + - name: Push the approved commit to ci/pr- + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + git init --quiet mirror + cd mirror + auth="AUTHORIZATION: basic $(printf 'x-access-token:%s' "$GITHUB_TOKEN" | base64 -w0)" + git remote add origin "https://github.com/${REPO}.git" + # Fetch the approved commit by SHA, not the moving pull ref, so a + # push that races the label can never be mirrored unapproved. + git -c http.extraheader="$auth" fetch --quiet --no-tags --depth=1 origin "$HEAD_SHA" + test "$(git rev-parse FETCH_HEAD)" = "$HEAD_SHA" + # The mirror branch belongs to this workflow; a re-approval replaces it. + git -c http.extraheader="$auth" push --quiet --force origin "${HEAD_SHA}:refs/heads/ci/pr-${PR_NUMBER}" + echo "Mirrored ${HEAD_SHA} to ci/pr-${PR_NUMBER}; CircleCI pr-check will report on this commit." >> "$GITHUB_STEP_SUMMARY" + + revoke: + name: Require re-approval after a new push + if: >- + github.event.action == 'synchronize' && + github.event.pull_request.head.repo.full_name != github.repository && + contains(github.event.pull_request.labels.*.name, 'ci:run') + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + pull-requests: write + steps: + - name: Remove the ci:run label + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + # The event's label list can be stale; a label already removed is fine. + if ! gh api --method DELETE "repos/${REPO}/issues/${PR_NUMBER}/labels/ci:run" --silent 2>err.txt; then + grep -q "Label does not exist\|HTTP 404" err.txt || { cat err.txt; exit 1; } + fi + echo "New commits on a fork PR: ci:run removed. Re-add it after reviewing the new head." >> "$GITHUB_STEP_SUMMARY" + + cleanup: + name: Remove the mirror branch + if: >- + github.event.action == 'closed' && + github.event.pull_request.head.repo.full_name != github.repository + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: write + steps: + - name: Delete ci/pr- if present + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + if gh api "repos/${REPO}/git/ref/heads/ci/pr-${PR_NUMBER}" --silent 2>/dev/null; then + gh api --method DELETE "repos/${REPO}/git/refs/heads/ci/pr-${PR_NUMBER}" --silent + echo "Removed ci/pr-${PR_NUMBER}." >> "$GITHUB_STEP_SUMMARY" + fi diff --git a/AGENTS.md b/AGENTS.md index fa5f149f07..7cf3331333 100755 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,6 +129,7 @@ pnpm run tauri:build - Rust: prefer focused `#[cfg(test)]` unit tests near the changed module. Run both manifests after Rust changes. - Frontend: Vitest 3 + jsdom + Testing Library. From `apps/desktop-tauri`: `pnpm test` (`src/**/*.{test,spec}.{ts,tsx}`). - **Hosted PR check**: CircleCI Windows is primary and runs `scripts/local-check.ps1 -Slice ci` for ordinary/canonical PRs and `main`; micro PRs targeting `port/upstream-*` intentionally skip hosted Windows compute; a micro-named PR targeting `main` does not. `.github/workflows/pr-check.yml` is manual Blacksmith Windows reserve only. Budget details: `CONTEXT.md`, `.github/CI.md`, and ADRs under `docs/adr/`. +- **Fork PRs and reviews**: CircleCI never triggers on fork PRs by itself. After reading the diff, a maintainer adds the `ci:run` label; `fork-ci-mirror.yml` copies that head to `ci/pr-` for CircleCI, and any new push removes the label. CodeRabbit (`.coderabbit.yaml`) reviews every PR, drafts included, against these rules; fix or answer its findings before merging. - **Hosted mirror**: `.\scripts\local-check.ps1 -Slice ci`. The default no-parameter developer slice remains available and does not run full installer/smoke unless requested. - Parser / fetcher changes: add deterministic samples or fixtures where practical. - No coverage thresholds are configured — do not invent any. diff --git a/scripts/circleci-pr-common.ps1 b/scripts/circleci-pr-common.ps1 index daaeda5574..53427742da 100644 --- a/scripts/circleci-pr-common.ps1 +++ b/scripts/circleci-pr-common.ps1 @@ -50,21 +50,37 @@ function Get-TriggerGateDecision { # every other branch push skips. CircleCI delivers same-repo PR builds # as branch pipelines, so PR association comes from the compile-time # GitHub App pipeline value pipeline.event.context.github.pr_url. + # GitHub App pipelines never fire for fork PRs, so the fork-ci-mirror + # workflow copies a maintainer-approved fork head to ci/pr-; + # those pushes run the checks too. $isPr = -not [string]::IsNullOrWhiteSpace($PrUrl) $isMainPush = $Branch -in @('main', 'master') - if (-not $isPr -and -not $isMainPush) { + $isForkMirror = -not [string]::IsNullOrWhiteSpace((Get-ForkMirrorPrNumber -Branch $Branch)) + if (-not $isPr -and -not $isMainPush -and -not $isForkMirror) { return [pscustomobject]@{ Skip = $true - Reason = "Branch push to '$Branch' (not a PR, not main/master): hosted pr-check skips." + Reason = "Branch push to '$Branch' (not a PR, not main/master, not a ci/pr- fork mirror): hosted pr-check skips." } } return [pscustomobject]@{ Skip = $false - Reason = "Trigger gate passed (PR: $isPr, branch: '$Branch')." + Reason = "Trigger gate passed (PR: $isPr, fork mirror: $isForkMirror, branch: '$Branch')." } } +<# +.SYNOPSIS + PR number of a ci/pr- fork-mirror branch, or '' for any other + branch. The fork-ci-mirror workflow is the only writer of these branches. +#> +function Get-ForkMirrorPrNumber { + param([AllowEmptyString()][string]$Branch) + + if ($Branch -match '^ci/pr-([1-9]\d*)$') { return $Matches[1] } + return '' +} + <# .SYNOPSIS Extract a SHA-256 digest for FileName from official checksum text. diff --git a/scripts/circleci-pr-gates.ps1 b/scripts/circleci-pr-gates.ps1 index bd229fe1d2..37db10d499 100644 --- a/scripts/circleci-pr-gates.ps1 +++ b/scripts/circleci-pr-gates.ps1 @@ -94,7 +94,7 @@ if ($isMainPush) { # PR association without a populated event value (e.g. api trigger): # resolve the base from the public GitHub pulls API. try { - $prNumber = '' + $prNumber = Get-ForkMirrorPrNumber -Branch $Branch if ($PrUrl -match '/pull/(\d+)') { $prNumber = $Matches[1] } if ([string]::IsNullOrWhiteSpace($prNumber)) { throw 'PR number not available from pipeline values.' } if ([string]::IsNullOrWhiteSpace($env:CIRCLE_PROJECT_USERNAME) -or [string]::IsNullOrWhiteSpace($env:CIRCLE_PROJECT_REPONAME)) { diff --git a/scripts/circleci-pr.tests.ps1 b/scripts/circleci-pr.tests.ps1 index a776beda5d..20338cd0e0 100644 --- a/scripts/circleci-pr.tests.ps1 +++ b/scripts/circleci-pr.tests.ps1 @@ -88,6 +88,16 @@ Assert-True $decision.Skip 'non-PR topic branch skips' Assert-True ($decision.Reason -match 'codex/topic') 'topic branch skip reason' $decision = Get-TriggerGateDecision -BudgetMode 'off' -Branch 'main' -PrUrl '' Assert-True $decision.Skip 'budget off wins over main push' +$decision = Get-TriggerGateDecision -BudgetMode 'normal' -Branch 'ci/pr-750' -PrUrl '' +Assert-True (-not $decision.Skip) 'fork mirror branch runs' +$decision = Get-TriggerGateDecision -BudgetMode 'off' -Branch 'ci/pr-750' -PrUrl '' +Assert-True $decision.Skip 'budget off wins over fork mirror' +foreach ($branch in @('ci/pr-', 'ci/pr-0', 'ci/pr-12x', 'ci/pr-7/extra', 'x/ci/pr-7', 'ci/pr-7-old')) { + $decision = Get-TriggerGateDecision -BudgetMode 'normal' -Branch $branch -PrUrl '' + Assert-True $decision.Skip "look-alike mirror branch '$branch' skips" +} +Assert-Equal (Get-ForkMirrorPrNumber -Branch 'ci/pr-750') '750' 'mirror PR number parsed' +Assert-Equal (Get-ForkMirrorPrNumber -Branch 'main') '' 'non-mirror branch has no PR number' Write-Host '==> Hosted Node provisioning guard' $prRunnerText = Get-Content -Raw -LiteralPath (Join-Path $scriptRoot 'run-circleci-pr-check.ps1')