From 1984d3a70288593408f43aa6cfbe0f7f9ac2d8e0 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:16:30 +0000 Subject: [PATCH 1/5] fix(config): open the oauth-url link in the browser and print it unwrapped (SUPPORT-17391) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Zora Jelínková --- .../kbagent/references/commands-reference.md | 2 +- .../skills/kbagent/references/gotchas.md | 24 +++ src/keboola_agent_cli/auth/environment.py | 32 ++++ .../commands/_config_oauth.py | 52 +++++- tests/test_auth_environment.py | 51 ++++++ tests/test_config_row_cli.py | 150 ++++++++++++++++++ 6 files changed, 303 insertions(+), 8 deletions(-) diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index 83281d11d..2b7f1c3e8 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -150,7 +150,7 @@ Requires a **super-admin** Manage API token (same kind as `org setup`). Same def - `config row-create --project NAME --component-id ID --config-id ID --name ROW_NAME [--description D] [--configuration JSON|@file|-] [--is-disabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- create a new configuration row. Returns the full row dict with `id`, `name`, `version`. Optional `--configuration` accepts JSON inline, `@file`, or stdin (`-`). `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). - `config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--change-description TEXT] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- update an existing configuration row. Pass only the fields you want to change; omitted fields are preserved. `--is-disabled` / `--is-enabled` toggle the row's enabled state. `--change-description` sets the new row version's `changeDescription` audit line (default: auto-generated). `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). - `config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes]` -- delete a configuration row. Destructive (gated behind `--allow-destructive`). Branch-aware. Without `--yes` and outside `--json` mode, prompts for interactive confirmation; `--json` mode auto-skips the prompt. -- `config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL]` -- return the OAuth authorization URL for a component that uses OAuth authentication. **Requires a master Storage API token** (canManageTokens privilege) -- non-master tokens fail with `MISSING_MASTER_TOKEN` exit 3 on a fail-fast pre-flight check before any HTTP write happens. Open the URL in a browser to complete the OAuth flow. +- `config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open]` -- return the OAuth authorization URL for a component that uses OAuth authentication. **Requires a master Storage API token** (canManageTokens privilege) -- non-master tokens fail with `MISSING_MASTER_TOKEN` exit 3 on a fail-fast pre-flight check before any HTTP write happens. (since vNEXT) the URL is opened in the default browser automatically in interactive human mode (never under `--json` or when stdout is not a terminal); `--no-open` prints it only. The URL is also printed unwrapped for copying -- **never re-render it as prose in a chat answer**, wrapping renderers link only its first row, which drops the config id and makes the wizard answer `Failed to load config data` (see gotchas). - `config state-get --project NAME --component-id ID --config-id ID [--row-id ID] [--branch ID]` -- (since 0.84.2, #593) read a configuration's runtime `state`. Without `--row-id` returns the root config's state; with `--row-id` returns that row's state (a missing row id fails loudly, it does not silently return `{}`). For row-based components the root state node is unused -- read the row state instead. See [config-state-workflow](references/config-state-workflow.md). - `config state-set --project NAME --component-id ID --config-id ID [--row-id ID] --state JSON|@file|- [--branch ID] [--dry-run] [--yes]` -- (since 0.84.2, #593) write a configuration's runtime `state` via the dedicated branch-scoped `PUT .../state` endpoint. `--state` must be a JSON object (not an array/scalar) under 4 MB. `--row-id` targets a row's state instead of the root. `--dry-run` previews the current-vs-new diff (same shape as `config update --dry-run`) without writing; a no-op (new state equals current) short-circuits with `changed: false` and no write. Guarded write: prompts for confirmation unless `--yes` or `--json`. This is the fix for `config update --set 'state...'`, which never reached runtime state (see [gotchas](references/gotchas.md)). See [config-state-workflow](references/config-state-workflow.md) for the seed-before-migrate playbook. diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index fd33996b9..9869df3dd 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -4981,3 +4981,27 @@ single-project command: project is not the first row of `project list`. - The fix also covers `kbagent serve`: the `/kai/*` routes, `POST /documentation/query`, and `GET /components?query=...` resolved the project the same wrong way. + +## An OAuth authorization URL survives only as long as one visual row (SUPPORT-17391) + +The `config oauth-url` link is ~200 chars (`token` + encoded `sapiUrl` + +`#//`), so it fits no terminal row, and two layers then +truncate it. Both end in the same misleading browser message, +`Failed to load config data. Please contact us on support@keboola.com` -- +`token`/`sapiUrl` come first and survive, the config id in the fragment does +not, so the wizard authenticates and then 404s on the configuration: + +- **Rich layout.** `console.print(url)` inserts REAL newlines at the console + width; a copy of one visual row, or a line-based parse of stdout, loses the + tail. `soft_wrap=True` is mandatory for anything printing a URL. +- **Per-row link detection.** Terminals and chat renderers that autodetect URLs + scope detection to one visual row, so a wrapped URL becomes a link to its + first row only. Nothing kbagent prints can prevent that in a renderer it does + not control (e.g. an assistant transcript). + +Since vNEXT the command therefore opens the URL itself (interactive human mode +only; `--no-open` opts out) and prints it unwrapped as a copy fallback. Under +WSL `open_browser` routes through `wslview`, because `webbrowser` keys its +Windows handling off `sys.platform == "win32"` -- `linux` inside WSL -- and +would otherwise find no handler or open a Linux-side browser that shares none +of the logins the flow needs. diff --git a/src/keboola_agent_cli/auth/environment.py b/src/keboola_agent_cli/auth/environment.py index ed2ca9d9d..23d48f193 100644 --- a/src/keboola_agent_cli/auth/environment.py +++ b/src/keboola_agent_cli/auth/environment.py @@ -28,6 +28,10 @@ # wslview from stalling the login flow. _WSLVIEW_PROBE_TIMEOUT_SECONDS = 2.0 +# Timeout for handing a URL to `wslview`: it returns as soon as the Windows side +# has accepted the request, so anything longer means the shim is wedged. +_WSLVIEW_OPEN_TIMEOUT_SECONDS = 10.0 + @dataclass(frozen=True) class BrowserEnvironment: @@ -168,6 +172,26 @@ def detect_browser_environment() -> BrowserEnvironment: return BrowserEnvironment(loopback_browser_usable=True, reason="", opener=opener) +def _open_via_wslview(url: str) -> bool: + """Hand ``url`` to the Windows-side default browser through `wslview`. + + Runs synchronously so the exit status is the answer: unlike the + `webbrowser` path there is no thread to swallow, and the whole point is to + know whether Windows accepted the URL. Output is captured, never logged -- + the URL must not reach stderr. + """ + try: + completed = subprocess.run( + ["wslview", url], + capture_output=True, + timeout=_WSLVIEW_OPEN_TIMEOUT_SECONDS, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return False + return completed.returncode == 0 + + def open_browser(url: str) -> bool: """Best-effort `webbrowser.open` on a daemon thread. @@ -177,7 +201,15 @@ def open_browser(url: str) -> bool: before spawning the thread) and True once the open has been dispatched; a True return does not guarantee a browser window actually appeared, only that a handler accepted the request. + + Under WSL the URL goes to `wslview` first: `webbrowser` keys its Windows + handling off ``sys.platform == "win32"``, which is ``linux`` inside WSL, so + it would either find no handler at all or open a browser in the Linux + session -- a separate profile that shares none of the logins the URL needs. """ + if _env_flag_set("WSL_INTEROP") and _wslview_is_working(): + return _open_via_wslview(url) + try: webbrowser.get() except webbrowser.Error: diff --git a/src/keboola_agent_cli/commands/_config_oauth.py b/src/keboola_agent_cli/commands/_config_oauth.py index 8f6016cdd..ca49481a9 100644 --- a/src/keboola_agent_cli/commands/_config_oauth.py +++ b/src/keboola_agent_cli/commands/_config_oauth.py @@ -16,10 +16,16 @@ import typer from rich.markup import escape +from ..auth.environment import open_browser from ..errors import ConfigError, ErrorCode, KeboolaApiError from ._helpers import get_formatter, get_service, map_error_to_exit_code +def _should_open(*, no_open: bool, is_terminal: bool) -> bool: + """Whether to hand the URL to a browser: interactive use, unless opted out.""" + return is_terminal and not no_open + + def register(app: typer.Typer) -> None: """Mount the oauth-url command onto ``app`` (the ``config`` Typer group).""" @@ -27,7 +33,8 @@ def register(app: typer.Typer) -> None: "oauth-url", rich_help_panel="OAuth", help=( - "Requires master token. Generate an OAuth authorization URL for a component configuration." + "Requires master token. Generate an OAuth authorization URL for a component " + "configuration and open it in the default browser." ), ) def config_oauth_url( @@ -52,16 +59,33 @@ def config_oauth_url( "--redirect-url", help="Optional URL to return to after the OAuth flow completes (sets returnUrl query param)", ), + no_open: bool = typer.Option( + False, + "--no-open", + help="Only print the authorization URL, do not open a browser", + ), ) -> None: """Generate an OAuth authorization URL for a component configuration. - Opens a short-lived, component-scoped authorization link. - The user must open this URL in a browser and grant access. + Mints a short-lived, component-scoped authorization link and opens it in + the default browser, so the URL never has to be clicked or copied: it is + ~200 characters, and a terminal or chat renderer that wraps it turns the + visible link into its first row only, which drops the configuration id + and makes the authorization page answer "Failed to load config data". + The complete URL is still printed for copying elsewhere. + + The browser is left alone in `--json` mode and when stdout is not a + terminal, so scripted and piped callers never get a stray window; + `--no-open` suppresses it in interactive use too. \b Examples: kbagent config oauth-url --project P --component-id keboola.ex-google-drive --config-id ID + # Print the URL without opening a browser (e.g. authorizing on another machine) + kbagent config oauth-url --project P --component-id keboola.ex-google-drive --config-id ID \\ + --no-open + # Redirect back to a custom URL after the OAuth flow completes kbagent config oauth-url --project P --component-id keboola.ex-google-drive --config-id ID \\ --redirect-url https://example.com/oauth-done @@ -87,12 +111,26 @@ def config_oauth_url( ) raise typer.Exit(code=map_error_to_exit_code(exc)) from None + url = result["url"] + if formatter.json_mode: formatter.output(result) - else: + return + + formatter.console.print( + f"[bold]OAuth URL for[/bold] [cyan]{escape(component_id)}[/cyan]/" + f"[cyan]{escape(config_id)}[/cyan]:\n" + ) + # soft_wrap keeps Rich from laying the URL out at the console width: + # by default it inserts real newlines, so copying one visual row yields + # a truncated URL. markup=False stops a literal "[" in the token from + # being read as a style tag. + formatter.console.print(url, soft_wrap=True, highlight=False, markup=False) + + should_open = _should_open(no_open=no_open, is_terminal=formatter.console.is_terminal) + if should_open and open_browser(url): formatter.console.print( - f"[bold]OAuth URL for[/bold] [cyan]{escape(component_id)}[/cyan]/" - f"[cyan]{escape(config_id)}[/cyan]:\n" + "\n[dim]Opened in your default browser. Grant access there.[/dim]" ) - formatter.console.print(f" [link]{result['url']}[/link]") + else: formatter.console.print("\n[dim]Open this URL in a browser and grant access.[/dim]") diff --git a/tests/test_auth_environment.py b/tests/test_auth_environment.py index 40891c1a9..89559d730 100644 --- a/tests/test_auth_environment.py +++ b/tests/test_auth_environment.py @@ -366,3 +366,54 @@ def _boom(_url: str) -> bool: for thread in list(environment.threading.enumerate()): if thread is not environment.threading.current_thread(): thread.join(timeout=1.0) + + +class TestOpenBrowserUnderWsl: + """WSL is `sys.platform == "linux"`, so `webbrowser` never reaches Windows itself.""" + + def test_hands_the_url_to_wslview(self, monkeypatch: pytest.MonkeyPatch) -> None: + calls: list[list[str]] = [] + + def _run(cmd: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[bytes]: + calls.append(cmd) + return subprocess.CompletedProcess(cmd, 0, b"", b"") + + monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") + monkeypatch.setattr(environment.shutil, "which", lambda _name: "/usr/bin/wslview") + monkeypatch.setattr(environment.subprocess, "run", _run) + + url = "https://external.keboola.com/oauth/index.html?token=t&sapiUrl=u#/comp/cfg" + + assert open_browser(url) is True + assert calls == [["wslview", "--version"], ["wslview", url]] + + def test_reports_failure_when_wslview_rejects_the_url( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + def _run(cmd: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[bytes]: + code = 0 if cmd[1:] == ["--version"] else 1 + return subprocess.CompletedProcess(cmd, code, b"", b"") + + monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") + monkeypatch.setattr(environment.shutil, "which", lambda _name: "/usr/bin/wslview") + monkeypatch.setattr(environment.subprocess, "run", _run) + + assert open_browser("https://external.keboola.com/oauth/index.html?token=t") is False + + def test_falls_back_to_webbrowser_without_a_working_wslview( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + opened: list[str] = [] + + monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") + monkeypatch.setattr(environment.shutil, "which", lambda _name: None) + monkeypatch.setattr(environment.webbrowser, "get", lambda: object()) + monkeypatch.setattr(environment.webbrowser, "open", opened.append) + + assert open_browser("https://external.keboola.com/oauth/index.html?token=t") is True + + for thread in list(environment.threading.enumerate()): + if thread is not environment.threading.current_thread(): + thread.join(timeout=1.0) + + assert opened == ["https://external.keboola.com/oauth/index.html?token=t"] diff --git a/tests/test_config_row_cli.py b/tests/test_config_row_cli.py index 8af879fad..a38e20d70 100644 --- a/tests/test_config_row_cli.py +++ b/tests/test_config_row_cli.py @@ -9,6 +9,7 @@ from helpers import setup_single_project from keboola_agent_cli.cli import app +from keboola_agent_cli.commands._config_oauth import _should_open from keboola_agent_cli.errors import KeboolaApiError from keboola_agent_cli.services.config_service import ConfigService @@ -644,6 +645,155 @@ def test_redirect_url_propagates(self, tmp_config_dir: Path) -> None: output = json.loads(result.output) assert output["data"]["redirect_url"] == "https://example.com/done" + @pytest.mark.parametrize("columns", ["40", "80", "120"]) + def test_human_output_keeps_url_on_one_line(self, tmp_config_dir: Path, columns: str) -> None: + """The printed URL carries no inserted newline at any console width.""" + service = self._make_oauth_service(tmp_config_dir) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "keboola_agent_cli.commands._config_oauth.get_service", + lambda ctx, name: service, + ) + result = runner.invoke( + app, + [ + "--config-dir", + str(tmp_config_dir), + "config", + "oauth-url", + "--project", + "prod", + "--component-id", + "keboola.ex-google-drive", + "--config-id", + "cfg-001", + ], + env={"COLUMNS": columns, "TERM": "dumb"}, + ) + + assert result.exit_code == 0, result.output + assert OAUTH_RESULT["url"] in result.output + + def test_no_open_flag_leaves_the_browser_alone(self, tmp_config_dir: Path) -> None: + """--no-open never dispatches an open, whatever the console reports.""" + service = self._make_oauth_service(tmp_config_dir) + + def _fail(_url: str) -> bool: + raise AssertionError("browser must not be opened with --no-open") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "keboola_agent_cli.commands._config_oauth.get_service", + lambda ctx, name: service, + ) + mp.setattr("keboola_agent_cli.commands._config_oauth.open_browser", _fail) + result = runner.invoke( + app, + [ + "--config-dir", + str(tmp_config_dir), + "config", + "oauth-url", + "--project", + "prod", + "--component-id", + "keboola.ex-google-drive", + "--config-id", + "cfg-001", + "--no-open", + ], + ) + + assert result.exit_code == 0, result.output + assert OAUTH_RESULT["url"] in result.output + + def test_json_mode_leaves_the_browser_alone(self, tmp_config_dir: Path) -> None: + """--json is a machine surface: no browser, unchanged payload.""" + service = self._make_oauth_service(tmp_config_dir) + + def _fail(_url: str) -> bool: + raise AssertionError("browser must not be opened in --json mode") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "keboola_agent_cli.commands._config_oauth.get_service", + lambda ctx, name: service, + ) + mp.setattr("keboola_agent_cli.commands._config_oauth.open_browser", _fail) + result = _invoke( + tmp_config_dir, + "oauth-url", + [ + "--project", + "prod", + "--component-id", + "keboola.ex-google-drive", + "--config-id", + "cfg-001", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.output)["data"]["url"] == OAUTH_RESULT["url"] + + def test_interactive_terminal_opens_the_complete_url(self, tmp_config_dir: Path) -> None: + """On a terminal the whole URL -- not a wrapped fragment -- reaches the browser.""" + service = self._make_oauth_service(tmp_config_dir) + opened: list[str] = [] + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "keboola_agent_cli.commands._config_oauth.get_service", + lambda ctx, name: service, + ) + + def _record(url: str) -> bool: + opened.append(url) + return True + + def _always_open(**_kwargs: bool) -> bool: + return True + + mp.setattr("keboola_agent_cli.commands._config_oauth.open_browser", _record) + mp.setattr("keboola_agent_cli.commands._config_oauth._should_open", _always_open) + result = runner.invoke( + app, + [ + "--config-dir", + str(tmp_config_dir), + "config", + "oauth-url", + "--project", + "prod", + "--component-id", + "keboola.ex-google-drive", + "--config-id", + "cfg-001", + ], + env={"COLUMNS": "40"}, + ) + + assert result.exit_code == 0, result.output + assert opened == [OAUTH_RESULT["url"]] + assert "Opened in your default browser" in result.output + + +class TestShouldOpenBrowser: + """The interactive-only gate on the automatic browser open.""" + + @pytest.mark.parametrize( + ("no_open", "is_terminal", "expected"), + [ + (False, True, True), + (True, True, False), + (False, False, False), + (True, False, False), + ], + ) + def test_gate(self, no_open: bool, is_terminal: bool, expected: bool) -> None: + assert _should_open(no_open=no_open, is_terminal=is_terminal) is expected + # --------------------------------------------------------------------------- # is_disabled / is_enabled CLI tests From a271d42b42f9eec2a48c40e8851a5e57b2f67640 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:51:25 +0000 Subject: [PATCH 2/5] docs(skill): tell agents to echo the oauth-url link in a fenced code block (SUPPORT-17391) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Zora Jelínková --- plugins/kbagent/skills/kbagent/SKILL.md | 1 + .../kbagent/references/commands-reference.md | 2 +- .../kbagent/references/oauth-workflow.md | 29 +++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 plugins/kbagent/skills/kbagent/references/oauth-workflow.md diff --git a/plugins/kbagent/skills/kbagent/SKILL.md b/plugins/kbagent/skills/kbagent/SKILL.md index 5b70f8ad0..21a5fd598 100644 --- a/plugins/kbagent/skills/kbagent/SKILL.md +++ b/plugins/kbagent/skills/kbagent/SKILL.md @@ -412,6 +412,7 @@ For detailed response parsing rules and common pitfalls, see [gotchas](reference | **Billing / PAYG credits** (balance only; the shape of the invoice-history gap; PAYG_NOT_AVAILABLE; units) | [billing-workflow](references/billing-workflow.md) | | Dev branches | [branch-workflow](references/branch-workflow.md) | | Encrypting secrets before a config write | [encrypt-workflow](references/encrypt-workflow.md) | +| **Authorizing an OAuth component** (`config oauth-url`; auto-open, always echo the URL in a fenced block, verify `oauth_api.version`) | [oauth-workflow](references/oauth-workflow.md) | | Sync & Git-branching (GitOps) | [sync-workflow](references/sync-workflow.md) | | Sync row-level internals (manifest v3, hoist, encryption) | [sync-rows-workflow](references/sync-rows-workflow.md) | | **Promote configs source -> destination project** (from-scratch GitHub Actions pull -> validate -> push pipeline built on `sync`; PR-gated, cross-project dry-run diff) -- a **separate skill**, not a reference doc | [kbagent-promotion-pipeline](../kbagent-promotion-pipeline/SKILL.md) | diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index 2b7f1c3e8..c27bd549b 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -150,7 +150,7 @@ Requires a **super-admin** Manage API token (same kind as `org setup`). Same def - `config row-create --project NAME --component-id ID --config-id ID --name ROW_NAME [--description D] [--configuration JSON|@file|-] [--is-disabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- create a new configuration row. Returns the full row dict with `id`, `name`, `version`. Optional `--configuration` accepts JSON inline, `@file`, or stdin (`-`). `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). - `config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--change-description TEXT] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- update an existing configuration row. Pass only the fields you want to change; omitted fields are preserved. `--is-disabled` / `--is-enabled` toggle the row's enabled state. `--change-description` sets the new row version's `changeDescription` audit line (default: auto-generated). `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). - `config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes]` -- delete a configuration row. Destructive (gated behind `--allow-destructive`). Branch-aware. Without `--yes` and outside `--json` mode, prompts for interactive confirmation; `--json` mode auto-skips the prompt. -- `config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open]` -- return the OAuth authorization URL for a component that uses OAuth authentication. **Requires a master Storage API token** (canManageTokens privilege) -- non-master tokens fail with `MISSING_MASTER_TOKEN` exit 3 on a fail-fast pre-flight check before any HTTP write happens. (since vNEXT) the URL is opened in the default browser automatically in interactive human mode (never under `--json` or when stdout is not a terminal); `--no-open` prints it only. The URL is also printed unwrapped for copying -- **never re-render it as prose in a chat answer**, wrapping renderers link only its first row, which drops the config id and makes the wizard answer `Failed to load config data` (see gotchas). +- `config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open]` -- return the OAuth authorization URL for a component that uses OAuth authentication. **Requires a master Storage API token** (canManageTokens privilege) -- non-master tokens fail with `MISSING_MASTER_TOKEN` exit 3 on a fail-fast pre-flight check before any HTTP write happens. (since vNEXT) the URL is opened in the default browser automatically in interactive human mode (never under `--json` or when stdout is not a terminal); `--no-open` prints it only. The URL is also printed unwrapped for copying -- **never re-render it as prose in a chat answer**, wrapping renderers link only its first row, which drops the config id and makes the wizard answer `Failed to load config data`; echo it in a fenced code block instead. See [oauth-workflow](oauth-workflow.md). - `config state-get --project NAME --component-id ID --config-id ID [--row-id ID] [--branch ID]` -- (since 0.84.2, #593) read a configuration's runtime `state`. Without `--row-id` returns the root config's state; with `--row-id` returns that row's state (a missing row id fails loudly, it does not silently return `{}`). For row-based components the root state node is unused -- read the row state instead. See [config-state-workflow](references/config-state-workflow.md). - `config state-set --project NAME --component-id ID --config-id ID [--row-id ID] --state JSON|@file|- [--branch ID] [--dry-run] [--yes]` -- (since 0.84.2, #593) write a configuration's runtime `state` via the dedicated branch-scoped `PUT .../state` endpoint. `--state` must be a JSON object (not an array/scalar) under 4 MB. `--row-id` targets a row's state instead of the root. `--dry-run` previews the current-vs-new diff (same shape as `config update --dry-run`) without writing; a no-op (new state equals current) short-circuits with `changed: false` and no write. Guarded write: prompts for confirmation unless `--yes` or `--json`. This is the fix for `config update --set 'state...'`, which never reached runtime state (see [gotchas](references/gotchas.md)). See [config-state-workflow](references/config-state-workflow.md) for the seed-before-migrate playbook. diff --git a/plugins/kbagent/skills/kbagent/references/oauth-workflow.md b/plugins/kbagent/skills/kbagent/references/oauth-workflow.md new file mode 100644 index 000000000..08ee848e4 --- /dev/null +++ b/plugins/kbagent/skills/kbagent/references/oauth-workflow.md @@ -0,0 +1,29 @@ +# OAuth authorization workflow (`config oauth-url`) + +Getting an OAuth-based component (`keboola.ex-facebook-ads-v2`, `keboola.ex-google-analytics-v4`, ...) authorized for a configuration. + +## Steps + +1. `kbagent config oauth-url --project NAME --component-id ID --config-id ID` + Needs a **master** token (`canManageTokens`); a non-master token fails fast with `MISSING_MASTER_TOKEN` (exit 3). The minted token lives 1 hour, so generate the link when the user is ready to click through, not ahead of time. + Since vNEXT the command opens the URL in the user's default browser itself (interactive human mode only -- never under `--json`, never when stdout is not a terminal, suppressed by `--no-open`). +2. The user completes the provider's consent screen in that browser. +3. Verify: `kbagent config detail --project NAME --component-id ID --config-id ID` -- the `oauth_api.version` in the configuration is bumped (`3` -> `4`, ...) once credentials are stored. Do not claim success without this check; the browser tab tells you nothing. + +## Reporting the link in an answer + +Always give the user the URL as well as opening it -- they may be on a different machine, or the browser handoff may fail silently (an OS handler accepting a URL is not proof a window appeared). + +Put it in a **fenced code block**, on its own, never as inline prose: + +```` +Opened the Facebook Ads authorization link in your browser. If it did not open: + +``` +https://external.keboola.com/oauth/index.html?token=...&sapiUrl=...#/keboola.ex-facebook-ads-v2/01m0zkt36y0mwcvya1apnrvn3k +``` +```` + +The URL is ~200 chars and fits no terminal row. Inline text gets wrapped by the renderer, which then link-detects **only the first visual row** -- the click drops the trailing `#//` and the wizard answers `Failed to load config data. Please contact us on support@keboola.com` even though the token and stack URL are fine. A fenced block is not reflowed, so a copy is intact; clickability does not matter because the command already opened it. See [gotchas](gotchas.md). + +Never truncate, shorten or "prettify" the URL, and never split it across lines yourself. From 70eddfd13e920be77d55a402da28a4f11ba1d2fa Mon Sep 17 00:00:00 2001 From: soustruh Date: Sat, 10 Oct 2026 03:28:50 +0200 Subject: [PATCH 3/5] fix(config): wait for the browser opener, report browser_opened in --json, verify by the config version --- plugins/kbagent/skills/kbagent/SKILL.md | 2 +- .../kbagent/references/commands-reference.md | 2 +- .../kbagent/references/oauth-workflow.md | 13 ++-- .../commands/_config_oauth.py | 14 +++- tests/test_config_row_cli.py | 77 ++++++++++++++++++- 5 files changed, 97 insertions(+), 11 deletions(-) diff --git a/plugins/kbagent/skills/kbagent/SKILL.md b/plugins/kbagent/skills/kbagent/SKILL.md index c4b8a1d06..ffb4422eb 100644 --- a/plugins/kbagent/skills/kbagent/SKILL.md +++ b/plugins/kbagent/skills/kbagent/SKILL.md @@ -456,7 +456,7 @@ For detailed response parsing rules and common pitfalls, see [gotchas](reference | Dev branches | [branch-workflow](references/branch-workflow.md) | | **Merge requests** (dev branch -> production with review; conflicts + resolve; auto-merge is destructive; `--json merge` needs an explicit target) | [merge-request-workflow](references/merge-request-workflow.md) | | Encrypting secrets before a config write | [encrypt-workflow](references/encrypt-workflow.md) | -| **Authorizing an OAuth component** (`config oauth-url`; auto-open, always echo the URL in a fenced block, verify `oauth_api.version`) | [oauth-workflow](references/oauth-workflow.md) | +| **Authorizing an OAuth component** (`config oauth-url`; no browser under `--json`, always echo the URL in a fenced block, verify by the config `version` bump) | [oauth-workflow](references/oauth-workflow.md) | | Sync & Git-branching (GitOps) | [sync-workflow](references/sync-workflow.md) | | Sync row-level internals (manifest v3, hoist, encryption) | [sync-rows-workflow](references/sync-rows-workflow.md) | | **Promote configs source -> destination project** (from-scratch GitHub Actions pull -> validate -> push pipeline built on `sync`; PR-gated, cross-project dry-run diff) -- a **separate skill**, not a reference doc | [kbagent-promotion-pipeline](../kbagent-promotion-pipeline/SKILL.md) | diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index 87c8f40eb..c6780173f 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -159,7 +159,7 @@ Requires a **super-admin** Manage API token (same kind as `org setup`). Same def - `config row-create --project NAME --component-id ID --config-id ID --name ROW_NAME [--description D] [--configuration JSON|@file|-] [--is-disabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- create a new configuration row. Returns the full row dict with `id`, `name`, `version`. Optional `--configuration` accepts JSON inline, `@file`, or stdin (`-`). `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). - `config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--change-description TEXT] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- update an existing configuration row. Pass only the fields you want to change; omitted fields are preserved. `--is-disabled` / `--is-enabled` toggle the row's enabled state. `--change-description` sets the new row version's `changeDescription` audit line (default: auto-generated). `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). - `config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes]` -- delete a configuration row. Destructive (gated behind `--allow-destructive`). Branch-aware. Without `--yes` and outside `--json` mode, prompts for interactive confirmation; `--json` mode auto-skips the prompt. -- `config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open]` -- return the OAuth authorization URL for a component that uses OAuth authentication. **Requires a master Storage API token** (canManageTokens privilege) -- non-master tokens fail with `MISSING_MASTER_TOKEN` exit 3 on a fail-fast pre-flight check before any HTTP write happens. (since vNEXT) the URL is opened in the default browser automatically in interactive human mode (never under `--json` or when stdout is not a terminal); `--no-open` prints it only. The URL is also printed unwrapped for copying -- **never re-render it as prose in a chat answer**, wrapping renderers link only its first row, which drops the config id and makes the wizard answer `Failed to load config data`; echo it in a fenced code block instead. See [oauth-workflow](oauth-workflow.md). +- `config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open]` -- return the OAuth authorization URL for a component that uses OAuth authentication. **Requires a master Storage API token** (canManageTokens privilege) -- non-master tokens fail with `MISSING_MASTER_TOKEN` exit 3 on a fail-fast pre-flight check before any HTTP write happens. (since vNEXT) the URL is opened in the default browser automatically in interactive human mode (never under `--json` or when stdout is not a terminal); `--no-open` prints it only. The `--json` result has `browser_opened: false`: never tell the user the link is open. The URL is also printed unwrapped for copying -- **never re-render it as prose in a chat answer**, wrapping renderers link only its first row, which drops the config id and makes the wizard answer `Failed to load config data`; echo it in a fenced code block instead. See [oauth-workflow](oauth-workflow.md). - `config state-get --project NAME --component-id ID --config-id ID [--row-id ID] [--branch ID]` -- (since 0.84.2, #593) read a configuration's runtime `state`. Without `--row-id` returns the root config's state; with `--row-id` returns that row's state (a missing row id fails loudly, it does not silently return `{}`). For row-based components the root state node is unused -- read the row state instead. See [config-state-workflow](references/config-state-workflow.md). - `config state-set --project NAME --component-id ID --config-id ID [--row-id ID] --state JSON|@file|- [--branch ID] [--dry-run] [--yes]` -- (since 0.84.2, #593) write a configuration's runtime `state` via the dedicated branch-scoped `PUT .../state` endpoint. `--state` must be a JSON object (not an array/scalar) under 4 MB. `--row-id` targets a row's state instead of the root. `--dry-run` previews the current-vs-new diff (same shape as `config update --dry-run`) without writing; a no-op (new state equals current) short-circuits with `changed: false` and no write. Guarded write: prompts for confirmation unless `--yes` or `--json`. This is the fix for `config update --set 'state...'`, which never reached runtime state (see [gotchas](references/gotchas.md)). See [config-state-workflow](references/config-state-workflow.md) for the seed-before-migrate playbook. diff --git a/plugins/kbagent/skills/kbagent/references/oauth-workflow.md b/plugins/kbagent/skills/kbagent/references/oauth-workflow.md index 08ee848e4..bd1c7df6b 100644 --- a/plugins/kbagent/skills/kbagent/references/oauth-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/oauth-workflow.md @@ -6,24 +6,25 @@ Getting an OAuth-based component (`keboola.ex-facebook-ads-v2`, `keboola.ex-goog 1. `kbagent config oauth-url --project NAME --component-id ID --config-id ID` Needs a **master** token (`canManageTokens`); a non-master token fails fast with `MISSING_MASTER_TOKEN` (exit 3). The minted token lives 1 hour, so generate the link when the user is ready to click through, not ahead of time. - Since vNEXT the command opens the URL in the user's default browser itself (interactive human mode only -- never under `--json`, never when stdout is not a terminal, suppressed by `--no-open`). -2. The user completes the provider's consent screen in that browser. -3. Verify: `kbagent config detail --project NAME --component-id ID --config-id ID` -- the `oauth_api.version` in the configuration is bumped (`3` -> `4`, ...) once credentials are stored. Do not claim success without this check; the browser tab tells you nothing. + Since vNEXT the command opens the URL in the user's default browser itself (interactive human mode only -- never under `--json`, never when stdout is not a terminal, suppressed by `--no-open`). Under `--json` the result has `browser_opened: false`. + Before you give the link to the user, run `kbagent config detail --project NAME --component-id ID --config-id ID` and note the configuration `version`. Step 3 compares against it. +2. The user completes the provider's consent screen in the browser. +3. Verify: run the same `config detail` again. The authorization is stored when the configuration `version` is higher than the one you noted and `configuration.authorization.oauth_api.id` is set. Do not use `oauth_api.version` as the signal: it is the OAuth Broker API version, the wizard always writes `3`, so it does not change when the user authorizes. Do not claim success without this check; the browser tab tells you nothing. ## Reporting the link in an answer -Always give the user the URL as well as opening it -- they may be on a different machine, or the browser handoff may fail silently (an OS handler accepting a URL is not proof a window appeared). +Always give the user the URL. Tell the user that the link is open in the browser only when the result has `browser_opened: true`. Under `--json` it is always `false`: nothing was opened, so ask the user to open the link. Give the URL also when it is `true` -- the user may be on a different machine, or the browser handoff may fail silently (an OS handler accepting a URL is not proof a window appeared). Put it in a **fenced code block**, on its own, never as inline prose: ```` -Opened the Facebook Ads authorization link in your browser. If it did not open: +Open this Facebook Ads authorization link in your browser: ``` https://external.keboola.com/oauth/index.html?token=...&sapiUrl=...#/keboola.ex-facebook-ads-v2/01m0zkt36y0mwcvya1apnrvn3k ``` ```` -The URL is ~200 chars and fits no terminal row. Inline text gets wrapped by the renderer, which then link-detects **only the first visual row** -- the click drops the trailing `#//` and the wizard answers `Failed to load config data. Please contact us on support@keboola.com` even though the token and stack URL are fine. A fenced block is not reflowed, so a copy is intact; clickability does not matter because the command already opened it. See [gotchas](gotchas.md). +The URL is ~200 chars and fits no terminal row. Inline text gets wrapped by the renderer, which then link-detects **only the first visual row** -- the click drops the trailing `#//` and the wizard answers `Failed to load config data. Please contact us on support@keboola.com` even though the token and stack URL are fine. A fenced block is not reflowed, so a copy of it is intact. See [gotchas](gotchas.md). Never truncate, shorten or "prettify" the URL, and never split it across lines yourself. diff --git a/src/keboola_agent_cli/commands/_config_oauth.py b/src/keboola_agent_cli/commands/_config_oauth.py index ca49481a9..396e4a159 100644 --- a/src/keboola_agent_cli/commands/_config_oauth.py +++ b/src/keboola_agent_cli/commands/_config_oauth.py @@ -20,6 +20,12 @@ from ..errors import ConfigError, ErrorCode, KeboolaApiError from ._helpers import get_formatter, get_service, map_error_to_exit_code +# The command exits right after the open, and `open_browser` runs the opener +# on a daemon thread that dies with the process. Same wait as +# `data-app password --open`: long enough for `webbrowser` to start the +# opener, short enough that an opener it blocks on cannot hold the command. +_BROWSER_OPEN_WAIT_SECONDS = 2.0 + def _should_open(*, no_open: bool, is_terminal: bool) -> bool: """Whether to hand the URL to a browser: interactive use, unless opted out.""" @@ -76,7 +82,8 @@ def config_oauth_url( The browser is left alone in `--json` mode and when stdout is not a terminal, so scripted and piped callers never get a stray window; - `--no-open` suppresses it in interactive use too. + `--no-open` suppresses it in interactive use too. The `--json` output + has `browser_opened: false`: give the URL to the user. \b Examples: @@ -114,6 +121,9 @@ def config_oauth_url( url = result["url"] if formatter.json_mode: + # --json never opens a browser. The key tells an agent so, so that + # it gives the URL to the user and does not report an open. + result["browser_opened"] = False formatter.output(result) return @@ -128,7 +138,7 @@ def config_oauth_url( formatter.console.print(url, soft_wrap=True, highlight=False, markup=False) should_open = _should_open(no_open=no_open, is_terminal=formatter.console.is_terminal) - if should_open and open_browser(url): + if should_open and open_browser(url, wait_seconds=_BROWSER_OPEN_WAIT_SECONDS): formatter.console.print( "\n[dim]Opened in your default browser. Grant access there.[/dim]" ) diff --git a/tests/test_config_row_cli.py b/tests/test_config_row_cli.py index a38e20d70..a649f4f92 100644 --- a/tests/test_config_row_cli.py +++ b/tests/test_config_row_cli.py @@ -1,6 +1,7 @@ """CLI tests for config row-create, row-update, and oauth-url commands.""" import json +import time from pathlib import Path from unittest.mock import MagicMock @@ -8,6 +9,7 @@ from typer.testing import CliRunner, Result from helpers import setup_single_project +from keboola_agent_cli.auth import environment from keboola_agent_cli.cli import app from keboola_agent_cli.commands._config_oauth import _should_open from keboola_agent_cli.errors import KeboolaApiError @@ -748,7 +750,7 @@ def test_interactive_terminal_opens_the_complete_url(self, tmp_config_dir: Path) lambda ctx, name: service, ) - def _record(url: str) -> bool: + def _record(url: str, **_kwargs: float) -> bool: opened.append(url) return True @@ -778,6 +780,79 @@ def _always_open(**_kwargs: bool) -> bool: assert opened == [OAUTH_RESULT["url"]] assert "Opened in your default browser" in result.output + def test_opener_has_run_when_the_command_returns(self, tmp_config_dir: Path) -> None: + """The process exits right after the command, and the opener thread dies with it. + + The fake opener is slow on purpose: without the wait, the command + returns (and reports an open) before the opener has run. + """ + service = self._make_oauth_service(tmp_config_dir) + opened: list[str] = [] + + def _slow_open(url: str) -> bool: + time.sleep(0.2) + opened.append(url) + return True + + def _always_open(**_kwargs: bool) -> bool: + return True + + with pytest.MonkeyPatch.context() as mp: + # The real open_browser, on its webbrowser path: no WSL, no real browser. + mp.delenv("WSL_INTEROP", raising=False) + mp.setattr(environment, "_wslview_is_working", lambda: False) + mp.setattr(environment.webbrowser, "get", lambda: object()) + mp.setattr(environment.webbrowser, "open", _slow_open) + mp.setattr( + "keboola_agent_cli.commands._config_oauth.get_service", + lambda ctx, name: service, + ) + mp.setattr("keboola_agent_cli.commands._config_oauth._should_open", _always_open) + result = runner.invoke( + app, + [ + "--config-dir", + str(tmp_config_dir), + "config", + "oauth-url", + "--project", + "prod", + "--component-id", + "keboola.ex-google-drive", + "--config-id", + "cfg-001", + ], + ) + assert opened == [OAUTH_RESULT["url"]] + + assert result.exit_code == 0, result.output + assert "Opened in your default browser" in result.output + + def test_json_mode_says_no_browser_opened(self, tmp_config_dir: Path) -> None: + """An agent runs with --json: the output must tell it that nothing opened.""" + service = self._make_oauth_service(tmp_config_dir) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "keboola_agent_cli.commands._config_oauth.get_service", + lambda ctx, name: service, + ) + result = _invoke( + tmp_config_dir, + "oauth-url", + [ + "--project", + "prod", + "--component-id", + "keboola.ex-google-drive", + "--config-id", + "cfg-001", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.output)["data"]["browser_opened"] is False + class TestShouldOpenBrowser: """The interactive-only gate on the automatic browser open.""" From d2ef6fe4acd4cebd886d73691483047f63b4015e Mon Sep 17 00:00:00 2001 From: soustruh Date: Sat, 10 Oct 2026 03:52:05 +0200 Subject: [PATCH 4/5] fix(auth): open the browser through wslview without blocking, add --no-open to the command lists --- CLAUDE.md | 4 +- .../kbagent/references/oauth-workflow.md | 2 +- src/keboola_agent_cli/auth/environment.py | 37 ++++---- .../commands/_config_oauth.py | 2 +- src/keboola_agent_cli/commands/context.py | 3 +- tests/test_auth_environment.py | 90 ++++++++++++++++++- tests/test_oauth_workflow_doc.py | 52 +++++++++++ 7 files changed, 164 insertions(+), 26 deletions(-) create mode 100644 tests/test_oauth_workflow_doc.py diff --git a/CLAUDE.md b/CLAUDE.md index af707b045..d4a73d6ac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -555,7 +555,9 @@ kbagent config set-folder --project NAME --component-id ID --config-id ID --name kbagent config row-create --project NAME --component-id ID --config-id ID --name ROW_NAME [--description D] [--configuration JSON|@file|-] [--is-disabled] [--branch ID] [--allow-plaintext-on-encrypt-failure] kbagent config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--change-description TEXT] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure] kbagent config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes] -kbagent config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] +kbagent config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open] +# oauth-url: opens the link in the default browser (interactive human mode only; never with +# --json); --no-open only prints it. Version gate in gotchas.md. kbagent config state-get --project NAME --component-id ID --config-id ID [--row-id ID] [--branch ID] kbagent config state-set --project NAME --component-id ID --config-id ID [--row-id ID] --state JSON|@file|- [--branch ID] [--dry-run] [--yes] # state-get/state-set (0.84.2+, #593): read/write a config's runtime state via the dedicated diff --git a/plugins/kbagent/skills/kbagent/references/oauth-workflow.md b/plugins/kbagent/skills/kbagent/references/oauth-workflow.md index bd1c7df6b..2f70cec44 100644 --- a/plugins/kbagent/skills/kbagent/references/oauth-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/oauth-workflow.md @@ -5,7 +5,7 @@ Getting an OAuth-based component (`keboola.ex-facebook-ads-v2`, `keboola.ex-goog ## Steps 1. `kbagent config oauth-url --project NAME --component-id ID --config-id ID` - Needs a **master** token (`canManageTokens`); a non-master token fails fast with `MISSING_MASTER_TOKEN` (exit 3). The minted token lives 1 hour, so generate the link when the user is ready to click through, not ahead of time. + Needs a **master** token (`canManageTokens`); a non-master token fails fast with `MISSING_MASTER_TOKEN` (exit 3). The created token lives 1 hour, so generate the link when the user is ready to click through, not ahead of time. Since vNEXT the command opens the URL in the user's default browser itself (interactive human mode only -- never under `--json`, never when stdout is not a terminal, suppressed by `--no-open`). Under `--json` the result has `browser_opened: false`. Before you give the link to the user, run `kbagent config detail --project NAME --component-id ID --config-id ID` and note the configuration `version`. Step 3 compares against it. 2. The user completes the provider's consent screen in the browser. diff --git a/src/keboola_agent_cli/auth/environment.py b/src/keboola_agent_cli/auth/environment.py index b054acb2c..6ea51f530 100644 --- a/src/keboola_agent_cli/auth/environment.py +++ b/src/keboola_agent_cli/auth/environment.py @@ -172,24 +172,22 @@ def detect_browser_environment() -> BrowserEnvironment: return BrowserEnvironment(loopback_browser_usable=True, reason="", opener=opener) -def _open_via_wslview(url: str) -> bool: - """Hand ``url`` to the Windows-side default browser through `wslview`. +def _open_via_wslview(url: str) -> None: + """Thread target for `open_browser` under WSL: hand ``url`` to `wslview`. - Runs synchronously so the exit status is the answer: unlike the - `webbrowser` path there is no thread to swallow, and the whole point is to - know whether Windows accepted the URL. Output is captured, never logged -- - the URL must not reach stderr. + `wslview` passes the URL to the Windows-side default browser. Output is + captured, never logged, and every error is swallowed -- the URL must not + reach stderr (see `_open_silently`). """ try: - completed = subprocess.run( + subprocess.run( ["wslview", url], capture_output=True, timeout=_WSLVIEW_OPEN_TIMEOUT_SECONDS, check=False, ) - except (OSError, subprocess.SubprocessError): - return False - return completed.returncode == 0 + except Exception: + return def open_browser(url: str, *, wait_seconds: float = 0.0) -> bool: @@ -211,16 +209,19 @@ def open_browser(url: str, *, wait_seconds: float = 0.0) -> bool: handling off ``sys.platform == "win32"``, which is ``linux`` inside WSL, so it would either find no handler at all or open a browser in the Linux session -- a separate profile that shares none of the logins the URL needs. + It runs on the same daemon thread and honors ``wait_seconds`` the same way; + a working `wslview` (probed synchronously) counts as the available handler. """ if _env_flag_set("WSL_INTEROP") and _wslview_is_working(): - return _open_via_wslview(url) - - try: - webbrowser.get() - except webbrowser.Error: - return False - - thread = threading.Thread(target=_open_silently, args=(url,), daemon=True) + target = _open_via_wslview + else: + try: + webbrowser.get() + except webbrowser.Error: + return False + target = _open_silently + + thread = threading.Thread(target=target, args=(url,), daemon=True) thread.start() if wait_seconds > 0: thread.join(timeout=wait_seconds) diff --git a/src/keboola_agent_cli/commands/_config_oauth.py b/src/keboola_agent_cli/commands/_config_oauth.py index 396e4a159..b0a6dc940 100644 --- a/src/keboola_agent_cli/commands/_config_oauth.py +++ b/src/keboola_agent_cli/commands/_config_oauth.py @@ -73,7 +73,7 @@ def config_oauth_url( ) -> None: """Generate an OAuth authorization URL for a component configuration. - Mints a short-lived, component-scoped authorization link and opens it in + Creates a short-lived, component-scoped authorization link and opens it in the default browser, so the URL never has to be clicked or copied: it is ~200 characters, and a terminal or chat renderer that wraps it turns the visible link into its first row only, which drops the configuration id diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index 17fbca623..4f01d022b 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -574,9 +574,10 @@ kbagent config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes] Delete a configuration row. Destructive; --yes to skip confirmation prompt. - kbagent config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] + kbagent config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open] Return the OAuth authorization URL for a component that uses OAuth authentication. Open the URL in a browser to complete the OAuth flow. + Opens it in the default browser in interactive human mode (never with --json); --no-open only prints it (since vNEXT). kbagent config state-get --project NAME --component-id ID --config-id ID [--row-id ID] [--branch ID] Read a configuration's runtime state (the same dict --with-state attaches diff --git a/tests/test_auth_environment.py b/tests/test_auth_environment.py index 3f9cf627a..db15657e5 100644 --- a/tests/test_auth_environment.py +++ b/tests/test_auth_environment.py @@ -10,6 +10,8 @@ from __future__ import annotations import subprocess +import threading +import time from collections.abc import Callable from typing import Any @@ -385,6 +387,12 @@ def _boom(_url: str) -> bool: class TestOpenBrowserUnderWsl: """WSL is `sys.platform == "linux"`, so `webbrowser` never reaches Windows itself.""" + @staticmethod + def _join_other_threads() -> None: + for thread in list(environment.threading.enumerate()): + if thread is not environment.threading.current_thread(): + thread.join(timeout=1.0) + def test_hands_the_url_to_wslview(self, monkeypatch: pytest.MonkeyPatch) -> None: calls: list[list[str]] = [] @@ -399,20 +407,94 @@ def _run(cmd: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[bytes]: url = "https://external.keboola.com/oauth/index.html?token=t&sapiUrl=u#/comp/cfg" assert open_browser(url) is True + self._join_other_threads() assert calls == [["wslview", "--version"], ["wslview", url]] - def test_reports_failure_when_wslview_rejects_the_url( + def test_slow_wslview_does_not_block_a_caller_that_does_not_wait( self, monkeypatch: pytest.MonkeyPatch ) -> None: + """`auth login` passes no ``wait_seconds``: the open must stay fire-and-forget.""" + release = threading.Event() + opened = threading.Event() + + def _run(cmd: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[bytes]: + if cmd[1:] != ["--version"]: + release.wait(timeout=5.0) + opened.set() + return subprocess.CompletedProcess(cmd, 0, b"", b"") + + monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") + monkeypatch.setattr(environment.shutil, "which", lambda _name: "/usr/bin/wslview") + monkeypatch.setattr(environment.subprocess, "run", _run) + + started = time.monotonic() + assert open_browser("https://external.keboola.com/oauth/index.html?token=t") is True + assert time.monotonic() - started < 1.0 + assert not opened.is_set() + + release.set() + self._join_other_threads() + assert opened.is_set() + + def test_waits_for_wslview_up_to_wait_seconds(self, monkeypatch: pytest.MonkeyPatch) -> None: + """`config oauth-url` exits right after the open, so it waits for `wslview`.""" + opened = threading.Event() + def _run(cmd: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[bytes]: - code = 0 if cmd[1:] == ["--version"] else 1 - return subprocess.CompletedProcess(cmd, code, b"", b"") + if cmd[1:] != ["--version"]: + time.sleep(0.2) + opened.set() + return subprocess.CompletedProcess(cmd, 0, b"", b"") monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") monkeypatch.setattr(environment.shutil, "which", lambda _name: "/usr/bin/wslview") monkeypatch.setattr(environment.subprocess, "run", _run) - assert open_browser("https://external.keboola.com/oauth/index.html?token=t") is False + assert ( + open_browser("https://external.keboola.com/oauth/index.html?token=t", wait_seconds=2.0) + is True + ) + assert opened.is_set() + + def test_wait_seconds_bounds_a_wedged_wslview(self, monkeypatch: pytest.MonkeyPatch) -> None: + release = threading.Event() + + def _run(cmd: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[bytes]: + if cmd[1:] != ["--version"]: + release.wait(timeout=5.0) + return subprocess.CompletedProcess(cmd, 0, b"", b"") + + monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") + monkeypatch.setattr(environment.shutil, "which", lambda _name: "/usr/bin/wslview") + monkeypatch.setattr(environment.subprocess, "run", _run) + + started = time.monotonic() + assert ( + open_browser("https://external.keboola.com/oauth/index.html?token=t", wait_seconds=0.2) + is True + ) + assert time.monotonic() - started < 1.0 + + release.set() + self._join_other_threads() + + def test_wslview_error_is_swallowed( + self, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + def _run(cmd: list[str], **_kwargs: Any) -> subprocess.CompletedProcess[bytes]: + if cmd[1:] != ["--version"]: + raise subprocess.TimeoutExpired(cmd, 10.0) + return subprocess.CompletedProcess(cmd, 0, b"", b"") + + monkeypatch.setenv("WSL_INTEROP", "/run/WSL/1_interop") + monkeypatch.setattr(environment.shutil, "which", lambda _name: "/usr/bin/wslview") + monkeypatch.setattr(environment.subprocess, "run", _run) + + assert ( + open_browser("https://external.keboola.com/oauth/index.html?token=t", wait_seconds=1.0) + is True + ) + assert "token=t" not in capsys.readouterr().err def test_falls_back_to_webbrowser_without_a_working_wslview( self, monkeypatch: pytest.MonkeyPatch diff --git a/tests/test_oauth_workflow_doc.py b/tests/test_oauth_workflow_doc.py new file mode 100644 index 000000000..71d55288c --- /dev/null +++ b/tests/test_oauth_workflow_doc.py @@ -0,0 +1,52 @@ +"""The OAuth workflow doc must tell an agent what the wizard and the command really do. + +Two claims in the first version of ``oauth-workflow.md`` were wrong: + +- It told the agent to wait until ``authorization.oauth_api.version`` goes + ``3`` -> ``4``. That field is the OAuth Broker API version: the wizard always + writes ``3`` (help.keboola.com, "OAuth" in the common interface), so the + check never passes and an agent reports a failure after a success. The + configuration ``version`` goes up when the wizard saves the credentials. +- Its model answer said "Opened ... in your browser", but agents run with + ``--json``, which never opens a browser (``browser_opened: false``). +""" + +from __future__ import annotations + +from pathlib import Path + +SKILL_DIR = Path(__file__).parent.parent / "plugins/kbagent/skills/kbagent" +WORKFLOW = SKILL_DIR / "references/oauth-workflow.md" + + +def _section(heading: str) -> str: + """The body of one ``## `` section of the workflow doc.""" + text = WORKFLOW.read_text(encoding="utf-8") + parts = text.split(f"## {heading}\n", 1) + assert len(parts) == 2, f"section {heading!r} is gone" + return parts[1].split("\n## ", 1)[0] + + +def _verify_step() -> str: + steps = _section("Steps") + parts = steps.split("\n3. ", 1) + assert len(parts) == 2, "the verify step (3.) is gone" + return parts[1] + + +def test_verify_step_uses_the_config_version_and_the_credentials_id() -> None: + verify = _verify_step() + assert "configuration `version`" in verify + assert "oauth_api.id" in verify + + +def test_no_doc_expects_oauth_api_version_to_change() -> None: + assert "-> `4`" not in WORKFLOW.read_text(encoding="utf-8") + skill = (SKILL_DIR / "SKILL.md").read_text(encoding="utf-8") + assert "verify `oauth_api.version`" not in skill + + +def test_model_answer_does_not_claim_an_open() -> None: + reporting = _section("Reporting the link in an answer") + assert "browser_opened" in reporting + assert "Opened the" not in reporting From 7ca73e1d91bc26c193ef3e91a0173ad756273115 Mon Sep 17 00:00:00 2001 From: soustruh Date: Sat, 10 Oct 2026 08:55:13 +0200 Subject: [PATCH 5/5] fix(config): open the oauth-url link only where auth login would open a browser --- CLAUDE.md | 3 +- .../kbagent/references/commands-reference.md | 2 +- .../skills/kbagent/references/gotchas.md | 5 +- .../kbagent/references/oauth-workflow.md | 2 +- .../commands/_config_oauth.py | 21 +- tests/test_config_row_cli.py | 208 +++++++++--------- tests/test_oauth_workflow_doc.py | 52 ----- 7 files changed, 122 insertions(+), 171 deletions(-) delete mode 100644 tests/test_oauth_workflow_doc.py diff --git a/CLAUDE.md b/CLAUDE.md index d4a73d6ac..c4380ebb2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -557,7 +557,8 @@ kbagent config row-update --project NAME --component-id ID --config-id ID --row- kbagent config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes] kbagent config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open] # oauth-url: opens the link in the default browser (interactive human mode only; never with -# --json); --no-open only prints it. Version gate in gotchas.md. +# --json, over SSH, in a container or in WSL without wslview); --no-open only prints it. +# Version gate in gotchas.md. kbagent config state-get --project NAME --component-id ID --config-id ID [--row-id ID] [--branch ID] kbagent config state-set --project NAME --component-id ID --config-id ID [--row-id ID] --state JSON|@file|- [--branch ID] [--dry-run] [--yes] # state-get/state-set (0.84.2+, #593): read/write a config's runtime state via the dedicated diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index c6780173f..738e2788c 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -159,7 +159,7 @@ Requires a **super-admin** Manage API token (same kind as `org setup`). Same def - `config row-create --project NAME --component-id ID --config-id ID --name ROW_NAME [--description D] [--configuration JSON|@file|-] [--is-disabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- create a new configuration row. Returns the full row dict with `id`, `name`, `version`. Optional `--configuration` accepts JSON inline, `@file`, or stdin (`-`). `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). - `config row-update --project NAME --component-id ID --config-id ID --row-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--change-description TEXT] [--is-disabled | --is-enabled] [--branch ID] [--allow-plaintext-on-encrypt-failure]` -- update an existing configuration row. Pass only the fields you want to change; omitted fields are preserved. `--is-disabled` / `--is-enabled` toggle the row's enabled state. `--change-description` sets the new row version's `changeDescription` audit line (default: auto-generated). `#`-prefixed secrets auto-encrypt before write (fail-closed; since 0.54.0, #378). - `config row-delete --project NAME --component-id ID --config-id ID --row-id ID [--branch ID] [--yes]` -- delete a configuration row. Destructive (gated behind `--allow-destructive`). Branch-aware. Without `--yes` and outside `--json` mode, prompts for interactive confirmation; `--json` mode auto-skips the prompt. -- `config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open]` -- return the OAuth authorization URL for a component that uses OAuth authentication. **Requires a master Storage API token** (canManageTokens privilege) -- non-master tokens fail with `MISSING_MASTER_TOKEN` exit 3 on a fail-fast pre-flight check before any HTTP write happens. (since vNEXT) the URL is opened in the default browser automatically in interactive human mode (never under `--json` or when stdout is not a terminal); `--no-open` prints it only. The `--json` result has `browser_opened: false`: never tell the user the link is open. The URL is also printed unwrapped for copying -- **never re-render it as prose in a chat answer**, wrapping renderers link only its first row, which drops the config id and makes the wizard answer `Failed to load config data`; echo it in a fenced code block instead. See [oauth-workflow](oauth-workflow.md). +- `config oauth-url --project NAME --component-id ID --config-id ID [--redirect-url URL] [--no-open]` -- return the OAuth authorization URL for a component that uses OAuth authentication. **Requires a master Storage API token** (canManageTokens privilege) -- non-master tokens fail with `MISSING_MASTER_TOKEN` exit 3 on a fail-fast pre-flight check before any HTTP write happens. (since vNEXT) the URL is opened in the default browser automatically in interactive human mode (never under `--json`, when stdout is not a terminal, or where `auth login` would not open a browser either: SSH session, container, WSL without a working `wslview`); `--no-open` prints it only. The `--json` result has `browser_opened: false`: never tell the user the link is open. The URL is also printed unwrapped for copying -- **never re-render it as prose in a chat answer**, wrapping renderers link only its first row, which drops the config id and makes the wizard answer `Failed to load config data`; echo it in a fenced code block instead. See [oauth-workflow](oauth-workflow.md). - `config state-get --project NAME --component-id ID --config-id ID [--row-id ID] [--branch ID]` -- (since 0.84.2, #593) read a configuration's runtime `state`. Without `--row-id` returns the root config's state; with `--row-id` returns that row's state (a missing row id fails loudly, it does not silently return `{}`). For row-based components the root state node is unused -- read the row state instead. See [config-state-workflow](references/config-state-workflow.md). - `config state-set --project NAME --component-id ID --config-id ID [--row-id ID] --state JSON|@file|- [--branch ID] [--dry-run] [--yes]` -- (since 0.84.2, #593) write a configuration's runtime `state` via the dedicated branch-scoped `PUT .../state` endpoint. `--state` must be a JSON object (not an array/scalar) under 4 MB. `--row-id` targets a row's state instead of the root. `--dry-run` previews the current-vs-new diff (same shape as `config update --dry-run`) without writing; a no-op (new state equals current) short-circuits with `changed: false` and no write. Guarded write: prompts for confirmation unless `--yes` or `--json`. This is the fix for `config update --set 'state...'`, which never reached runtime state (see [gotchas](references/gotchas.md)). See [config-state-workflow](references/config-state-workflow.md) for the seed-before-migrate playbook. diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index b066cb648..1e519954e 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -6075,7 +6075,10 @@ not, so the wizard authenticates and then 404s on the configuration: not control (e.g. an assistant transcript). Since vNEXT the command therefore opens the URL itself (interactive human mode -only; `--no-open` opts out) and prints it unwrapped as a copy fallback. Under +only; `--no-open` opts out) and prints it unwrapped as a copy fallback. It +skips the open where `auth login` skips it too: an SSH session (`webbrowser` +would start a console browser such as lynx or w3m on the terminal), a +container, and WSL without a working `wslview`. Under WSL `open_browser` routes through `wslview`, because `webbrowser` keys its Windows handling off `sys.platform == "win32"` -- `linux` inside WSL -- and would otherwise find no handler or open a Linux-side browser that shares none diff --git a/plugins/kbagent/skills/kbagent/references/oauth-workflow.md b/plugins/kbagent/skills/kbagent/references/oauth-workflow.md index 2f70cec44..1c4bf1143 100644 --- a/plugins/kbagent/skills/kbagent/references/oauth-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/oauth-workflow.md @@ -6,7 +6,7 @@ Getting an OAuth-based component (`keboola.ex-facebook-ads-v2`, `keboola.ex-goog 1. `kbagent config oauth-url --project NAME --component-id ID --config-id ID` Needs a **master** token (`canManageTokens`); a non-master token fails fast with `MISSING_MASTER_TOKEN` (exit 3). The created token lives 1 hour, so generate the link when the user is ready to click through, not ahead of time. - Since vNEXT the command opens the URL in the user's default browser itself (interactive human mode only -- never under `--json`, never when stdout is not a terminal, suppressed by `--no-open`). Under `--json` the result has `browser_opened: false`. + Since vNEXT the command opens the URL in the user's default browser itself (interactive human mode only -- never under `--json`, never when stdout is not a terminal, never over SSH, in a container or in WSL without a working `wslview`, suppressed by `--no-open`). Under `--json` the result has `browser_opened: false`. Before you give the link to the user, run `kbagent config detail --project NAME --component-id ID --config-id ID` and note the configuration `version`. Step 3 compares against it. 2. The user completes the provider's consent screen in the browser. 3. Verify: run the same `config detail` again. The authorization is stored when the configuration `version` is higher than the one you noted and `configuration.authorization.oauth_api.id` is set. Do not use `oauth_api.version` as the signal: it is the OAuth Broker API version, the wizard always writes `3`, so it does not change when the user authorizes. Do not claim success without this check; the browser tab tells you nothing. diff --git a/src/keboola_agent_cli/commands/_config_oauth.py b/src/keboola_agent_cli/commands/_config_oauth.py index b0a6dc940..d2d63477f 100644 --- a/src/keboola_agent_cli/commands/_config_oauth.py +++ b/src/keboola_agent_cli/commands/_config_oauth.py @@ -16,7 +16,7 @@ import typer from rich.markup import escape -from ..auth.environment import open_browser +from ..auth.environment import detect_browser_environment, open_browser from ..errors import ConfigError, ErrorCode, KeboolaApiError from ._helpers import get_formatter, get_service, map_error_to_exit_code @@ -27,11 +27,6 @@ _BROWSER_OPEN_WAIT_SECONDS = 2.0 -def _should_open(*, no_open: bool, is_terminal: bool) -> bool: - """Whether to hand the URL to a browser: interactive use, unless opted out.""" - return is_terminal and not no_open - - def register(app: typer.Typer) -> None: """Mount the oauth-url command onto ``app`` (the ``config`` Typer group).""" @@ -82,7 +77,9 @@ def config_oauth_url( The browser is left alone in `--json` mode and when stdout is not a terminal, so scripted and piped callers never get a stray window; - `--no-open` suppresses it in interactive use too. The `--json` output + `--no-open` suppresses it in interactive use too. It is also left alone + where `auth login` would not open one either: an SSH session, a + container, or WSL without a working `wslview`. The `--json` output has `browser_opened: false`: give the URL to the user. \b @@ -137,7 +134,15 @@ def config_oauth_url( # being read as a style tag. formatter.console.print(url, soft_wrap=True, highlight=False, markup=False) - should_open = _should_open(no_open=no_open, is_terminal=formatter.console.is_terminal) + # Same rule as `auth login`: over SSH `webbrowser` would start a console + # browser (lynx, w3m) on the user's terminal, and WSL without `wslview` + # gets a Linux-side browser without the user's logins. The detection + # runs last, so --no-open and piped runs skip its `wslview` probe. + should_open = ( + formatter.console.is_terminal + and not no_open + and detect_browser_environment().loopback_browser_usable + ) if should_open and open_browser(url, wait_seconds=_BROWSER_OPEN_WAIT_SECONDS): formatter.console.print( "\n[dim]Opened in your default browser. Grant access there.[/dim]" diff --git a/tests/test_config_row_cli.py b/tests/test_config_row_cli.py index a649f4f92..b5951bf65 100644 --- a/tests/test_config_row_cli.py +++ b/tests/test_config_row_cli.py @@ -11,7 +11,6 @@ from helpers import setup_single_project from keboola_agent_cli.auth import environment from keboola_agent_cli.cli import app -from keboola_agent_cli.commands._config_oauth import _should_open from keboola_agent_cli.errors import KeboolaApiError from keboola_agent_cli.services.config_service import ConfigService @@ -677,110 +676,124 @@ def test_human_output_keeps_url_on_one_line(self, tmp_config_dir: Path, columns: assert result.exit_code == 0, result.output assert OAUTH_RESULT["url"] in result.output - def test_no_open_flag_leaves_the_browser_alone(self, tmp_config_dir: Path) -> None: - """--no-open never dispatches an open, whatever the console reports.""" + def _run_oauth_url( + self, + tmp_config_dir: Path, + mp: pytest.MonkeyPatch, + *extra: str, + json_mode: bool = False, + ) -> tuple[Result, list[str]]: + """Run oauth-url with a recording opener; return the result and the opened URLs.""" service = self._make_oauth_service(tmp_config_dir) + opened: list[str] = [] - def _fail(_url: str) -> bool: - raise AssertionError("browser must not be opened with --no-open") + def _record(url: str, **_kwargs: float) -> bool: + opened.append(url) + return True + mp.setattr( + "keboola_agent_cli.commands._config_oauth.get_service", + lambda ctx, name: service, + ) + mp.setattr("keboola_agent_cli.commands._config_oauth.open_browser", _record) + result = runner.invoke( + app, + [ + *(["--json"] if json_mode else []), + "--config-dir", + str(tmp_config_dir), + "config", + "oauth-url", + "--project", + "prod", + "--component-id", + "keboola.ex-google-drive", + "--config-id", + "cfg-001", + *extra, + ], + env={"COLUMNS": "40"}, + ) + return result, opened + + @staticmethod + def _desktop_environment(mp: pytest.MonkeyPatch) -> None: + """A machine where `auth login` would open a browser too.""" + mp.setattr( + "keboola_agent_cli.commands._config_oauth.detect_browser_environment", + lambda: environment.BrowserEnvironment( + loopback_browser_usable=True, reason="", opener="xdg-open" + ), + ) + + def test_interactive_terminal_opens_the_complete_url( + self, tmp_config_dir: Path, force_colour: None + ) -> None: + """On a terminal the whole URL -- not a wrapped fragment -- reaches the browser.""" with pytest.MonkeyPatch.context() as mp: - mp.setattr( - "keboola_agent_cli.commands._config_oauth.get_service", - lambda ctx, name: service, - ) - mp.setattr("keboola_agent_cli.commands._config_oauth.open_browser", _fail) - result = runner.invoke( - app, - [ - "--config-dir", - str(tmp_config_dir), - "config", - "oauth-url", - "--project", - "prod", - "--component-id", - "keboola.ex-google-drive", - "--config-id", - "cfg-001", - "--no-open", - ], - ) + self._desktop_environment(mp) + result, opened = self._run_oauth_url(tmp_config_dir, mp) assert result.exit_code == 0, result.output - assert OAUTH_RESULT["url"] in result.output - - def test_json_mode_leaves_the_browser_alone(self, tmp_config_dir: Path) -> None: - """--json is a machine surface: no browser, unchanged payload.""" - service = self._make_oauth_service(tmp_config_dir) - - def _fail(_url: str) -> bool: - raise AssertionError("browser must not be opened in --json mode") + assert opened == [OAUTH_RESULT["url"]] + assert "Opened in your default browser" in result.output + @pytest.mark.parametrize( + ("extra", "json_mode"), + [(["--no-open"], False), ([], True)], + ids=["no-open", "json"], + ) + def test_terminal_run_with_opt_out_leaves_the_browser_alone( + self, tmp_config_dir: Path, force_colour: None, extra: list[str], json_mode: bool + ) -> None: + """--no-open and --json never open a browser, even on a terminal.""" with pytest.MonkeyPatch.context() as mp: - mp.setattr( - "keboola_agent_cli.commands._config_oauth.get_service", - lambda ctx, name: service, - ) - mp.setattr("keboola_agent_cli.commands._config_oauth.open_browser", _fail) - result = _invoke( - tmp_config_dir, - "oauth-url", - [ - "--project", - "prod", - "--component-id", - "keboola.ex-google-drive", - "--config-id", - "cfg-001", - ], - ) + self._desktop_environment(mp) + result, opened = self._run_oauth_url(tmp_config_dir, mp, *extra, json_mode=json_mode) assert result.exit_code == 0, result.output - assert json.loads(result.output)["data"]["url"] == OAUTH_RESULT["url"] - - def test_interactive_terminal_opens_the_complete_url(self, tmp_config_dir: Path) -> None: - """On a terminal the whole URL -- not a wrapped fragment -- reaches the browser.""" - service = self._make_oauth_service(tmp_config_dir) - opened: list[str] = [] + assert opened == [] + assert "Opened in your default browser" not in result.output + def test_non_terminal_run_leaves_the_browser_alone(self, tmp_config_dir: Path) -> None: + """A piped run (CliRunner stdout is no terminal) prints the URL only.""" with pytest.MonkeyPatch.context() as mp: - mp.setattr( - "keboola_agent_cli.commands._config_oauth.get_service", - lambda ctx, name: service, - ) + self._desktop_environment(mp) + result, opened = self._run_oauth_url(tmp_config_dir, mp) - def _record(url: str, **_kwargs: float) -> bool: - opened.append(url) - return True + assert result.exit_code == 0, result.output + assert opened == [] + assert OAUTH_RESULT["url"] in result.output + assert "Open this URL in a browser" in result.output - def _always_open(**_kwargs: bool) -> bool: - return True + @pytest.mark.parametrize( + ("env_var", "value"), + [("SSH_CONNECTION", "10.0.0.1 50000 10.0.0.2 22"), ("WSL_INTEROP", "/run/WSL/1_interop")], + ids=["ssh", "wsl-without-wslview"], + ) + def test_terminal_without_a_usable_browser_leaves_the_browser_alone( + self, tmp_config_dir: Path, force_colour: None, env_var: str, value: str + ) -> None: + """Where `auth login` would not open a browser, oauth-url does not either. - mp.setattr("keboola_agent_cli.commands._config_oauth.open_browser", _record) - mp.setattr("keboola_agent_cli.commands._config_oauth._should_open", _always_open) - result = runner.invoke( - app, - [ - "--config-dir", - str(tmp_config_dir), - "config", - "oauth-url", - "--project", - "prod", - "--component-id", - "keboola.ex-google-drive", - "--config-id", - "cfg-001", - ], - env={"COLUMNS": "40"}, - ) + Runs the real `detect_browser_environment`: over SSH `webbrowser` would + start a console browser on the user's terminal. + """ + with pytest.MonkeyPatch.context() as mp: + for name in ("SSH_CONNECTION", "SSH_TTY", "WSL_INTEROP"): + mp.delenv(name, raising=False) + mp.setattr(environment, "_is_containerized", lambda: False) + mp.setattr(environment, "_wslview_is_working", lambda: False) + mp.setenv(env_var, value) + result, opened = self._run_oauth_url(tmp_config_dir, mp) assert result.exit_code == 0, result.output - assert opened == [OAUTH_RESULT["url"]] - assert "Opened in your default browser" in result.output + assert opened == [] + assert "Open this URL in a browser" in result.output - def test_opener_has_run_when_the_command_returns(self, tmp_config_dir: Path) -> None: + def test_opener_has_run_when_the_command_returns( + self, tmp_config_dir: Path, force_colour: None + ) -> None: """The process exits right after the command, and the opener thread dies with it. The fake opener is slow on purpose: without the wait, the command @@ -794,9 +807,6 @@ def _slow_open(url: str) -> bool: opened.append(url) return True - def _always_open(**_kwargs: bool) -> bool: - return True - with pytest.MonkeyPatch.context() as mp: # The real open_browser, on its webbrowser path: no WSL, no real browser. mp.delenv("WSL_INTEROP", raising=False) @@ -807,7 +817,7 @@ def _always_open(**_kwargs: bool) -> bool: "keboola_agent_cli.commands._config_oauth.get_service", lambda ctx, name: service, ) - mp.setattr("keboola_agent_cli.commands._config_oauth._should_open", _always_open) + self._desktop_environment(mp) result = runner.invoke( app, [ @@ -854,22 +864,6 @@ def test_json_mode_says_no_browser_opened(self, tmp_config_dir: Path) -> None: assert json.loads(result.output)["data"]["browser_opened"] is False -class TestShouldOpenBrowser: - """The interactive-only gate on the automatic browser open.""" - - @pytest.mark.parametrize( - ("no_open", "is_terminal", "expected"), - [ - (False, True, True), - (True, True, False), - (False, False, False), - (True, False, False), - ], - ) - def test_gate(self, no_open: bool, is_terminal: bool, expected: bool) -> None: - assert _should_open(no_open=no_open, is_terminal=is_terminal) is expected - - # --------------------------------------------------------------------------- # is_disabled / is_enabled CLI tests # --------------------------------------------------------------------------- diff --git a/tests/test_oauth_workflow_doc.py b/tests/test_oauth_workflow_doc.py deleted file mode 100644 index 71d55288c..000000000 --- a/tests/test_oauth_workflow_doc.py +++ /dev/null @@ -1,52 +0,0 @@ -"""The OAuth workflow doc must tell an agent what the wizard and the command really do. - -Two claims in the first version of ``oauth-workflow.md`` were wrong: - -- It told the agent to wait until ``authorization.oauth_api.version`` goes - ``3`` -> ``4``. That field is the OAuth Broker API version: the wizard always - writes ``3`` (help.keboola.com, "OAuth" in the common interface), so the - check never passes and an agent reports a failure after a success. The - configuration ``version`` goes up when the wizard saves the credentials. -- Its model answer said "Opened ... in your browser", but agents run with - ``--json``, which never opens a browser (``browser_opened: false``). -""" - -from __future__ import annotations - -from pathlib import Path - -SKILL_DIR = Path(__file__).parent.parent / "plugins/kbagent/skills/kbagent" -WORKFLOW = SKILL_DIR / "references/oauth-workflow.md" - - -def _section(heading: str) -> str: - """The body of one ``## `` section of the workflow doc.""" - text = WORKFLOW.read_text(encoding="utf-8") - parts = text.split(f"## {heading}\n", 1) - assert len(parts) == 2, f"section {heading!r} is gone" - return parts[1].split("\n## ", 1)[0] - - -def _verify_step() -> str: - steps = _section("Steps") - parts = steps.split("\n3. ", 1) - assert len(parts) == 2, "the verify step (3.) is gone" - return parts[1] - - -def test_verify_step_uses_the_config_version_and_the_credentials_id() -> None: - verify = _verify_step() - assert "configuration `version`" in verify - assert "oauth_api.id" in verify - - -def test_no_doc_expects_oauth_api_version_to_change() -> None: - assert "-> `4`" not in WORKFLOW.read_text(encoding="utf-8") - skill = (SKILL_DIR / "SKILL.md").read_text(encoding="utf-8") - assert "verify `oauth_api.version`" not in skill - - -def test_model_answer_does_not_claim_an_open() -> None: - reporting = _section("Reporting the link in an answer") - assert "browser_opened" in reporting - assert "Opened the" not in reporting