ci: add cross-platform install matrix and API smoke checks#32
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideAdds a cross-platform GitHub Actions workflow to validate installation and API behavior across OSes and Python versions, introduces an optional live Claude/Haiku validation job, replaces the bash-only mock Claude binary with a cross-platform Python-based launcher, fixes validation error status-code handling, adjusts path validation tests, and updates dependencies and developer docs to support the new CI matrix. Sequence diagram for optional live Claude + Haiku validation jobsequenceDiagram
actor Dev as Developer
participant GH as GitHub
participant WF as Workflow_install_matrix
participant J1 as Job_install_check
participant J2 as Job_api_smoke
participant J3 as Job_live_claude_haiku
participant API as Claude_Code_API_Server
participant ClaudeCLI as Claude_CLI
participant Anth as Anthropic_API
Dev->>GH: Trigger workflow_dispatch\nrun_live_claude=true\nlive_model=haiku_id
GH->>WF: Start Install Matrix + API Smoke
WF->>J1: Run install-check matrix
J1->>J1: pip install -e . (pep517 or fallback)
J1->>J1: pip check and import claude_code_api, greenlet, sqlalchemy
J1-->>WF: Install matrix complete
WF->>J2: Run api-smoke matrix
J2->>J2: pip install -e .[test] --use-pep517
J2->>J2: pytest -m not e2e
J2->>API: Start FastAPI app for tests
J2->>API: GET /health and GET /v1/models
API-->>J2: 200 OK responses
J2-->>WF: API smoke complete
WF->>J3: Run live-claude-haiku (manual, continue-on-error)
J3->>J3: Verify ANTHROPIC_API_KEY secret
J3->>ClaudeCLI: Install and configure CLI with API key
J3->>API: Start python -m claude_code_api.main
J3->>API: Poll GET /health until 200 or timeout
J3->>ClaudeCLI: Run pytest tests against live model
ClaudeCLI->>Anth: Call Anthropic model API
Anth-->>ClaudeCLI: Streaming/completed responses
ClaudeCLI-->>J3: Exit status for live tests
J3-->>WF: Job result (non-blocking)
WF-->>GH: Overall workflow status
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello @Mehdi-Bl, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's continuous integration by introducing robust cross-platform installation and API smoke tests. The primary goal is to proactively catch platform-specific regressions and ensure consistent behavior across supported operating systems and Python versions, thereby improving the overall stability and reliability of the application. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Review Summary by QodoAdd cross-platform install matrix and API smoke checks
WalkthroughsDescription• Add cross-platform CI matrix for install validation across Linux/macOS/Windows • Support Python 3.11 and 3.12 with PEP 517 and fallback install modes • Implement deterministic API smoke tests for /health and /v1/models endpoints • Replace bash-only mock Claude binary with cross-platform Python launcher • Clean up deprecated SQLAlchemy imports and HTTP status code fallbacks • Document matrix workflow and optional live Claude validation in dev guide Diagramflowchart LR
A["CI Workflow<br/>install-matrix.yml"] --> B["Install Check<br/>3 OS × 2 Python × 2 modes"]
B --> C["API Smoke Tests<br/>health + models endpoints"]
C --> D["Optional Live<br/>Claude + Haiku"]
E["Mock Claude Binary<br/>conftest.py"] --> C
F["Code Cleanup<br/>database.py + main.py"] --> C
File Changes1. .github/workflows/install-matrix.yml
|
There was a problem hiding this comment.
Code Review
This pull request introduces a cross-platform CI testing matrix to validate installation and basic API functionality on Linux, macOS, and Windows. To support this, the test suite's mock claude binary has been refactored from a bash script into a cross-platform Python script. The changes also include minor fixes for deprecation warnings and documentation updates for the new CI workflow. My review focuses on improving the readability and maintainability of the new test helper script. The changes are solid and the move to a cross-platform test mock is a great improvement.
| runner_code = "\n".join( | ||
| [ | ||
| "#!/usr/bin/env python3", | ||
| "import sys", | ||
| "", | ||
| f"DEFAULT_FIXTURE = {str(default_fixture)!r}", | ||
| f"FIXTURE_RULES = {serialized_rules!r}", | ||
| "", | ||
| "def _extract_prompt(args):", | ||
| " for idx, value in enumerate(args):", | ||
| " if value == '-p' and idx + 1 < len(args):", | ||
| " return args[idx + 1]", | ||
| " return ''", | ||
| "", | ||
| "def _resolve_fixture(prompt):", | ||
| " prompt_lower = prompt.lower()", | ||
| " fixture_path = DEFAULT_FIXTURE", | ||
| " for rule in FIXTURE_RULES:", | ||
| " if any(match in prompt_lower for match in rule['matches']):", | ||
| " fixture_path = rule['fixture_path']", | ||
| " return fixture_path", | ||
| "", | ||
| "def main():", | ||
| " args = sys.argv[1:]", | ||
| " if args and args[0] == '--version':", | ||
| " print('Claude Code 1.0.0')", | ||
| " return 0", | ||
| " prompt = _extract_prompt(args)", | ||
| " fixture_path = _resolve_fixture(prompt)", | ||
| " with open(fixture_path, 'r', encoding='utf-8') as handle:", | ||
| " sys.stdout.write(handle.read())", | ||
| " return 0", | ||
| "", | ||
| "if __name__ == '__main__':", | ||
| " raise SystemExit(main())", | ||
| "", | ||
| ] | ||
| ) |
There was a problem hiding this comment.
For better readability and maintainability, consider using a multiline f-string with textwrap.dedent to define the runner_code. This avoids the need for a list of strings and "\n".join(), making the embedded script look more like a regular Python script. You'll need to add import textwrap at the top of the file for this suggestion to work.
runner_code = textwrap.dedent(f"""
#!/usr/bin/env python3
import sys
DEFAULT_FIXTURE = {str(default_fixture)!r}
FIXTURE_RULES = {serialized_rules!r}
def _extract_prompt(args):
for idx, value in enumerate(args):
if value == '-p' and idx + 1 < len(args):
return args[idx + 1]
return ''
def _resolve_fixture(prompt):
prompt_lower = prompt.lower()
fixture_path = DEFAULT_FIXTURE
for rule in FIXTURE_RULES:
if any(match in prompt_lower for match in rule['matches']):
fixture_path = rule['fixture_path']
return fixture_path
def main():
args = sys.argv[1:]
if args and args[0] == '--version':
print('Claude Code 1.0.0')
return 0
prompt = _extract_prompt(args)
fixture_path = _resolve_fixture(prompt)
with open(fixture_path, 'r', encoding='utf-8') as handle:
sys.stdout.write(handle.read())
return 0
if __name__ == '__main__':
raise SystemExit(main())
""")| launcher_code = "\n".join( | ||
| [ | ||
| "#!/usr/bin/env sh", | ||
| f'exec "{sys.executable}" "{runner_path}" "$@"', | ||
| "", | ||
| ] | ||
| ) |
There was a problem hiding this comment.
For improved readability, you can use a multiline f-string to define the POSIX launcher script. This is more concise than joining a list of strings.
launcher_code = f"""#!/usr/bin/env sh
exec \"{sys.executable}\" \"{runner_path}\" \"$@\"
"""There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `.github/workflows/install-matrix.yml:118` </location>
<code_context>
+
+ live-claude-haiku:
+ name: Live Claude + Haiku validation (manual)
+ if: ${{ github.event_name == 'workflow_dispatch' && inputs.run_live_claude && secrets.ANTHROPIC_API_KEY != '' }}
+ runs-on: ubuntu-latest
+ needs: api-smoke
</code_context>
<issue_to_address>
**issue (bug_risk):** The workflow_dispatch input reference is likely incorrect and will prevent this job from running.
For `workflow_dispatch` events, inputs are exposed as `github.event.inputs.<name>`, not `inputs.<name>`. As a result, `inputs.run_live_claude` will always be falsey and this job will never run. You likely want something like:
```yaml
if: >-
${{ github.event_name == 'workflow_dispatch' && github.event.inputs.run_live_claude == 'true' && secrets.ANTHROPIC_API_KEY != '' }}
```
(adjust the string comparison if the input is passed differently).
</issue_to_address>
### Comment 2
<location> `.github/workflows/install-matrix.yml:137-138` </location>
<code_context>
+ - name: Install project with test extras
+ run: python -m pip install -e ".[test]" --use-pep517
+
+ - name: Install Claude CLI
+ run: curl -fsSL https://claude.ai/install.sh | bash
+
+ - name: Add Claude CLI path
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Piping curl directly to bash introduces a supply-chain risk.
Using `curl -fsSL … | bash` makes the workflow dependent on executing whatever that endpoint serves at runtime. Where possible, prefer a more controlled install path (e.g., versioned release or package manager) and/or pin via checksum or signature verification so a compromised install endpoint can’t silently impact CI runs.
Suggested implementation:
```
- name: Install Claude CLI
env:
CLAUDE_INSTALL_URL: https://claude.ai/install.sh
# TODO: Replace this placeholder with the official SHA256 for the installer at CLAUDE_INSTALL_URL
CLAUDE_INSTALL_SHA256: "<expected-sha256-of-install.sh>"
run: |
set -euo pipefail
curl -fsSL "$CLAUDE_INSTALL_URL" -o /tmp/claude-install.sh
echo "$CLAUDE_INSTALL_SHA256 /tmp/claude-install.sh" | sha256sum -c -
bash /tmp/claude-install.sh
```
1. Determine the official SHA256 checksum for the `https://claude.ai/install.sh` script (e.g., from release notes or by downloading the script once in a controlled environment and computing `sha256sum install.sh`).
2. Replace the `"<expected-sha256-of-install.sh>"` placeholder with that exact checksum value so the workflow will fail if the installer content changes unexpectedly.
3. Optionally, you may want to pin `CLAUDE_INSTALL_URL` to a specific versioned installer URL if the project provides one, further reducing the risk of unexpected changes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| - name: Install Claude CLI | ||
| run: curl -fsSL https://claude.ai/install.sh | bash |
There was a problem hiding this comment.
🚨 suggestion (security): Piping curl directly to bash introduces a supply-chain risk.
Using curl -fsSL … | bash makes the workflow dependent on executing whatever that endpoint serves at runtime. Where possible, prefer a more controlled install path (e.g., versioned release or package manager) and/or pin via checksum or signature verification so a compromised install endpoint can’t silently impact CI runs.
Suggested implementation:
- name: Install Claude CLI
env:
CLAUDE_INSTALL_URL: https://claude.ai/install.sh
# TODO: Replace this placeholder with the official SHA256 for the installer at CLAUDE_INSTALL_URL
CLAUDE_INSTALL_SHA256: "<expected-sha256-of-install.sh>"
run: |
set -euo pipefail
curl -fsSL "$CLAUDE_INSTALL_URL" -o /tmp/claude-install.sh
echo "$CLAUDE_INSTALL_SHA256 /tmp/claude-install.sh" | sha256sum -c -
bash /tmp/claude-install.sh
- Determine the official SHA256 checksum for the
https://claude.ai/install.shscript (e.g., from release notes or by downloading the script once in a controlled environment and computingsha256sum install.sh). - Replace the
"<expected-sha256-of-install.sh>"placeholder with that exact checksum value so the workflow will fail if the installer content changes unexpectedly. - Optionally, you may want to pin
CLAUDE_INSTALL_URLto a specific versioned installer URL if the project provides one, further reducing the risk of unexpected changes.
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewⓘ The new review experience is currently in Beta. Learn more |
| if os.name == "nt": | ||
| launcher_path = Path(temp_dir) / "claude.cmd" | ||
| launcher_code = f'@echo off\r\n"{sys.executable}" "{runner_path}" %*\r\n' | ||
| launcher_path.write_text(launcher_code, encoding="utf-8") | ||
| return str(launcher_path) |
There was a problem hiding this comment.
1. Windows .cmd exec fails 🐞 Bug ⛯ Reliability
On Windows, the new mock Claude launcher returns a .cmd path, but the app spawns the Claude binary with asyncio.create_subprocess_exec (no shell). This is likely to fail on windows-latest, breaking deterministic tests that hit /v1/chat/completions in the new CI matrix.
Agent Prompt
## Issue description
Windows test runs will likely fail because the mock Claude launcher returns a `.cmd` batch script, while the application spawns the Claude binary via `asyncio.create_subprocess_exec` (no shell). Batch scripts generally need a command interpreter (`cmd.exe /c`) to run.
## Issue Context
- The new CI matrix includes `windows-latest`.
- Deterministic tests call `/v1/chat/completions`, which triggers the subprocess spawn.
- `settings.claude_binary_path` may be a `.cmd` on Windows both for the mock and potentially for a real Claude install discovered via PATH.
## Fix Focus Areas
- claude_code_api/core/claude_manager.py[44-105]
- claude_code_api/core/claude_manager.py[355-380]
- claude_code_api/core/claude_manager.py[614-624]
- tests/conftest.py[89-93]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Review Summary by QodoAdd cross-platform install matrix and API smoke checks with test improvements
WalkthroughsDescription• Add cross-platform CI matrix for install validation across Linux/macOS/Windows with Python 3.11-3.12 • Replace bash-only mock Claude binary with cross-platform Python launcher in tests • Add deterministic API smoke tests for /health and /v1/models endpoints • Include optional manual live Claude + Haiku validation job (non-blocking) • Add explicit greenlet runtime dependency and requests test dependency • Fix HTTP 422 status code fallback to numeric constant for compatibility Diagramflowchart LR
A["CI Workflow Trigger"] --> B["Install Matrix Job"]
B --> C["Multi-OS/Python Test"]
C --> D["Dependency Validation"]
D --> E["API Smoke Tests"]
E --> F["Health & Models Check"]
F --> G["Optional Live Claude"]
H["Mock Claude Binary"] --> I["Cross-Platform Launcher"]
I --> J["POSIX & Windows Support"]
File Changes1. .github/workflows/install-matrix.yml
|
|
Persistent review updated to latest commit 4c5a2f1 |
Summary
.github/workflows/install-matrix.yml--use-pep517+ fallbackpip install -e .)pytest -m "not e2e"+ targeted/healthand/v1/modelssmoke tests)run_live_claudeandANTHROPIC_API_KEY)tests/conftest.pydeclarative_baseimport and HTTP 422 constant fallback)docs/dev.mdWhy
PR #27 highlighted a platform-specific install/runtime regression. This adds a CI guardrail to catch install/API regressions early across supported operating systems.
Validation
make lintpytest -q -m "not e2e"make test-no-covmake testmake sonarmake sonar-cloud(fails locally: missingSONAR_CLOUD_TOKENin Vault/environment)python -m pip install -e . --use-pep517 && python -m pip check && python -c "import claude_code_api, greenlet, sqlalchemy"python -m pip uninstall -y claude-code-api && python -m pip install -e . && python -m pip check && python -c "import claude_code_api, greenlet, sqlalchemy"Summary by Sourcery
Add cross-platform CI workflow to validate package installation and API health across OS, Python versions, and install modes, while improving test portability and tightening dependency and validation behavior.
New Features:
Enhancements:
CI:
Documentation: