Deployments that serve the marketplace from a different host than the
API (reverse-proxy split, CDN-fronted marketplace, etc.) now bootstrap
cleanly. Default behaviour unchanged — falls back to
`{server_host}/marketplace.git/` derived from the workspace's
bootstrapped server URL.
Implementation:
- AGNES_MARKETPLACE_URL parsed via urlparse; missing scheme/host
fail-fast with a clear error rather than silently falling back, so
operator misconfigurations surface immediately.
- PAT is re-injected into the override URL the same way as the default
path (user-info form of HTTP basic for git clone), then stripped on
the post-clone `remote set-url` so it doesn't sit at rest in
.git/config.
- Trailing slash normalized so urljoin-style consumers don't drop the
path. Path defaults to /marketplace.git/ if the override omits it.
Help text updated to document the override.
Tests: 2 new in test_cli_refresh_marketplace.py (env override happy
path + invalid URL rejection). 46/46 refresh_marketplace passes.
This commit is contained in:
parent
c910281df1
commit
82a4b663d7
3 changed files with 122 additions and 16 deletions
|
|
@ -10,6 +10,9 @@ CalVer image tags (`stable-YYYY.MM.N`, `dev-YYYY.MM.N`) are produced for every C
|
||||||
|
|
||||||
## [Unreleased]
|
## [Unreleased]
|
||||||
|
|
||||||
|
### Added
|
||||||
|
- `AGNES_MARKETPLACE_URL` env override for `agnes refresh-marketplace --bootstrap`. Pre-fix the marketplace endpoint was hardcoded to `{server_host}/marketplace.git/`, which broke deployments that serve the marketplace from a different host than the API (reverse-proxy split, CDN-fronted marketplace). When set, the env var is parsed via `urlparse`; missing scheme or host fails fast with a clear error (operator misconfiguration surfaces immediately). The PAT injection / strip behavior is preserved on the override path. Default behavior unchanged when the env var is empty / unset. Closes #345 A.
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
- `agnes query --json` is now a shortcut for `--format json` — paste-prompts and LLM-assisted analysts routinely reach for `--json` first, and the typer "Did you mean `--stdin`?" suggestion the missing flag previously produced was actively misleading. `--json --format <other>` is rejected as mutually exclusive (`--json --format json` is redundantly allowed). Closes #345 D.
|
- `agnes query --json` is now a shortcut for `--format json` — paste-prompts and LLM-assisted analysts routinely reach for `--json` first, and the typer "Did you mean `--stdin`?" suggestion the missing flag previously produced was actively misleading. `--json --format <other>` is rejected as mutually exclusive (`--json --format json` is redundantly allowed). Closes #345 D.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -76,7 +76,10 @@ def refresh_marketplace(
|
||||||
"If no marketplace clone exists yet, clone it and register the "
|
"If no marketplace clone exists yet, clone it and register the "
|
||||||
"local path with Claude Code. Used by the install flow as a "
|
"local path with Claude Code. Used by the install flow as a "
|
||||||
"one-liner replacement for an inline `git clone` + chmod + "
|
"one-liner replacement for an inline `git clone` + chmod + "
|
||||||
"`claude plugin marketplace add` sequence."
|
"`claude plugin marketplace add` sequence. Marketplace URL "
|
||||||
|
"defaults to {server_host}/marketplace.git/; override via the "
|
||||||
|
"AGNES_MARKETPLACE_URL env var when the marketplace lives on "
|
||||||
|
"a different host than the API."
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
):
|
):
|
||||||
|
|
@ -177,6 +180,34 @@ def _bootstrap_clone(token: str) -> bool:
|
||||||
in plaintext at `.git/config` (refreshes use the credential helper).
|
in plaintext at `.git/config` (refreshes use the credential helper).
|
||||||
Returns False on any failure.
|
Returns False on any failure.
|
||||||
"""
|
"""
|
||||||
|
# Marketplace URL resolution (issue #345 A):
|
||||||
|
# 1. `AGNES_MARKETPLACE_URL` env var — explicit override for deployments
|
||||||
|
# that serve the marketplace from a different host than the API
|
||||||
|
# (reverse-proxy split, CDN-fronted marketplace, etc.). Must be a
|
||||||
|
# full base URL including scheme, ending at the `/marketplace.git/`
|
||||||
|
# path or its equivalent on the operator's deployment.
|
||||||
|
# 2. Fallback to ``{scheme}://{server_host}/marketplace.git/`` derived
|
||||||
|
# from the workspace's bootstrapped server URL.
|
||||||
|
marketplace_base = os.environ.get("AGNES_MARKETPLACE_URL", "").strip()
|
||||||
|
if marketplace_base:
|
||||||
|
parsed = urlparse(marketplace_base)
|
||||||
|
if not parsed.scheme or not parsed.hostname:
|
||||||
|
typer.echo(
|
||||||
|
f"error: AGNES_MARKETPLACE_URL={marketplace_base!r} is not a full URL "
|
||||||
|
"(expected scheme://host[:port]/marketplace.git/).",
|
||||||
|
err=True,
|
||||||
|
)
|
||||||
|
return False
|
||||||
|
# Normalize: ensure trailing slash so urljoin-style consumers don't drop the path.
|
||||||
|
clean_url = marketplace_base if marketplace_base.endswith("/") else marketplace_base + "/"
|
||||||
|
scheme = parsed.scheme
|
||||||
|
# Re-inject the PAT into the env-supplied URL so the clone authenticates.
|
||||||
|
host_with_port = parsed.netloc.split("@", 1)[-1]
|
||||||
|
path = parsed.path if parsed.path else "/marketplace.git/"
|
||||||
|
if not path.endswith("/"):
|
||||||
|
path += "/"
|
||||||
|
auth_url = f"{scheme}://x:{token}@{host_with_port}{path}"
|
||||||
|
else:
|
||||||
server_url = get_server_url()
|
server_url = get_server_url()
|
||||||
if not server_url:
|
if not server_url:
|
||||||
typer.echo("error: no server URL configured; run `agnes init` first.", err=True)
|
typer.echo("error: no server URL configured; run `agnes init` first.", err=True)
|
||||||
|
|
@ -190,6 +221,8 @@ def _bootstrap_clone(token: str) -> bool:
|
||||||
if parsed.port:
|
if parsed.port:
|
||||||
server_host = f"{server_host}:{parsed.port}"
|
server_host = f"{server_host}:{parsed.port}"
|
||||||
scheme = parsed.scheme or "https"
|
scheme = parsed.scheme or "https"
|
||||||
|
auth_url = f"{scheme}://x:{token}@{server_host}/marketplace.git/"
|
||||||
|
clean_url = f"{scheme}://{server_host}/marketplace.git/"
|
||||||
|
|
||||||
# Stale dir without a `.git/` subdir means an interrupted prior install;
|
# Stale dir without a `.git/` subdir means an interrupted prior install;
|
||||||
# remove it so the fresh clone has somewhere to land.
|
# remove it so the fresh clone has somewhere to land.
|
||||||
|
|
@ -202,9 +235,6 @@ def _bootstrap_clone(token: str) -> bool:
|
||||||
|
|
||||||
CLONE_DIR.parent.mkdir(parents=True, exist_ok=True)
|
CLONE_DIR.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
|
||||||
auth_url = f"{scheme}://x:{token}@{server_host}/marketplace.git/"
|
|
||||||
clean_url = f"{scheme}://{server_host}/marketplace.git/"
|
|
||||||
|
|
||||||
typer.echo(f"Cloning marketplace from {clean_url} into {CLONE_DIR}...")
|
typer.echo(f"Cloning marketplace from {clean_url} into {CLONE_DIR}...")
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
|
|
||||||
|
|
@ -616,6 +616,79 @@ def test_bootstrap_with_no_existing_clone_clones_and_registers(
|
||||||
assert add_calls[0].cmd[4] == str(clone_target)
|
assert add_calls[0].cmd[4] == str(clone_target)
|
||||||
|
|
||||||
|
|
||||||
|
def test_bootstrap_honors_marketplace_url_env_override(
|
||||||
|
tmp_path, monkeypatch, with_token, claude_in_path, recorder,
|
||||||
|
):
|
||||||
|
"""``AGNES_MARKETPLACE_URL`` overrides the derived ``server_host/marketplace.git/``
|
||||||
|
base for deployments that serve the marketplace from a different host
|
||||||
|
than the API — reverse-proxy split, CDN-fronted marketplace, etc.
|
||||||
|
Issue #345 A.
|
||||||
|
"""
|
||||||
|
cfg_dir = tmp_path / "_cfg"
|
||||||
|
(cfg_dir / "config.yaml").write_text(
|
||||||
|
"server: https://agnes.example.com\n", encoding="utf-8",
|
||||||
|
)
|
||||||
|
monkeypatch.setenv("AGNES_MARKETPLACE_URL", "https://plugins.example.com/marketplace.git/")
|
||||||
|
|
||||||
|
clone_target = tmp_path / "fresh_marketplace"
|
||||||
|
monkeypatch.setattr(rm_module, "CLONE_DIR", clone_target)
|
||||||
|
|
||||||
|
real_run = recorder.run
|
||||||
|
|
||||||
|
def fake_run(cmd, *args, **kwargs):
|
||||||
|
if cmd[:2] == ["git", "clone"]:
|
||||||
|
(clone_target / ".git").mkdir(parents=True, exist_ok=True)
|
||||||
|
(clone_target / ".claude-plugin").mkdir(parents=True, exist_ok=True)
|
||||||
|
(clone_target / ".claude-plugin" / "marketplace.json").write_text(
|
||||||
|
json.dumps({"name": "agnes", "plugins": []}),
|
||||||
|
encoding="utf-8",
|
||||||
|
)
|
||||||
|
return real_run(cmd, *args, **kwargs)
|
||||||
|
|
||||||
|
monkeypatch.setattr(rm_module.subprocess, "run", fake_run)
|
||||||
|
|
||||||
|
result = runner.invoke(refresh_marketplace_app, ["--bootstrap"])
|
||||||
|
assert result.exit_code == 0, result.output
|
||||||
|
|
||||||
|
# Clone URL must point at the env-override host, NOT at the
|
||||||
|
# api server's hostname, and must carry the PAT.
|
||||||
|
clone_calls = [c for c in recorder.calls if c.cmd[:2] == ["git", "clone"]]
|
||||||
|
assert len(clone_calls) == 1
|
||||||
|
url_arg = next(a for a in clone_calls[0].cmd if a.startswith("https://"))
|
||||||
|
assert "plugins.example.com/marketplace.git/" in url_arg
|
||||||
|
assert "agnes.example.com" not in url_arg
|
||||||
|
assert with_token in url_arg
|
||||||
|
|
||||||
|
# PAT-stripped URL after clone is also the override host.
|
||||||
|
set_url_calls = [
|
||||||
|
c for c in recorder.calls
|
||||||
|
if c.cmd[:5] == ["git", "-C", str(clone_target), "remote", "set-url"]
|
||||||
|
]
|
||||||
|
assert len(set_url_calls) == 1
|
||||||
|
new_url = set_url_calls[0].cmd[6]
|
||||||
|
assert "plugins.example.com/marketplace.git/" in new_url
|
||||||
|
assert with_token not in new_url
|
||||||
|
|
||||||
|
|
||||||
|
def test_bootstrap_rejects_invalid_marketplace_url_env(
|
||||||
|
tmp_path, monkeypatch, with_token, claude_in_path,
|
||||||
|
):
|
||||||
|
"""``AGNES_MARKETPLACE_URL`` without scheme is rejected with a clear
|
||||||
|
error — silent fallback to the derived URL would hide an operator
|
||||||
|
misconfiguration. Issue #345 A.
|
||||||
|
"""
|
||||||
|
cfg_dir = tmp_path / "_cfg"
|
||||||
|
(cfg_dir / "config.yaml").write_text(
|
||||||
|
"server: https://agnes.example.com\n", encoding="utf-8",
|
||||||
|
)
|
||||||
|
monkeypatch.setenv("AGNES_MARKETPLACE_URL", "plugins.example.com/marketplace.git/")
|
||||||
|
monkeypatch.setattr(rm_module, "CLONE_DIR", tmp_path / "fresh_marketplace")
|
||||||
|
|
||||||
|
result = runner.invoke(refresh_marketplace_app, ["--bootstrap"])
|
||||||
|
assert result.exit_code == 1
|
||||||
|
assert "AGNES_MARKETPLACE_URL" in result.output
|
||||||
|
|
||||||
|
|
||||||
def test_bootstrap_clone_failure_exits_nonzero(
|
def test_bootstrap_clone_failure_exits_nonzero(
|
||||||
tmp_path, monkeypatch, with_token, claude_in_path, recorder,
|
tmp_path, monkeypatch, with_token, claude_in_path, recorder,
|
||||||
):
|
):
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue