diff --git a/.claude/guardrails.md b/.claude/guardrails.md index 7bf21fcc000..8d760b4dfe8 100644 --- a/.claude/guardrails.md +++ b/.claude/guardrails.md @@ -84,8 +84,20 @@ moves the permission, not just the steps: Every condition uses the PR author, not `github.actor`: on Jules branches most runs showed the repository owner as the actor, and a re-run can change who -triggered it. A Jules change gets its e2e run on the push Claude's merge -makes and on the PR into `main`, both after the diff was read. +triggered it. A Jules change gets its e2e run on the PR into `main`, after the +diff was read. + +**No CI on pushes to `feature/**` (2026-10-29).** The PR-author conditions had +a gap the first feature-branch run exposed within the hour. Jules names its +own branch after the one it starts from (`feature/head-metadata-dedup-`), and `ci-lint`, `ci-tests`, `ci-migrations`, and `ci-image` also ran on +every push to `feature/**`. A push run has no PR, so no author, and the +pusher is the repository owner, because Jules pushes with the owner's GitHub +grant. The `renderer-image` job of that push run got its GCP login before +Claude cancelled it; only code Jules had not touched ran with it. Since then +`feature/**` is gone from every push trigger, so a feature branch and Jules's +branches under it get CI only through pull requests, and `Run E2E Tests` and +`renderer-image` also refuse any ref under refs/heads/feature at job level. **Gemini 3 Pro against Flash in Jules (2026-10-28).** The same two prompts (#9636, a three-file dedup; #9629, a 29-file migration) ran on both. Flash diff --git a/.claude/skills/jules-task/SKILL.md b/.claude/skills/jules-task/SKILL.md index ee650423e36..92e69d6b958 100644 --- a/.claude/skills/jules-task/SKILL.md +++ b/.claude/skills/jules-task/SKILL.md @@ -70,11 +70,12 @@ the code. `feature/` from `origin/main`, push it, and start one session per sub-task with that branch as the starting branch. The `feature/` prefix matters: `ci-lint` and `ci-tests` run on pull requests into `feature/**` - and on pushes to it (section 4). Sub-tasks that touch different files run in - parallel; a sub-task that builds on another starts after that one is merged, - from the updated branch. The first push of a new branch builds every image - once, because the workflows cannot diff a range that starts at zero; that - costs runner time, not money. + (section 4). A push to `feature/**` runs no CI at all: Jules names its own + branch `feature/-`, and a push run has no PR author to + recognise it by, so before 2026-10-29 such a push ran the GCP login on + unread Jules code. Sub-tasks that touch different files run in parallel; a + sub-task that builds on another starts after that one is merged, from the + updated branch. - **At most 10 sessions in flight** per Claude session (owner, 2026-10-29). Sessions can take 15 to 30 minutes, so ten keep the quota busy without ten settled sessions landing in one context at once; more work goes to a second @@ -195,7 +196,7 @@ Open the pull request against `feature/`, never `main`; the description en skipped. Every Jules PR runs without secrets and without an OIDC token, like a Dependabot PR: `Run Tests` has no `id-token` permission, and the job that can reach GCP skips Jules and Dependabot PRs. The e2e tests run - on the push the merge makes and on the final PR into `main`. + on the final PR into `main`, after the code was read. 5. Known flaky test: `test_local_backend_kills_the_container_on_cancellation` failed once on a frontend-only PR; rerun the failed job. @@ -209,10 +210,8 @@ Open the pull request against `feature/`, never `main`; the description en - **A sub-PR into a feature branch this session leads:** Claude squash-merges it (`gh pr merge --squash`) once the review in section 4 is done and its - checks are green. Owner's rule of 2026-10-28. The push the merge makes runs - CI again, with e2e; a push run can be cancelled by the next merge's push and - checks only `HEAD~1..HEAD`, so treat it as a bonus, not as the gate. A red - push run gets a fix-forward task or a revert before the next merge. + checks are green. Owner's rule of 2026-10-28. The merge's push runs no CI; + the PR into `main` checks the combined result, with e2e. - **Before each merge, check the base.** When `gh pr view --json mergeStateStatus` says `BEHIND` or `DIRTY`, ask the session to merge `origin/feature/` into its branch and rerun its diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index c00b8b03c9e..2b85c0fef46 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -12,7 +12,7 @@ A companion guide `CLAUDE.md` at the repo root carries the shared rules for Clau - **Never echo secret values into transcripts or logs** - Verify secrets by exit code or metadata, never by printing them. - **Fix small build/test blockers directly, even when out of scope** - If a typecheck error, failing test, lint failure, or other small pipeline blocker shows up while you work on something unrelated, fix it in the same PR or a tiny follow-up — never leave it parked as "pre-existing" or "out of scope". A latent error that doesn't surface locally silently breaks the next Cloud Build, which deploys nothing while every PR check looks green. The bar: if the fix is under ~20 lines and obviously correct, make it; if it would expand scope meaningfully, ask first rather than deferring silently. - **Structural fix over symptomatic fix** - When a cheap symptomatic fix and a correct structural fix compete, take the structural one: fix the cause, never mute the alarm. Never modify working code to make a broken test pass — fix the test or flag it. -- **Never manually merge a pipeline PR, and never bypass the pipeline** - Specifications and implementations go through the GitHub Actions workflows: `spec-create.yml` writes `plots/{spec-id}/specification.md` and `.yaml`, `impl-merge.yml` merges implementation PRs and creates their `metadata/*.yaml`, promotes the preview images to GCS and sets the `impl:{library}:done` label. Merging one by hand skips all of that and leaves `quality_score: null`, no review data, missing GCS images and the issue open. So: never write those files by hand, never run `gh pr merge` on a spec or implementation PR, and add the `approved` label to the **issue**, never to the PR. The full DON'T/DO table is in `CLAUDE.md`. Ordinary (non-pipeline) PRs are merged by the repository owner, not by an agent; the one exception is a PR Google Jules opened into a `feature/*` branch, which the Claude session that started the Jules session merges (`CLAUDE.md`, external-system writes). Every merge into `main` stays with the owner. +- **Never manually merge a pipeline PR, and never bypass the pipeline** - Specifications and implementations go through the GitHub Actions workflows: `spec-create.yml` writes `plots/{spec-id}/specification.md` and `.yaml`, `impl-merge.yml` merges implementation PRs and creates their `metadata/*.yaml`, promotes the preview images to GCS and sets the `impl:{library}:done` label. Merging one by hand skips all of that and leaves `quality_score: null`, no review data, missing GCS images and the issue open. So: never write those files by hand, never run `gh pr merge` on a spec or implementation PR, and add the `approved` label to the **issue**, never to the PR. The full DON'T/DO table is in `CLAUDE.md`. Ordinary (non-pipeline) PRs are merged by the repository owner, not by an agent; the one exception is a PR Google Jules opened into a `feature/*` branch, which the Claude session that started the Jules session merges (`CLAUDE.md`, external-system writes). Every merge into `main` stays with the owner. Pushes to `feature/**` run no CI, because Jules's own branches live there and a push run has no PR author; feature branches get CI through their PRs. ## Changelog + releases diff --git a/.github/workflows/ci-image.yml b/.github/workflows/ci-image.yml index a6d92f8507b..0e165dda222 100644 --- a/.github/workflows/ci-image.yml +++ b/.github/workflows/ci-image.yml @@ -31,7 +31,10 @@ on: branches: - main - develop - - 'feature/**' + # No `feature/**` here: Google Jules pushes its own branches under the + # feature branch it works on (`feature/-`), and a push run + # has no PR author, so `renderer-image` would hand unread Jules code its + # GCP login and OIDC token. Feature branches get CI through their PRs. pull_request: branches: - main @@ -503,7 +506,11 @@ jobs: # The job holds `id-token: write`, and PR code runs in it, so a PR Google Jules # opened never runs here: Jules opens PRs only into feature/* branches, which # this workflow does not cover, and this guards the one that lands on main by mistake. - if: github.event.pull_request.user.login != 'google-labs-jules[bot]' + # A push to a feature/ branch never runs it either: Jules's own branches live there. + if: >- + github.event.pull_request.user.login != 'google-labs-jules[bot]' && + !startsWith(github.ref, 'refs/heads/feature/') && + !startsWith(github.event.merge_group.base_ref, 'refs/heads/feature/') runs-on: ubuntu-latest permissions: contents: read diff --git a/.github/workflows/ci-lint.yml b/.github/workflows/ci-lint.yml index 43c6190c95b..95eefb592c7 100644 --- a/.github/workflows/ci-lint.yml +++ b/.github/workflows/ci-lint.yml @@ -6,7 +6,9 @@ on: branches: - main - develop - - 'feature/**' + # No `feature/**` here: Google Jules pushes its own branches under the + # feature branch it works on (`feature/-`), and a push run + # has no PR author to recognise it by. Feature branches get CI through their PRs. pull_request: branches: - main diff --git a/.github/workflows/ci-migrations.yml b/.github/workflows/ci-migrations.yml index eefbba049c4..6666d020179 100644 --- a/.github/workflows/ci-migrations.yml +++ b/.github/workflows/ci-migrations.yml @@ -34,7 +34,8 @@ on: branches: - main - develop - - 'feature/**' + # No `feature/**` here, as in ci-tests.yml: Jules's own branches live under + # `feature/`. Feature branches get CI through their PRs. pull_request: branches: - main diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index d37a4d1327b..26b7ffa3a71 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -6,7 +6,10 @@ on: branches: - main - develop - - 'feature/**' + # No `feature/**` here: Google Jules pushes its own branches under the + # feature branch it works on (`feature/-`), and a push run + # has no PR author, so the e2e job would treat unread Jules code as trusted + # and give it the GCP login. Feature branches get CI through their PRs. pull_request: branches: - main @@ -185,13 +188,17 @@ jobs: # Federation, starts the Cloud SQL proxy, and runs the e2e tests against real # PostgreSQL (a separate 'test' database). It runs after test, and only for # authors whose code has been read (UNTRUSTED_AUTHOR at the top, repeated here - # because a job-level `if` cannot read `env`). A Jules change gets it on the - # push its merge makes, and on the PR into main. + # because a job-level `if` cannot read `env`). It never runs for a feature + # branch, neither for a ref under it nor for a PR into it, so a Jules change + # gets it on the PR into main, after the diff was read. test-e2e: name: Run E2E Tests needs: test if: >- needs.test.outputs.should_test == 'true' && + !startsWith(github.ref, 'refs/heads/feature/') && + !startsWith(github.base_ref, 'feature/') && + !startsWith(github.event.merge_group.base_ref, 'refs/heads/feature/') && github.actor != 'dependabot[bot]' && !contains(fromJSON('["dependabot[bot]", "google-labs-jules[bot]"]'), github.event.pull_request.user.login) runs-on: ubuntu-latest diff --git a/CLAUDE.md b/CLAUDE.md index faef4e89c6d..40d0d875bff 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,7 +18,7 @@ A companion guide `.github/copilot-instructions.md` carries the shared rules for - **Repository prose follows the Google developer documentation style guide** - `docs/`, `README.md`, `agentic/docs/`, changelog entries, and PR/issue text use [Google style](https://developers.google.com/style) (sentence-case headings, second person, numbered procedures); the concrete rules and the house-style exception (`docs/reference/style-guide.md` governs website/brand surfaces) live in the `write-docs` skill. Existing docs migrate on touch, not via bulk rewrites. - **Update documentation when making changes** - When adding new features, events, or modifying behavior, always check if related documentation needs updating (e.g., `docs/reference/plausible.md` for analytics events, `docs/workflows/` for workflow changes, `docs/contributing.md` for user-facing changes). - **Changelog and releases** - see [Changelog + releases](#changelog--releases) below: every PR adds a fragment under `changelog.d/` and adds nothing to `CHANGELOG.md` (correcting the wording of a bullet already under `[Unreleased]` is allowed), a release folds the fragments and bumps the version files via a PR, and the GitHub release is that section condensed, never copied. -- **External-system writes need explicit, named authorization** - Merging or closing PRs/issues this session did not create, bulk merges, and label changes on others' PRs are blocked by the permission classifier unless the user named that action; a generic "ok, sounds good" authorizes nothing. One standing exception (owner, 2026-10-28): a PR that Jules opened for a session this session started counts as this session's own, so Claude merges it into a `feature/*` branch this session leads, or closes it as the loser or duplicate of a comparison and deletes its branch (`/jules-task`); every merge into `main` still needs the owner. CI runs a Jules or Dependabot PR without secrets and without an OIDC token, keyed on the PR author: `ci-tests.yml` keeps every GCP step in the `test-e2e` job, and `ci-image.yml`'s `renderer-image` skips Jules PRs. The same discipline covers every prod-touching action in interactive sessions — Cloud SQL writes/DDL, GCS production-folder changes, Secret Manager access, Cloud Build config: name the exact action, resource, and id, and ask before acting (the automated `spec-*`/`impl-*` workflows write these by design and are exempt). For any change to `.claude/settings*.json`, use the built-in `/update-config` skill (a Claude Code harness skill, not a repo command) immediately — direct writes are blocked as self-modification and retrying variants just burns round trips. +- **External-system writes need explicit, named authorization** - Merging or closing PRs/issues this session did not create, bulk merges, and label changes on others' PRs are blocked by the permission classifier unless the user named that action; a generic "ok, sounds good" authorizes nothing. One standing exception (owner, 2026-10-28): a PR that Jules opened for a session this session started counts as this session's own, so Claude merges it into a `feature/*` branch this session leads, or closes it as the loser or duplicate of a comparison and deletes its branch (`/jules-task`); every merge into `main` still needs the owner. CI runs a Jules or Dependabot PR without secrets and without an OIDC token, keyed on the PR author: `ci-tests.yml` keeps every GCP step in the `test-e2e` job, and `ci-image.yml`'s `renderer-image` skips Jules PRs. Pushes to `feature/**` run no CI, because Jules's own branches live there and a push run has no PR author. The same discipline covers every prod-touching action in interactive sessions — Cloud SQL writes/DDL, GCS production-folder changes, Secret Manager access, Cloud Build config: name the exact action, resource, and id, and ask before acting (the automated `spec-*`/`impl-*` workflows write these by design and are exempt). For any change to `.claude/settings*.json`, use the built-in `/update-config` skill (a Claude Code harness skill, not a repo command) immediately — direct writes are blocked as self-modification and retrying variants just burns round trips. - **Never echo secret values into the transcript** - Verify secrets by exit code or metadata, never by printing them; never create a Secret Manager version via `echo` (the trailing newline corrupts the value). - **Snapshot before destructive prod operations** - Before anything that can overwrite or delete shared prod DB data or GCS production objects (bulk UPDATE/DELETE, a data-rewriting migration, bulk GCS overwrite/delete): take a timestamped backup first (`pg_dump` to a new directory outside the working tree, `gsutil cp` to a backup prefix), sanity-check it (row/object counts — a silent empty snapshot is worse than none because it looks like safety), and never write into, delete, or rename an existing snapshot. - **After every merge, check that the site renders** - Yours, Dependabot's, or one the owner asked for: once the merge's deploys finish, run `scripts/site-smoke.mjs` (recipe in `/open-pr` §4); a `FAIL` is an incident, reported at once with the suspect merge and a revert proposal, and nothing else merges until the site renders. A green PR plus a green deploy blanked anyplot.ai for 45 minutes on 2026-10-22 (#12225). diff --git a/changelog.d/ci-no-feature-push.md b/changelog.d/ci-no-feature-push.md new file mode 100644 index 00000000000..d83836e428e --- /dev/null +++ b/changelog.d/ci-no-feature-push.md @@ -0,0 +1,12 @@ +### Security + +- **No CI on pushes to `feature/**`.** Google Jules names its own branch after + the feature branch it starts from (`feature/-`), and a + push run has no PR author, so the checks keyed on the PR author treated + unread Jules code as trusted: the first such push gave `renderer-image` its + GCP login before the run was cancelled. `ci-lint`, `ci-tests`, + `ci-migrations`, and `ci-image` no longer run on pushes to `feature/**`; + feature branches get CI through their pull requests. `Run E2E Tests` and + `renderer-image` also refuse to run for any `feature/` ref, a merge queue + into a feature branch, and, for the e2e job, any PR into a feature branch. + (#12441)