Emit verdict rows to S3 instead of ClickHouse

- verdict: emit a gzipped JSONEachRow row to fixed /tmp paths for the
  record workflow to upload, dropping the direct ClickHouse INSERT
- clickhouse_client: strip VerdictRow/INSERT/insert_verdict_row; keep
  only connect() as a read-path helper returning the driver Client
- workflow: record job drops ClickHouse write secrets, gains OIDC
  id-token + arc role, and aws s3 cp's the row to gha-artifacts
- tests: replace the fake CH insert client with a fake emit; assert
  gzip payload contents, key scheme, and default file-writing emit
- docs: README/CHEATSHEET describe the S3 -> replicator ingest path

The record job no longer holds ClickHouse write credentials or runs any
INSERT. Verdict rows land in s3://gha-artifacts/greenlight_pr_state/ and
the clickhouse-replicator-s3 Lambda ingests them into
misc.greenlight_pr_state; this keeps App-key and write access off the
untrusted-content boundary. greenlight retains ClickHouse READ access for
the service's own SELECTs. The object is a single PutObject with no
content-encoding so the ClickHouse s3() reader parses it as stored.

Signed-off-by: Jean Schmidt <contato@jschmidt.me>

Author: jeanschmidt

.github/workflows/greenlight-pr-review.yml

@@ -4,8 +4,10 @@ name: Green Light PR Review
 #   review  — UNPRIVILEGED. Runs the AI model over untrusted PR content. Has Bedrock
 #             (OIDC) access only; it can read code and emit a verdict file, nothing more.
 #   record  — PRIVILEGED. No model. Validates the verdict, mints the Green Light App
-#             token, and writes the row to ClickHouse. Kept separate so untrusted PR
-#             content never runs in a job that holds the App key or write creds.
+#             token, posts the review, and uploads the verdict row to
+#             s3://gha-artifacts/greenlight_pr_state/ (via OIDC role/arc) for the
+#             clickhouse-replicator-s3 Lambda to ingest. Kept separate so untrusted
+#             PR content never runs in a job that holds the App key.
 
 on:
   workflow_dispatch:
@@ -195,17 +197,15 @@ jobs:
     if: ${{ always() }}
     runs-on: ubuntu-latest
     timeout-minutes: 15
-    # Protected environment holding ALL record-job secrets (provision before first run):
-    #   GREENLIGHT_APP_ID and GREENLIGHT_APP_PRIVATE_KEY (the Green Light App key), and
-    #   CLICKHOUSE_HUD_USER_WRITE_URL/_USERNAME/_PASSWORD (a dedicated ClickHouse write
-    #   user). These live on this environment and are NOT inherited from the repo/org.
+    # Protected environment holding the Green Light App key (provision before first run):
+    #   GREENLIGHT_APP_ID and GREENLIGHT_APP_PRIVATE_KEY. These live on this environment
+    #   and are NOT inherited from the repo/org. Verdict rows are emitted to S3 via the
+    #   OIDC role below, so no ClickHouse credentials are needed here.
     environment: greenlight-record
     permissions:
       contents: read
+      id-token: write
     env:
-      CLICKHOUSE_ENDPOINT: ${{ secrets.CLICKHOUSE_HUD_USER_WRITE_URL }}
-      CLICKHOUSE_USERNAME: ${{ secrets.CLICKHOUSE_HUD_USER_WRITE_USERNAME }}
-      CLICKHOUSE_PASSWORD: ${{ secrets.CLICKHOUSE_HUD_USER_WRITE_PASSWORD }}
       PR_NUMBER: ${{ github.event.inputs.pr_number }}
       HEAD_SHA: ${{ github.event.inputs.head_sha }}
       EVAL_HASH: ${{ github.event.inputs.eval_hash }}
@@ -226,6 +226,13 @@ jobs:
       - name: Install uv
         uses: ./.github/actions/setup-uv
 
+      - name: Configure AWS credentials via OIDC
+        # Ungated: success, cancelled, and failed all upload a verdict row to S3.
+        uses: aws-actions/configure-aws-credentials@v4
+        with:
+          role-to-assume: arn:aws:iam::308535385114:role/arc
+          aws-region: us-east-1
+
       - name: Download verdict artifact
         if: ${{ needs.review.result == 'success' }}
         uses: actions/download-artifact@v4
@@ -307,3 +314,13 @@ jobs:
             --head-sha "$HEAD_SHA" \
             --eval-hash "$EVAL_HASH" \
             --eval-job-url "$RUN_URL"
+
+      - name: Upload verdict row to S3
+        # Exactly one record step above produced the row + key at fixed /tmp paths.
+        # No `if:` — the default success() runs this after whichever branch ran and
+        # skips it if that branch failed (skipped siblings don't count as failures).
+        # Single PutObject, no --content-encoding: the ClickHouse s3() reader expects
+        # the object stored as-is and would read 0 rows if it were marked gzip-encoded.
+        run: |
+          set -euo pipefail
+          aws s3 cp /tmp/greenlight-verdict-row.json.gz "s3://gha-artifacts/$(cat /tmp/greenlight-verdict-key.txt)"

greenlight/CHEATSHEET.md

@@ -131,11 +131,13 @@ automated DDL), so @clee2000 or @huydhn apply them manually:
 
 - `001_create_misc_greenlight_pr_state.sql` — create the table
 - `002_alter_greenlight_pr_state_version_default.sql` — set the `version` DEFAULT
-
-No per-table grant file is needed: the PR-review workflow writes as a shared ClickHouse
-write user (`CLICKHOUSE_HUD_USER_WRITE_*` secrets on the `greenlight-record` environment)
-that already holds INSERT through its broad grants. The `verdict` subcommand reads the
-standard `CLICKHOUSE_*` connection variables (`CLICKHOUSE_HOST` or its
-`CLICKHOUSE_ENDPOINT` alias, `CLICKHOUSE_USERNAME`, `CLICKHOUSE_PASSWORD`, and
-`CLICKHOUSE_PORT` default `8443`) and only ever INSERTs rows. The review-side fingerprint
-computation and the land-time verifier that reads this table back are not built yet.
+- `003_alter_greenlight_pr_state_add_meta.sql` — add the `_meta` column the replicator needs
+
+The `verdict` subcommand does NOT write ClickHouse directly: it emits a gzipped JSON row
+that the record workflow uploads to `s3://gha-artifacts/greenlight_pr_state/`, and the
+clickhouse-replicator-s3 path ingests it into the table. greenlight keeps ClickHouse READ
+access for the service's SELECTs via `clickhouse_client.connect()`, which reads the standard
+`CLICKHOUSE_*` connection variables (`CLICKHOUSE_HOST` or its `CLICKHOUSE_ENDPOINT` alias,
+`CLICKHOUSE_USERNAME`, `CLICKHOUSE_PASSWORD`, and `CLICKHOUSE_PORT` default `8443`). The
+review-side fingerprint computation and the land-time verifier that reads this table back
+are not built yet.

greenlight/README.md

@@ -39,24 +39,29 @@ The `review` examples require `PYTORCH_GREENLIGHT_GITHUB_TOKEN` to be set; witho
 ### Recording a verdict
 
 A privileged CI job records a review verdict with `verdict`. It runs once (never a
-daemon): it writes a row to `misc.greenlight_pr_state` — storing the passed-in
-`eval_hash` verbatim — then, for `LAND`/`NO_LAND`, acts on the PR (`LAND` approves;
-`NO_LAND` dismisses greenlight's own prior approval and comments). `CANCELLED` and
-`FAILED` markers only record a row. The `reason` must be a canonical `ALLOWED_REASONS`
-code, and the model's message is defanged before it is posted to GitHub (the full
-message is stored verbatim in ClickHouse).
+daemon): it emits a gzipped single-line JSON row (whose `reason` must be a canonical
+`ALLOWED_REASONS` code) that the record workflow uploads to
+`s3://gha-artifacts/greenlight_pr_state/`, where the clickhouse-replicator-s3 path ingests
+it into `misc.greenlight_pr_state` — the command never writes ClickHouse directly. Then,
+for `LAND`/`NO_LAND`, it acts on the PR (`LAND` approves; `NO_LAND` dismisses greenlight's
+own prior approval and comments). `CANCELLED` and `FAILED` markers only emit the row. The
+model's message is defanged before it is posted to GitHub, while the full message is stored
+verbatim in the emitted row.
 
 ```bash
 just run verdict --pr 123 --head-sha "$SHA" --verdict-file verdict.json \
   --eval-hash "$EVAL_HASH" --bot-login 'greenlight-app[bot]'   # LAND/NO_LAND
-just run verdict --pr 123 --head-sha "$SHA" --status CANCELLED  # marker: row only
+just run verdict --pr 123 --head-sha "$SHA" --status CANCELLED  # marker: emit row only
 just run verdict --pr 123 --head-sha "$SHA" --verdict-file verdict.json \
   --eval-hash "$EVAL_HASH" --dry-run                            # offline; logs only
 ```
 
-`verdict` needs `PYTORCH_GREENLIGHT_GITHUB_TOKEN` (for `LAND`/`NO_LAND`) and the
-`CLICKHOUSE_*` credentials to write; `--dry-run` needs neither. `--bot-login` (the
-greenlight GitHub App's `<slug>[bot]` account) is required for `NO_LAND`.
+The command writes the gzipped row to `/tmp/greenlight-verdict-row.json.gz` and its
+bucket-relative key to `/tmp/greenlight-verdict-key.txt`; the workflow `aws s3 cp`s the
+former to the latter. There is no direct ClickHouse write, so no `CLICKHOUSE_*` credentials
+are needed here; `verdict` needs `PYTORCH_GREENLIGHT_GITHUB_TOKEN` to post `LAND`/`NO_LAND`,
+and `--dry-run` needs nothing. `--bot-login` (the greenlight GitHub App's `<slug>[bot]`
+account) is required for `NO_LAND`.
 
 Configuration is read from the environment via `PYTORCH_GREENLIGHT_*` variables:
 
@@ -83,10 +88,13 @@ on failure, and clean signal shutdown — all built and tested. `review` fetches
 PRs from a fixed set of trusted authors in `pytorch/pytorch` (read-only GitHub) and
 logs them; it requires `PYTORCH_GREENLIGHT_GITHUB_TOKEN`.
 
-Also works: the `verdict` subcommand records a PR-review verdict to
-`misc.greenlight_pr_state` (storing the passed-in `eval_hash` verbatim) and, for
-LAND/NO_LAND, acts on the PR — approve, or dismiss greenlight's prior approval and
-comment. It is a one-shot call meant for a privileged CI job.
+Also works: the `verdict` subcommand emits a PR-review verdict row (with the passed-in
+`eval_hash` verbatim) for the record workflow to upload to
+`s3://gha-artifacts/greenlight_pr_state/`, where the clickhouse-replicator-s3 path ingests
+it into `misc.greenlight_pr_state`; for LAND/NO_LAND it also acts on the PR — approve, or
+dismiss greenlight's prior approval and comment. It is a one-shot call for a privileged CI
+job and never writes ClickHouse directly. The service keeps ClickHouse READ access
+(`clickhouse_client.connect()`) for its own SELECTs.
 
 Not built yet: risk-scoring and the review decision in `review`; the AI code-review
 workflow (a separate component) that produces the verdict; and the review-side
@@ -120,9 +128,9 @@ src/greenlight/
   cli.py           # CLI parsing (review + verdict subcommands), dispatch, exit codes
   runner.py        # run_forever(): resilient daemon loop; execute_once(): one-shot phase run
   review.py        # fetch open PRs from trusted authors in pytorch/pytorch and log them; raises on failure
-  verdict.py       # one-shot: record a PR-review verdict, then approve/dismiss/comment on the PR
+  verdict.py       # one-shot: emit a verdict row for S3->replicator, then approve/dismiss/comment on the PR
   github_client.py # GitHub PR access: read PR list/fingerprint + post verdict actions
-  clickhouse_client.py # ClickHouse connection + verdict-row insert into misc.greenlight_pr_state
+  clickhouse_client.py # ClickHouse connection helper for the service's read (SELECT) queries
   pr_hash.py       # eval_hash land-guard: deterministic PR fingerprint hash
   config.py        # PYTORCH_GREENLIGHT_* environment configuration
   guards.py        # single-instance lock + per-iteration SIGALRM timeout + hard watchdog

greenlight/src/greenlight/cli.py

@@ -47,9 +47,9 @@ def build_parser() -> argparse.ArgumentParser:
         "verdict",
         help="record a PR review verdict once and act on GitHub",
         description=(
-            "Record a single PR-review verdict in ClickHouse and, for LAND/NO_LAND, act on the PR. "
-            "Runs once outside the daemon loop and lock. Requires PYTORCH_GREENLIGHT_GITHUB_TOKEN for "
-            "LAND/NO_LAND and CLICKHOUSE_* credentials to write."
+            "Record a single PR-review verdict: emit the row for S3 -> ClickHouse ingestion and, "
+            "for LAND/NO_LAND, post the GitHub review. Runs once outside the daemon loop and lock. "
+            "Requires PYTORCH_GREENLIGHT_GITHUB_TOKEN for LAND/NO_LAND."
         ),
     )
     verdict_parser.add_argument("--repo", default=review.TARGET_REPO, help="owner/name of the repository")

greenlight/src/greenlight/clickhouse_client.py

@@ -1,25 +1,21 @@
-"""ClickHouse access for the greenlight service: connection and verdict-row insert.
+"""ClickHouse connection helper for the greenlight service.
 
-Connection settings come from the standard ``CLICKHOUSE_*`` environment variables.
-Verdict rows are written to ``misc.greenlight_pr_state``; its ``_inserted_at`` column is
-MATERIALIZED server-side and is therefore never part of an insert.
+Connection settings come from the standard ``CLICKHOUSE_*`` environment variables. The
+returned client is used for the service's read (SELECT) queries; verdict writes go
+through the S3 -> replicator path (see ``verdict``), not a direct INSERT from here.
 """
 
 from __future__ import annotations
 
 import logging
 import os
-from dataclasses import astuple, dataclass, fields
-from typing import TYPE_CHECKING, Protocol
+from typing import TYPE_CHECKING
 
 if TYPE_CHECKING:
-    from collections.abc import Sequence
-    from datetime import datetime
+    from clickhouse_connect.driver.client import Client
 
 logger = logging.getLogger(__name__)
 
-TABLE = "misc.greenlight_pr_state"
-
 _DEFAULT_PORT = 8443
 _CLICKHOUSE_CLOUD_DOMAIN = ".clickhouse.cloud"
 _ENV_HELP = (
@@ -28,37 +24,6 @@
 )
 
 
-class ClickHouseClient(Protocol):
-    def insert(
-        self,
-        table: str,
-        data: Sequence[Sequence[object]],
-        *,
-        column_names: Sequence[str],
-    ) -> object: ...
-
-
-@dataclass(frozen=True, slots=True)
-class VerdictRow:
-    repo: str
-    pr_number: int
-    head_sha: str
-    status: str
-    reason: str
-    eval_hash: str
-    message: str
-    eval_job: str
-    agent_job: str
-    version: datetime
-
-
-INSERT_COLUMNS: tuple[str, ...] = tuple(f.name for f in fields(VerdictRow))
-
-
-def insert_verdict_row(client: ClickHouseClient, row: VerdictRow) -> None:
-    client.insert(TABLE, [list(astuple(row))], column_names=list(INSERT_COLUMNS))
-
-
 def _require_env(name: str) -> str:
     value = os.environ.get(name)
     if not value:
@@ -92,15 +57,15 @@ def _ensure_clickhouse_cloud_no_proxy() -> None:
             os.environ[var] = ",".join([*entries, _CLICKHOUSE_CLOUD_DOMAIN])
 
 
-def connect() -> ClickHouseClient:
+def connect() -> Client:
     host = _host_from_env()
     username = _require_env("CLICKHOUSE_USERNAME")
     password = _require_env("CLICKHOUSE_PASSWORD")
     port = _port_from_env()
     _ensure_clickhouse_cloud_no_proxy()
     import clickhouse_connect  # lazy: keeps this module importable without the dep
 
-    client: ClickHouseClient = clickhouse_connect.get_client(
+    client: Client = clickhouse_connect.get_client(
         host=host,
         username=username,
         password=password,