Repository navigation
Allow headless commands with connected BYOK providers (fixes #22) #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -101,3 +101,79 @@ def test_launch_tui_never_runs_first_time_setup(monkeypatch): | |||||
| m._launch_tui() | ||||||
|
|
||||||
| assert calls == [{"resume_session_id": None}] | ||||||
|
|
||||||
|
|
||||||
| def test_require_auth_passes_for_connected_byok_provider(monkeypatch): | ||||||
| from openhack.config import settings | ||||||
| monkeypatch.setattr(settings, "llm_provider", "openai") | ||||||
| monkeypatch.setattr(settings, "openhack_api_key", None) | ||||||
| monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai") | ||||||
|
|
||||||
| out = io.StringIO() | ||||||
| with contextlib.redirect_stdout(out): | ||||||
| assert m._require_auth() is True | ||||||
| assert out.getvalue() == "" | ||||||
|
|
||||||
|
|
||||||
| def test_require_auth_fails_for_disconnected_byok_provider(monkeypatch): | ||||||
| from openhack.config import settings | ||||||
| monkeypatch.setattr(settings, "llm_provider", "openai") | ||||||
| monkeypatch.setattr(settings, "openhack_api_key", None) | ||||||
| monkeypatch.delenv("OPENAI_API_KEY", raising=False) | ||||||
| monkeypatch.setattr("openhack.provider_auth.all_credentials", lambda: {}) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This test patches Patch
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: tests/test_cli_dispatch.py
Line: 123
Comment:
**Saved credentials break the test**
This test patches `all_credentials()`, but `is_connected()` calls `get_credential()`, which reads saved credentials directly. If a developer has a saved OpenAI API key or ChatGPT login, `_require_auth()` returns `True` and the test fails despite working code.
Patch `openhack.providers.get_credential` to return `None`, or give the test an isolated credential store.
```suggestion
monkeypatch.setattr("openhack.providers.get_credential", lambda name: None)
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly. |
||||||
|
|
||||||
| out = io.StringIO() | ||||||
| with contextlib.redirect_stdout(out): | ||||||
| assert m._require_auth() is False | ||||||
| assert "Error: provider 'openai' is not configured." in out.getvalue() | ||||||
| assert "OPENAI_API_KEY" in out.getvalue() | ||||||
|
|
||||||
|
|
||||||
| def test_require_auth_fails_for_openhack_when_logged_out(monkeypatch): | ||||||
| from openhack.config import settings | ||||||
| monkeypatch.setattr(settings, "llm_provider", "openhack") | ||||||
| monkeypatch.setattr(settings, "openhack_api_key", None) | ||||||
| monkeypatch.delenv("OPENHACK_API_KEY", raising=False) | ||||||
| monkeypatch.setattr("openhack.config.load_user_config", lambda: {}) | ||||||
|
|
||||||
| out = io.StringIO() | ||||||
| with contextlib.redirect_stdout(out): | ||||||
| assert m._require_auth() is False | ||||||
| assert "Error: not logged in." in out.getvalue() | ||||||
|
|
||||||
|
|
||||||
| def test_require_auth_passes_for_openhack_when_logged_in(monkeypatch): | ||||||
| from openhack.config import settings | ||||||
| monkeypatch.setattr(settings, "llm_provider", "openhack") | ||||||
| monkeypatch.setattr(settings, "openhack_api_key", "sk-openhack-test") | ||||||
|
|
||||||
| out = io.StringIO() | ||||||
| with contextlib.redirect_stdout(out): | ||||||
| assert m._require_auth() is True | ||||||
| assert out.getvalue() == "" | ||||||
|
|
||||||
|
|
||||||
| def test_cmd_scan_runs_with_byok_provider_without_openhack_key(monkeypatch, tmp_path): | ||||||
| from openhack.config import settings | ||||||
| monkeypatch.setattr(settings, "llm_provider", "openai") | ||||||
| monkeypatch.setattr(settings, "openhack_api_key", None) | ||||||
| monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai") | ||||||
|
|
||||||
| target_dir = tmp_path / "sample-project" | ||||||
| target_dir.mkdir() | ||||||
| monkeypatch.setattr("sys.argv", ["openhack", "--scan", str(target_dir)]) | ||||||
|
|
||||||
| scan_invoked = [] | ||||||
|
|
||||||
| async def _fake_run(target): | ||||||
| scan_invoked.append(target) | ||||||
|
|
||||||
| monkeypatch.setattr("openhack.headless_scan.run_headless_scan", _fake_run) | ||||||
|
|
||||||
| out = io.StringIO() | ||||||
| with contextlib.redirect_stdout(out): | ||||||
| m._cmd_scan() | ||||||
|
|
||||||
| assert scan_invoked == [str(target_dir)] | ||||||
| assert "Error: not logged in" not in out.getvalue() | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.envsettingsIf
LLM_PROVIDER=ollamais set in.env, with no saved provider orOLLAMA_API_KEY,_require_auth()rejects a valid keyless setup.Settingsreads.env, butis_connected()checks only saved config andos.environ. All four headless commands stop and ask for a key that Ollama does not need. This also breaks runs that previously passed with an OpenHack key.Use the resolved provider selection for the keyless check and add a
.envregression test.Prompt To Fix With AI