Conversation
Model multiple named contexts in credentials.yaml, each carrying its own engine/coordinator/dashboard URLs, a cloud-or-self-hosted kind, auth or an API key, and org/project selection. A legacy single-context file migrates to a `default` context on load; the active context is projected onto the existing top-level fields so current readers keep working. Persist account_id so a later foreign-secret pre-flight can compare owners. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rd [DEP-238] Add the context module: fetch the installation discovery document at /.well-known/arcade, classify a host as cloud or self-hosted, resolve the active context from either the CI environment (ARCADE_URL + ARCADE_API_KEY) or the credentials file, and refuse to contact an Arcade Cloud host while a self-hosted context is active. Route engine/coordinator URL construction and auth headers through the active context so a self-hosted target is selected without flags and CI supplies a bearer API key. Save OAuth credentials under a named context. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g [DEP-238] Add `arcade login --url <install-url>` (and ARCADE_URL): fetch discovery, report the reason and exit non-zero when deployments are off, refuse before sending anything when no discovery document exists, sign in against the discovered coordinator, and save a named context. Add `arcade context list|use|show` to manage contexts. Deploy, secret, org, and project now target the active context with no flags, unauthenticated deploy names the login command, and ARCADE_URL + ARCADE_API_KEY complete a deploy with no terminal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…238] Unit tests for the legacy-to-default migration and named-context round trip, discovery parsing and the predates-discovery path, cloud-host detection, the no-Cloud guard, login by URL, the context command, unauthenticated deploy naming the login command, and a CI-environment deploy driven without prompts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…238] In CI mode there is no OAuth credentials file and no saved org/project, so the org-scoped URL builder cannot run. Route API-key requests to the engine's credential-scoped routes instead — /v1/deployments, /v1/workers/:id, and /v1/admin/secrets — where the project API key resolves the project server-side. The regression test drives a CI deploy through the real URL builder rather than mocking past it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Current version of PR was reviewed by /review-bugbot on Sep 14, 19:20. It flagged 0 findings. Bugbot on commit |
Treat a deploy that ends in `failed` or `degraded` as a non-zero outcome: `degraded` reports that the previous release is still serving, and both raise so the command exits non-zero instead of returning success. Skip the "view full logs?" prompt when there is no interactive terminal so a CI deploy completes without blocking on input. The status poll now recognises `degraded` as a terminal state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
9b299ab to
5532fe1
Compare
|
Current version of PR was reviewed by /review-bugbot on Sep 14, 19:34. It flagged 0 findings. Bugbot on commit |
Exercise the empty and unauthenticated context-list paths, `context show` for a missing and a fully-populated context, the discovery server-error and bare-host-normalisation branches, and the no-Cloud guard's no-active-context no-op. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Current version of PR was reviewed by /review-bugbot on Sep 14, 20:06. It flagged 0 findings. Bugbot on commit |
…gured [DEP-238] Cloud is configured with the coordinator's bare host, while an installation's discovery document publishes the coordinator's API base, which already ends in the API path. Every call site appended the path unconditionally, so signing in to an installation asked for: https://cloud.bosslevel.dev/api/v1/api/v1/auth/cli_config and got a 404 that read like the coordinator was missing rather than like the address had been built twice. Both forms now normalise to the same endpoint before the path is appended, at the six places that build one. The trailing segment is only stripped when it ends the address, so a coordinator genuinely served under a longer path keeps it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…es [DEP-238] Engines from the discovery release onwards nest the service addresses under a urls object, and older ones publish them at the top level. A CLI that understood only one shape refused to sign in to half the installations it was pointed at, reporting that the document included no coordinator, with nothing an operator could do about it. Both shapes are now accepted. A top-level address wins over a nested one so an engine serving both is read the way it intends, and a urls field that is not an object is ignored rather than fatal, since a document we cannot fully parse is still worth the parts we can. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An installation running more than one deployment provider needs to be told which one a server belongs on, and there was no way to say so. The field is left out of the request entirely when no provider is named, rather than sent as a null. An installation that knows only one provider rejects the field, so a null would have broken every deploy that does not need it, including every deploy against Cloud. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… to [DEP-238] Named contexts moved the signed-in coordinator and tokens under the active context, and the credentials file no longer carries them at the top level at all. Usage identity still read the top level, so after this branch it found no access token, skipped the whoami call entirely, and fell back to the Cloud address for a request it was never going to make. Nothing surfaced, because the whole lookup is wrapped in a silent except: every event simply carried an anonymous id instead of the principal, and a self-hosted user was attributed to nobody. The fallback order matters for files written by older CLIs that have not been migrated yet. The named context wins, a lone context is used when none is marked active, and anything unresolvable falls back to the top level rather than failing, since losing attribution is better than breaking a CLI command over telemetry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Pushed four commits that were sitting uncommitted on my machine, each one a fix found by actually driving this CLI against a self-hosted installation. What landedReaching one coordinator endpoint whichever address was configured. Cloud is configured with the coordinator's bare host, while an installation's discovery document publishes its API base, which already ends in the API path. Every call site appended the path unconditionally, so signing in to an installation asked for Reading the discovery addresses in either shape. Engines from the discovery release onwards nest the addresses under a Naming the provider a server deploys to. An installation running more than one provider needs to be told which one a server belongs on. The field is omitted from the request entirely when unset rather than sent as null, because an installation that knows one provider rejects it and a null would break every deploy that does not need it. Reading usage credentials from the active context. Found by Bugbot and confirmed against the writer. Named contexts moved the signed-in coordinator and tokens under the active context, and the file no longer carries them at the top level at all, but usage identity still read the top level. It found no access token, skipped the whoami call, and every event carried an anonymous id instead of the principal. Nothing surfaced because the whole lookup sits inside a silent ProofEach fix ships the test that fails without it: DismissedLogin does not survive a corrupt credentials file. Real, and not this branch. The echo example is renamed to 🐕 Written by Kyoto, an AI agent, on Pascal's behalf — |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Logging in by url skips the already-signed-in guard, so a named context can be added at any time. Plain login still consulted the active context, so a user signed in to a self-hosted installation was told they were already logged in and could not add Cloud at all. The advice it offered was to log out, which deletes the credentials file and with it every context they had. The guard now asks about the context the login would actually write rather than whichever one happens to be active, so it still refuses a genuinely redundant sign-in and no longer refuses a new one. The context flag is passed through to the save for the same reason. It was accepted and ignored on this path, so naming a context without a url silently wrote to the default one, and a guard reading the named context while the save wrote elsewhere would have disagreed with itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…DEP-238] Making the guard read a named context instead of the active one changes what happens when only a self-hosted context exists. It must change nothing else, and two cases carry the whole compatibility claim. A credentials file written before named contexts existed migrates into the default context, so a user signed in to Cloud alone is still recognised and the redundant sign-in is still refused. A user signed in to both, with the self-hosted one active, is also still refused. That is the case the change could plausibly have broken, since it is the one where the active context and the context being written differ while both hold a token. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Checking a named context rather than the active one left the message behind. A user signed in to a self-hosted installation who ran plain login was told their Cloud organisation and project were "Active", while every other command kept using the self-hosted context. The one action offered was to log out, which deletes every saved context. The message now says which context it found and names the one that is actually active, and only uses the word active when those are the same context. The advice leads with switching rather than deleting, since switching is what someone in this position almost always wants and the destructive option was previously the only one on offer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Moving the guard onto a named context left the follow-up advice in the caller, which cannot tell whether that context is the active one. It therefore told every user to switch to a context they were often already using, directly under a line saying that context was active. The advice now sits beside the message it belongs to, where activeness is already known. An active session gets the logout wording it has always had, and a session in another context gets the switch. Neither is offered where it makes no sense. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s [DEP-305] Deploy resolves auth headers per request and reads the CI credentials straight from the environment each time. Partway through the command it loads the project's env file into the process, and a tool server project very often keeps an api key there. Calls before that point authenticated as the signed-in user over oauth, and calls after it authenticated with the project's key. The organisation in the url is resolved the same way, so the target changed with the credential and the deploy could land somewhere the user never chose. The environment is now captured once when the command starts, and every later resolution reads that capture. Credentials present at start still drive unattended runs exactly as before, which is the case the environment exists to serve. A file read later configures the server being deployed, which was never a statement about who is deploying it. Capturing per command rather than per process keeps a test runner that invokes many commands in one process honest, and the cli tests reset it between cases because a real invocation gets a fresh process and they do not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Fixed the deploy authentication switch, which was the one finding I had held back as a release blocker. The defect
The fixThe environment is captured once when the command starts, and every later resolution reads that capture. Credentials present at start still drive unattended runs exactly as before, which is the case the environment variables exist to serve. A file read later configures the server being deployed, which was never a statement about who is deploying it. The capture is per command rather than per process. That matters for the test runner, which invokes many commands in one process, and the CLI tests reset it between cases because a real invocation gets a fresh process and they do not. Four cases cover it: a project file cannot introduce credentials, cannot replace credentials that were already there, credentials present at start still drive unattended runs, and without pinning the environment is still read live. The one remaining failure, Dismissed: the coordinator falls back to Cloud in CI modeThe observation is right and the consequence is not. With CI credentials set, For a CI context that genuinely points at Cloud, the fallback is the correct answer. The two callers are the There is a real nit underneath it: that refusal advises 🐕 Written by Kyoto, an AI agent, on Pascal's behalf — |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 79a7085. Configure here.
…-305] Capturing at the start of the command was too late to do anything. The cli imports its utils module, which imports the mcp server settings module, which builds a settings object at import time and loads the project env file into the process as it does. All of that finishes before any command body runs, so the capture recorded values the env file had already supplied and the guard it was meant to provide did nothing at all. The capture now happens when the arcade_cli package itself is imported, from a module with no dependencies beyond the standard library, so nothing it pulls in can have loaded a file first. The proof runs in its own process, because the ordering only exists across a real import. It writes an env file naming an installation and a key, imports the cli from that directory, and asks whether unattended mode is armed. Without the capture the answer is yes, which is the defect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The config directory is resolved once at import from the environment, so a test that reaches for that constant writes to the real credentials file in the developer's home rather than to a temporary directory. Fixtures that redirect the directory cannot help, because the constant is already bound by the time any of them run. I did exactly this while writing the login guard tests and overwrote a working session with fixture data. Nothing in the suite does it now, but nothing stopped it either. Pointing the work directory at a fresh temporary one for the run binds that constant somewhere disposable, so the whole class of mistake costs nothing instead of costing a login. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sdreyer
left a comment
sdreyer
left a comment
There was a problem hiding this comment.
Reviewed for backwards compatibility, on-disk format, and CLI/engine wire compat. Ran the CLI suite at 0c628a4b in a clean worktree: 903 passed, 1 skipped. The DEP-305 import-ordering fix is correct and well-placed — capturing in arcade_cli/__init__.py from a stdlib-only module is the only spot early enough to beat the settings import that loads the project .env.
The wire story is clean in both directions. I diffed the full URL surface against main: every path an existing flow hits is byte-identical, coordinator_api() is pure normalization, and provider is dropped from the create-deployment body when unset (CreateDeploymentRequest has no other nullable fields, nested included), so an un-updated engine receives the exact old payload. account_id was already required on main, so there's no new coordinator obligation. New CLI against today's prod engine is safe.
One genuine break and one fail-open guard, plus rollout sequencing. Details inline; items that can't be anchored to the diff are below.
Version floors weren't raised on the sibling packages. libs/arcade-mcp-server/pyproject.toml:24 and libs/arcade-tdk/pyproject.toml:19 still declare arcade-core>=4.18.0, while core goes to 4.19.0 here. Combined with the credentials format change (see the inline comment on config_model.py), a pinned or Docker-built server env resolving 4.18 alongside a freshly-updated CLI silently loses its login — server.py:538 just logs "Arcade access token not configured" and auth tools start returning login instructions. CLAUDE.md's rule about bumping minimum dependency versions on breaking inter-library changes covers this case.
CI mode can still be armed by ambient shell env. DEP-305 closed the .env vector, but ARCADE_URL + ARCADE_API_KEY exported in a shell still silently switches an authenticated cloud user from OAuth to API-key auth and reshapes /secrets from org-scoped to /v1/admin — with an existing login sitting right there, unused, and nothing printed. ARCADE_API_KEY is already set in plenty of shells; ARCADE_URL being new is the only thing making the pair rare today. One line on stderr when CI mode wins over a saved context would make this diagnosable instead of mysterious.
Rollout ordering for degraded. I traced it to monorepo#4135; it's gated on ProviderName() == ProviderKubernetes with the reconciler wiring commented "only for the kubernetes provider", and providerNameOrDefault returns ProviderPorter. So Cloud is outside the blast radius and the exposure is self-hosted k8s installs — exactly this PR's audience. Since #4135 is still open, shipping CLI 1.16.0 first is free and gives operators a degraded-aware client before the engine can produce the state.
Two nits. org.py / project.py annotate host: str while defaulting to None — harmless at runtime since typer.Option returns Any, but the annotation now says the opposite of what the code does. And the autouse forget() in libs/tests/cli/conftest.py means every in-process test exercises _startup_environment's live-env fallback rather than the captured path; the pinning behavior rests entirely on the one subprocess test. Deliberate, I assume — just worth knowing that's the single thread holding it.
None of this blocks the feature work, which reads well — the named-context migration is careful and the discovery/predates path is a nice touch.
| elif self.active_context and self.active_context in self.contexts: | ||
| self.contexts[self.active_context] = self._to_named_context() | ||
|
|
||
| cloud = { |
There was a problem hiding this comment.
This makes the credentials file one-way. save_to_file now writes only active_context + contexts, dropping the flat auth / user / context keys. BaseConfig is extra="ignore", so arcade-core 4.18 parses the new file into an all-None Config. I ran the pre-PR model against a new-format file:
old-core reads new file -> authenticated: False | auth: None | user: None | ctx: None
The old->new direction is fine — I ran a legacy file through this loader and it migrates to default, stays authenticated, keeps kind: cloud and the org/project. It's the reverse that bites: a CLI downgrade, or any environment still on 4.18, sees a logged-out user.
And it isn't opt-in. auth_tokens.py:106 saves on token refresh, so the first authenticated command flips the format permanently.
Suggested fix — dual-write. Keep emitting the flat active-context keys alongside contexts/active_context. load_from_file already branches on "contexts" in cloud and overwrites the flat values via _apply_named_context, so old and new readers both work off one file and the downgrade cliff disappears. Roughly five lines here, and it's worth more than the floor bumps because a PyPI release can't be unpublished and a migrated file can't be un-migrated.
Separate nit, a few lines up: the if not self.contexts / elif active_context in self.contexts pair has no else. If contexts is populated but active_context is None or names a missing key, neither branch runs and the in-memory flat state — including a token just refreshed — is silently dropped from the write. load_from_file always leaves those consistent today, so I couldn't reach it, but it's a data-loss branch with no diagnostic.
| return resolve_active_context() | ||
| except FileNotFoundError: | ||
| return None | ||
| except Exception: |
There was a problem hiding this comment.
The no-Cloud guard fails open. guard_no_cloud gets its context from try_resolve_active_context, which swallows every exception into None, and then returns early on ctx is None. So any failure to read the credentials file — malformed YAML, a ValidationError on one context, a permissions error — silently disables the guard rather than tripping it.
For a feature whose own error message promises "Artifacts and requests stay inside your environment," open is the wrong direction to fail. The blanket except Exception also hides the reason, so a self-hosted user gets no signal that the protection stopped applying.
Worth distinguishing the two cases: FileNotFoundError means genuinely no config and unguarded is correct; anything else means a config exists but couldn't be read, which should refuse rather than proceed. Cheap version — have guard_no_cloud catch the load failure itself and raise NoCloudGuardError with the underlying reason, leaving try_resolve_active_context alone for the callers that legitimately want a soft default.
|
|
||
| ci_context = try_resolve_active_context() | ||
| if ci_context is not None and ci_context.is_ci and ci_context.api_key: | ||
| return {"Authorization": f"Bearer {ci_context.api_key}"} |
There was a problem hiding this comment.
Not a defect — a request, while this code is open.
get_auth_headers returns Authorization only, with no User-Agent or X-Arcade-CLI-Version. That means the engine has no way to tell which CLI is talking to it, so when degraded lands (monorepo#4135) it can't be gated per-client — it's all-or-nothing, and the only lever left is waiting on adoption.
Adoption will be slow: arcade update notifies rather than auto-installs, so the tail is pinned CI, Docker images, and uv tool install nobody re-runs. cli_version already goes to PostHog (usage/constants.py:7), so you can watch the distribution — but watching isn't gating.
A few lines adding the version header here turns step-5 of the rollout from "wait and hope" into something controllable.
| for i, log_line in enumerate(all_logs, 1): | ||
| console.print(f"{i:4d} | {log_line}", style="dim") | ||
|
|
||
| if final_status != "running": |
There was a problem hiding this comment.
Worth calling out in the release notes: this changes the exit code. Previously a failed deployment printed "Deployment failed" and exited 0 — the if running / elif failed pair had no raise. Pipelines that were silently green on broken deploys will now correctly go red. Right fix, but it'll look like a regression to whoever owns those pipelines.
The mirror-image hazard is on older clients. The pre-1.16 poll loop is while status in ["pending","unknown","updating"] with break on ["running","failed"]. Feed it degraded and the loop falls out, neither branch matches, and it exits 0 with no message — a silent pass on a degraded rollout. That's an argument for landing this CLI before monorepo#4135, not after.
| test: install ## Test the code with pytest | ||
| @echo "🚀 Testing libs: Running pytest" | ||
| @uv run pytest -W ignore -v libs/tests --cov=libs --cov-config=pyproject.toml --cov-report=xml | ||
| @ARCADE_WORK_DIR=$$(mktemp -d) uv run pytest -W ignore -v libs/tests --cov=libs --cov-config=pyproject.toml --cov-report=xml |
There was a problem hiding this comment.
Two things about this line — the intent is right, and I've done the same thing to my own credentials file.
1. The guard only covers make test. CLAUDE.md documents single tests as uv run pytest libs/tests/core/test_toolkit.py::TestClass::test_method, which is unprotected and still writes to the real ~/.arcade. That's the exact accident the commit message describes. The commit is right that a fixture can't help — the constant is bound at import — but os.environ.setdefault("ARCADE_WORK_DIR", tempfile.mkdtemp()) at the top of libs/tests/conftest.py runs before any test module imports arcade_core, and covers every invocation path rather than one target.
2. Setting this var puts the suite in a split-brain config. ARCADE_WORK_DIR resolves to two different files:
constants.py -> $ARCADE_WORK_DIR/.arcade/credentials.yaml
config_model -> $ARCADE_WORK_DIR/credentials.yaml
SAME: False
constants.py:4 appends .arcade; Config.get_config_dir_path() uses the var as the directory itself. Pre-existing — both are on main, and check_existing_login behaves the same either way — but this line makes the divergent configuration the default for every test run. The sharpest symptom for anyone who sets the var: login writes via Config.save_to_file() while logout deletes CREDENTIALS_FILE_PATH, so arcade logout prints "You're not logged in" and leaves the credentials on disk.
History, for whoever fixes it: get_config_dir_path() is the original (#31, then #50 swapped the settings object for the env var, same semantics — the var is the config dir). constants.py introduced the second reading later in #635. Seniority argues one way but call sites argue the other: the .arcade-suffix reading has five consumers (usage/identity.py, authn.py, main.py, command_tracker.py, update.py) against essentially just config_model itself. Unifying get_config_dir_path() onto ARCADE_CONFIG_PATH changes one function, leaves the five callers alone, and keeps this Makefile line working as written.
Happy to send that as a separate patch if you'd rather keep this PR focused.
2 participants
What & why
DEP-238. Teaches the
arcadeCLI to target a self-hosted Arcade installation instead of only Arcade Cloud.arcade login --url <install-url>(orARCADE_URL) fetches the installation's discovery document atGET /.well-known/arcade, signs in against the discovered coordinator, picks a project, and saves a named context. Deployments-off installations report the discovery reason and exit non-zero; a URL with no discovery document is refused before anything is sent (the "installation predates discovery" path).cloud/self_hostedkind + auth/org/project). The legacy single-context file migrates to adefaultcontext on load.arcade context list|use|showmanages and switches them.arcade deploy,secret,org, andprojectresolve their target from the active context; no flags needed. Unauthenticated deploy names the login command.ARCADE_URL+ARCADE_API_KEYcomplete a deploy with no terminal and no credentials file (bearer API key auth).api.arcade.dev,cloud.arcade.dev,*.arcade.dev) is refused. The hardcoded cloud defaults indeploy/secret/org/projectwere routed through the active context.Cloud users see no prompt and no new required flag.
Tests
libs/tests/cli/test_context.py,test_login_url.py,test_contexts_cmd.py,test_deploy_context.py:legacy→default migration + named-context round trip, discovery parsing + predates path, cloud-host detection, the no-Cloud guard, login by URL (incl. disabled-deployments and missing-discovery), the context command, unauthenticated deploy naming login, and a CI-env deploy driven without prompts. Full
libs/tests/clisuite green;ruff+mypyclean on changed libs.Proof
Backend/CLI-only change with no dashboard surface. Verified on the CLI via the unit suite above (behavior asserted with mocked discovery + OAuth). The e2e kind-lane run with the recording proxy and the Cloud-context regression against local-stack live in the monorepo BDD harness (out of this repo's scope); the spec promotion / tag moves are tracked there.
🤖 Generated with Claude Code
Note
Medium Risk
Changes authentication, credential file shape, and which hosts the CLI contacts—security-relevant for self-hosted vs cloud—but behavior is guarded by migration, discovery checks, and extensive tests.
Overview
Adds multi-installation support to the Arcade CLI: credentials are stored as named contexts (cloud vs self-hosted, engine/coordinator/dashboard URLs, auth, org/project), with legacy single-context files migrated to
defaulton load. Newarcade context list|use|showcommands switch and inspect contexts.Login gains
--url/ARCADE_URLplus optional--context: the CLI loads/.well-known/arcade, refuses missing or deployments-disabled installs before OAuth, then saves a named context from discovery.arcade deploy, secret, org, and project default to the active context’s URLs instead of hardcoded cloud hosts;coordinator_apinormalizes coordinator API paths.CI mode uses
ARCADE_URL+ARCADE_API_KEY(pinned at CLI import so project.envcannot arm CI later) for non-interactive deploy with API-key auth and/v1(admin) URL shapes. A no-Cloud guard blocks contacting*.arcade.devwhile a self-hosted context is active.Deploy also accepts optional
--provider, treats degraded rollout status, and fails non-running deployments; rootmake testisolates credentials viaARCADE_WORK_DIR. Bumps arcade-mcp to 1.16.0 and arcade-core to 4.19.0 with broad unit coverage.Reviewed by Cursor Bugbot for commit 0c628a4. Bugbot is set up for automated code reviews on this repo. Configure here.