fix(ci): repair 3 main-branch CI failures after upstream sync 0c5916f - #2
Merged
Merged
Conversation
Three run failures were traced back to the plastic-labs:main sync (merge 0c5916f) and to secrets that are not configured on the fork: 1) Static Analysis (run 30859200823) — basedpyright src/routers/conclusions.py:113:13 reported 'Implicit string concatenation not allowed (reportImplicitStringConcatenation)' and exited 1. The four-line literal block in query_conclusions() came from PR #1. Replaced the implicit concatenation with explicit '+' between literals, matching the style used elsewhere in this file. 2) FastAPI Tests with PostgreSQL and uv (run 30859200793) The postgres log spammed FATAL: role "root" does not exist because pg_isready's health check ran without '-U postgres' and defaulted to the OS user (root). Tightened the health-cmd to 'pg_isready -U postgres' so it authenticates against the POSTGRES_USER=postgres role defined by the service. While there, the actual single-test failure tests/crud/test_document.py::TestDocumentCRUD:: test_duplicate_rejection_reinforces_existing src/embedding_client.py:200 ValueError: OpenAI API key is required was caused by LLM_OPENAI_API_KEY/LLM_ANTHROPIC_API_KEY resolving to empty when secrets are absent on the fork. Added '|| "test-key"' fallbacks (matching the existing LLM_OPENAI_COMPATIBLE_API_KEY=test-key pattern). The fallback is inert for tests that pass embeddings directly and only matters when the embedding client is lazily initialized — production secrets still win when present. 3) Unified Tests (Fly Runner) (run 30859200927) Start Fly Runner / Start Fly Runner failed with Error: no access token available. Please login with 'flyctl auth login' because FLY_API_TOKEN was bound but empty. The workflow already references secrets.FLY_API_TOKEN_TESTING correctly — the secret simply isn't defined on the fork. Added a fail-fast 'Verify FLY_API_TOKEN is configured' step that emits ::error:: annotations naming the missing secret and exits 1 with an actionable message instead of letting flyctl die with the generic auth error. Austin must add these secrets to bbasketballer75/honcho (Settings > Secrets and variables > Actions) for the Fly Runner suite to actually run end-to-end: FLY_API_TOKEN_TESTING GH_TOKEN_ACTIONS OPENAI_API_KEY (optional — only needed if real embedding calls should be made; the test-key fallback already lets the suite pass) ANTHROPIC_API_KEY (same as OPENAI_API_KEY)
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Repairs three CI regressions introduced after an upstream sync by adjusting a pyright-triggering Python string, improving PostgreSQL service health checks, and making fork-friendly workflow behavior around missing secrets.
Changes:
- Fixes basedpyright’s implicit string concatenation warning in
query_conclusionsby making concatenation explicit. - Updates the Postgres service health check to use the
postgresuser to avoid CI noise/failures. - Adds safe defaults / fail-fast behavior in workflows when fork secrets are not configured.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/routers/conclusions.py |
Makes the ValidationException message use explicit + concatenation to satisfy basedpyright. |
.github/workflows/unittest.yml |
Uses pg_isready -U postgres for service health and provides test-key fallbacks for missing LLM secrets on forks. |
.github/workflows/start-fly-runner.yml |
Adds an early, clear failure when FLY_API_TOKEN_TESTING is not configured to prevent opaque flyctl failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the 3 main-branch CI breakages that appeared after the plastic-labs:main sync (merge 0c5916f):
src/routers/conclusions.py:113(PR #1)+between literalspg_isreadyhealth check defaulted to OS userrootinstead ofpostgres; (b)LLM_OPENAI_API_KEY/LLM_ANTHROPIC_API_KEYempty because secrets aren't configured on the fork--health-cmd "pg_isready -U postgres"; (b) `secrets.XFLY_API_TOKENenv was bound tosecrets.FLY_API_TOKEN_TESTINGbut the secret isn't configured on the fork::error::annotations so the missing secret is named clearly instead of dying insideflyctlFiles modified
src/routers/conclusions.py— reformat implicit string concatenation at line 113 (4-line ValidationException message → explicit+joins).github/workflows/unittest.yml— fixpg_isreadyhealth check, add API-key fallbacks for fork runs.github/workflows/start-fly-runner.yml— add fail-fast check forFLY_API_TOKEN_TESTINGManual follow-up for Austin
The workflow references are correct, but several secrets aren't defined on
bbasketballer75/honcho. They have to be added manually in Settings → Secrets and variables → Actions before the affected workflows can run end-to-end:FLY_API_TOKEN_TESTINGGH_TOKEN_ACTIONSGH_TOKENfor runner registration)flyctl secrets setstep will still run but registration will fail laterOPENAI_API_KEYtest-keyfallback already lets the suite pass — only add if you want real embedding callsANTHROPIC_API_KEYOPENAI_API_KEYThe CI failures were also producing these log noise lines that the pg_isready fix eliminates:
The
role "root"lines werepg_isreadyreconnect attempts (default user = OS user = root). Thedatabase ... does not existlines are pytest-xdist workers briefly connecting before their per-worker DB was created by thedb_enginesession fixture — those are benign and shouldn't be silenced.Verification
(The full
uv run basedpyrightrun shows one unrelateduvlooperror onsrc/deriver/__main__.py:5— uvloop is Linux/macOS only; Windows runner. Out of scope for this PR.)