fix(cli): I1+I2 review — surface manifest_unauthorized + add 3 typed-error tests
This commit is contained in:
parent
9b70ca3069
commit
b799aa534a
2 changed files with 104 additions and 0 deletions
|
|
@ -186,6 +186,22 @@ def init(
|
||||||
}}), err=True)
|
}}), err=True)
|
||||||
raise typer.Exit(1)
|
raise typer.Exit(1)
|
||||||
|
|
||||||
|
# `run_pull` records per-stage failures into `result.errors` and only
|
||||||
|
# raises for programming errors. A manifest-stage failure here means
|
||||||
|
# the analyst has a saved token + saved server URL but no parquets,
|
||||||
|
# no DuckDB views — surface a typed error so the operator knows the
|
||||||
|
# workspace is not actually queryable. Common cause: PAT validates
|
||||||
|
# against /api/catalog/tables but lacks resource_grants for any tables.
|
||||||
|
manifest_err = next((e for e in result.errors if e.get("stage") == "manifest"), None)
|
||||||
|
if manifest_err:
|
||||||
|
typer.echo(render_error(0, {"detail": {
|
||||||
|
"kind": "manifest_unauthorized",
|
||||||
|
"hint": "Manifest fetch failed — workspace partially set up. "
|
||||||
|
"Check that the PAT has resource_grants for at least one table.",
|
||||||
|
"message": manifest_err.get("error", ""),
|
||||||
|
}}), err=True)
|
||||||
|
raise typer.Exit(1)
|
||||||
|
|
||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
# Step 8: render AGNES_WORKSPACE.md from the static client-side
|
# Step 8: render AGNES_WORKSPACE.md from the static client-side
|
||||||
# template. Three placeholders: created_at, server_url, workspace_path.
|
# template. Three placeholders: created_at, server_url, workspace_path.
|
||||||
|
|
|
||||||
|
|
@ -131,3 +131,91 @@ def test_init_partial_state_friendly_exit(tmp_path, monkeypatch):
|
||||||
])
|
])
|
||||||
assert result.exit_code == 1
|
assert result.exit_code == 1
|
||||||
assert "Traceback" not in (result.output + (result.stderr or ""))
|
assert "Traceback" not in (result.output + (result.stderr or ""))
|
||||||
|
|
||||||
|
|
||||||
|
def test_init_auth_failed_on_401(tmp_path, monkeypatch):
|
||||||
|
"""PAT verify endpoint returns 401 -> auth_failed typed error, exit 1."""
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
monkeypatch.setenv("AGNES_CONFIG_DIR", str(tmp_path / "_cfg"))
|
||||||
|
|
||||||
|
def _api_get(path, *args, **kwargs):
|
||||||
|
resp = MagicMock()
|
||||||
|
resp.status_code = 401
|
||||||
|
resp.json.return_value = {"detail": "Invalid token"}
|
||||||
|
return resp
|
||||||
|
|
||||||
|
monkeypatch.setattr("cli.commands.init.api_get", _api_get, raising=False)
|
||||||
|
|
||||||
|
result = runner.invoke(init_app, [
|
||||||
|
"--server-url", "http://x",
|
||||||
|
"--token", "bad-pat",
|
||||||
|
"--workspace", str(tmp_path),
|
||||||
|
])
|
||||||
|
assert result.exit_code == 1
|
||||||
|
output = result.output + (result.stderr or "")
|
||||||
|
assert "Traceback" not in output
|
||||||
|
# Typed-error envelope should mention the kind or the actionable hint.
|
||||||
|
assert ("auth_failed" in output) or ("Token expired" in output) or ("Token format invalid" in output)
|
||||||
|
|
||||||
|
|
||||||
|
def test_init_server_unreachable_on_connect_error(tmp_path, monkeypatch):
|
||||||
|
"""Network failure during verify -> server_unreachable typed error, exit 1."""
|
||||||
|
monkeypatch.setenv("AGNES_CONFIG_DIR", str(tmp_path / "_cfg"))
|
||||||
|
|
||||||
|
def _api_get(path, *args, **kwargs):
|
||||||
|
raise ConnectionError("simulated network failure")
|
||||||
|
|
||||||
|
monkeypatch.setattr("cli.commands.init.api_get", _api_get, raising=False)
|
||||||
|
|
||||||
|
result = runner.invoke(init_app, [
|
||||||
|
"--server-url", "http://unreachable.example.com",
|
||||||
|
"--token", "test-pat",
|
||||||
|
"--workspace", str(tmp_path),
|
||||||
|
])
|
||||||
|
assert result.exit_code == 1
|
||||||
|
output = result.output + (result.stderr or "")
|
||||||
|
assert "Traceback" not in output
|
||||||
|
assert ("server_unreachable" in output) or ("Cannot reach" in output)
|
||||||
|
|
||||||
|
|
||||||
|
def test_init_manifest_unauthorized_when_pull_records_manifest_error(tmp_path, monkeypatch):
|
||||||
|
"""Manifest stage fails -> manifest_unauthorized typed error, exit 1.
|
||||||
|
|
||||||
|
Reproduces the I1 review finding: `run_pull` records per-stage failures
|
||||||
|
into `result.errors` rather than raising. Without the post-pull error
|
||||||
|
inspection, init would silently exit 0 with a partially-set-up workspace.
|
||||||
|
"""
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
from cli.lib.pull import PullResult
|
||||||
|
|
||||||
|
monkeypatch.setenv("AGNES_CONFIG_DIR", str(tmp_path / "_cfg"))
|
||||||
|
|
||||||
|
# init's verify-PAT call succeeds; welcome-fetch succeeds.
|
||||||
|
def _init_api_get(path, *args, **kwargs):
|
||||||
|
resp = MagicMock()
|
||||||
|
resp.status_code = 200
|
||||||
|
if path == "/api/welcome":
|
||||||
|
resp.json.return_value = {"content": "# Test\n"}
|
||||||
|
else:
|
||||||
|
resp.json.return_value = []
|
||||||
|
return resp
|
||||||
|
|
||||||
|
monkeypatch.setattr("cli.commands.init.api_get", _init_api_get, raising=False)
|
||||||
|
|
||||||
|
# run_pull returns a PullResult carrying a manifest-stage error.
|
||||||
|
def _fake_run_pull(server_url, token, workspace, *, dry_run=False):
|
||||||
|
result = PullResult()
|
||||||
|
result.errors.append({"stage": "manifest", "error": "401 Unauthorized"})
|
||||||
|
return result
|
||||||
|
|
||||||
|
monkeypatch.setattr("cli.commands.init.run_pull", _fake_run_pull, raising=False)
|
||||||
|
|
||||||
|
result = runner.invoke(init_app, [
|
||||||
|
"--server-url", "http://x",
|
||||||
|
"--token", "t",
|
||||||
|
"--workspace", str(tmp_path),
|
||||||
|
])
|
||||||
|
assert result.exit_code == 1
|
||||||
|
output = result.output + (result.stderr or "")
|
||||||
|
assert "Traceback" not in output
|
||||||
|
assert ("manifest_unauthorized" in output) or ("Manifest fetch failed" in output)
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue