Skip to content

CI: fail fast on expired/wrong-org API key - #97

Merged
chris-price19 merged 1 commit into
mainfrom
ci/auth-preflight-hardening
Aug 10, 2026
Merged

CI: fail fast on expired/wrong-org API key#97
chris-price19 merged 1 commit into
mainfrom
ci/auth-preflight-hardening

Conversation

@chris-price19

Copy link
Copy Markdown
Contributor

Why

The live test suite runs against prod, gated on a manually-rotated AS_API_KEY secret. When that key expires, becomes invalid, or is scoped to the wrong organization, the failure surfaces in a confusing way that looks like a code bug:

  • Wrong org / no data: test_get loops over data types and does except ClientError: continue, then reports pytest.fail("No data_id found for required data type 'rheed_image'") — an auth/org problem disguised as missing data.
  • Bad record / transient: an opaque HTTP 500 deep inside the parallel fan-out.

Neither points at the real cause (the key), and both only appear after the full 3×4 matrix has spun up.

What

  1. preflight CI job (new) — runs once after lint, before the test matrix (test now needs: preflight). It authenticates and checks the required data types are present, then fails fast with an actionable message that distinguishes the two failure modes:
    • 401/403 → "AS_API_KEY is expired, invalid, or malformed — refresh the AS_API_KEY repository secret."
    • auth OK but a required type returns 0 rows → "key is scoped to the WRONG organization — use a key for the org that holds the test corpus."
    • Backed by scripts/ci_auth_preflight.py (kept in sync with test_get's required_types).
  2. test_get no longer hides auth errors — a 401/403 from search() is re-raised as a clear pytest.fail instead of being swallowed by except ClientError: continue. Other ClientErrors (e.g. an alias unsupported for the org) stay benign and try the next alias.
  3. ruffT201 (print) ignored under scripts/, since CLI status scripts print by design.

Effect

  • An expired / wrong-org key now fails in ~1 minute with a message that says exactly what to do, before the matrix runs.
  • A genuine code regression still fails the same way it does today.
  • No change to what the tests assert.

Notes for reviewer

  • The preflight installs the package (uv pip install --system -e .) and hits https://api.atomscale.ai with secrets.AS_API_KEY, same as the test job.
  • Verified locally: pre-commit run passes ruff / ruff-format / codespell on the changed files; the script exits 1 with a clean message when no key is set. (ty only checks src/atomicds, untouched here.)
  • Follow-up options not included here: a scheduled key-expiry canary that opens an issue proactively, and pinning curated ResultIDs so a newly-ingested bad prod record can't wedge test_get.

🤖 Generated with Claude Code

Live tests run against prod gated on a manually-rotated API key, so an
expired/invalid or wrong-org key currently surfaces as a confusing failure
deep inside the suite — either a swallowed ClientError reported as
"No data_id found" (test_get) or an opaque HTTP 500.

- Add scripts/ci_auth_preflight.py and a `preflight` CI job (runs once
  before the 3x4 matrix). It authenticates and checks the required data
  types exist, failing fast with an actionable message that distinguishes
  an expired/invalid key (401/403) from a wrong-org key (auth OK, no data).
- test_get: re-raise 401/403 from search() as a clear pytest.fail instead
  of swallowing it via `except ClientError: continue` (other errors, e.g.
  an alias unsupported for the org, stay benign).
- ruff: ignore T201 under scripts/ (CLI status scripts print by design).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds a preflight CI gate that validates the production API key and required test corpus before launching the live-test matrix. It also surfaces authentication failures in test_get and permits status output from scripts.

  • Adds a single preflight job between linting and the test matrix.
  • Checks authentication and required data-type availability through a new CLI script.
  • Stops test_get from swallowing 401/403 responses.
  • Adds a Ruff exception for intentional printing under scripts/.

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking diagnostic gap for transport-level API failures in the new preflight.

The authentication and corpus checks correctly gate the matrix, but DNS, TLS, timeout, and connection failures escape the new ClientError handler and produce an unhandled traceback instead of an actionable preflight message.

Files Needing Attention: scripts/ci_auth_preflight.py

Important Files Changed

Filename Overview
.github/workflows/testing.yml Adds a preflight job after lint and makes the live-test matrix depend on its success.
scripts/ci_auth_preflight.py Validates API authentication and corpus availability, but transport exceptions bypass its diagnostic handling.
tests/test_client.py Converts search authentication responses into an explicit pytest failure while retaining fallback behavior for other HTTP errors.
pyproject.toml Allows intentional print calls in scripts without changing lint behavior elsewhere.

Sequence Diagram

sequenceDiagram
    participant CI as GitHub Actions
    participant P as Preflight script
    participant API as Atomscale API
    participant T as Test matrix
    CI->>P: Run after lint
    loop Required data types
        P->>API: "search(type, status=success)"
        API-->>P: Results or error
    end
    alt Authentication/corpus check passes
        P-->>CI: Exit 0
        CI->>T: Start matrix
    else Check fails
        P-->>CI: Exit 1
        CI--xT: Skip matrix
    end
Loading

Fix All in Claude Code Fix All in Conductor

Prompt To Fix All With AI
### Issue 1
scripts/ci_auth_preflight.py:43-44
**Transport failures bypass diagnostics**

When DNS, TLS, timeout, or connection errors occur, `client.search()` raises a transport exception rather than `ClientError`, so the preflight emits a raw traceback and skips the test matrix without the actionable diagnostic this job is intended to provide.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "Add CI auth preflight and surface auth e..." | Re-trigger Greptile

Comment on lines +43 to +44
data = client.search(data_type=data_type, status="success")
except ClientError as exc:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Transport failures bypass diagnostics

When DNS, TLS, timeout, or connection errors occur, client.search() raises a transport exception rather than ClientError, so the preflight emits a raw traceback and skips the test matrix without the actionable diagnostic this job is intended to provide.

Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/ci_auth_preflight.py
Line: 43-44

Comment:
**Transport failures bypass diagnostics**

When DNS, TLS, timeout, or connection errors occur, `client.search()` raises a transport exception rather than `ClientError`, so the preflight emits a raw traceback and skips the test matrix without the actionable diagnostic this job is intended to provide.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Conductor

@chris-price19
chris-price19 merged commit 8bb16d9 into main Aug 10, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants