From c09c85d13a5b7906ccb5c14d80bb067a27a508c5 Mon Sep 17 00:00:00 2001 From: Vojtech <119944107+cvrysanek@users.noreply.github.com> Date: Mon, 11 May 2026 23:54:51 +0400 Subject: [PATCH] fix(cta): clipboard fallback + fold Atlassian MCP into connectors (#249) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cta): fall back to textarea+execCommand when Clipboard API rejects The "Setup a new Claude Code" CTA fetches /auth/tokens, parses the JSON response, renders the setup script, THEN calls `navigator.clipboard.writeText()`. Modern browsers (Safari, Firefox, and Chrome on stricter configurations) reject `writeText` with NotAllowedError when transient user activation has been consumed by an intervening `await` — which is exactly the case here. Users perceived this as "the browser blocked the copy" and got the manual-paste fallback modal even though the textarea + `document.execCommand('copy')` path WOULD have worked synchronously without needing fresh user activation. `copyToClipboard` now: - prefers the modern Clipboard API (unchanged for the happy path) - on writeText rejection, falls back to `copyViaTextarea` instead of surfacing the rejection to the caller's catch block. `copyViaTextarea` is the previously-inline textarea fallback factored out into a named helper, with two small hardening touches: - `readonly` + `tabindex=-1` so the hidden textarea doesn't steal focus or pop the virtual keyboard on mobile. - explicit `setSelectionRange(0, text.length)` to belt-and-braces the selection on iOS Safari (where `.select()` alone sometimes selects zero chars on touch-focused textareas). Only the CTA button needed this — the Step-1 install-command and the connector-copy buttons all call `writeText` synchronously inside the click handler (no awaits in between), so they keep their existing user-gesture context and didn't hit the same rejection. No template changes there. * refactor(home): fold Atlassian MCP registration into connectors block The standalone "Register the Atlassian MCP server" step (was step 6 in the unified setup script) moves INTO the Atlassian connector's prompt body so all Atlassian-related setup lives in one logical group. Same intent that #247 carried for connectors, applied one level deeper: the hosted Remote MCP registration is part of "set up Atlassian", not its own ungrouped step. What changed: - `app/web/connector_prompts.py` — the Atlassian prompt's step 5 replaces the speculative "Register the on-demand Atlassian MCP under .claude/mcp/atlassian" line with the actual hosted Remote MCP registration: `claude mcp add --transport sse atlassian https://mcp.atlassian.com/v1/sse || true`. The `|| true` keeps re-runs idempotent and the body explains the OAuth-on-first-use contract. Both /home's Atlassian tile and the inlined setup-script Atlassian sub-block emit this line — single source of truth holds. - `app/web/setup_instructions.py` — `_mcp_servers_block` deleted; the `mcp_servers` step is removed from `_step_numbers`; resolve_lines no longer calls it. - Renumbering: install (1), init (2), catalog (3), preflight (4), marketplace (5), diagnose (6), connectors (7), confirm (8). Was: 6 = mcp_servers, 7 = diagnose, 8 = connectors, 9 = confirm. - `tests/test_setup_instructions.py` — Confirm step 9→8, Connect 8→7, diagnose 7→6, mcp_servers references dropped. `test_step_numbering_with_connectors_step` now asserts `"mcp_servers" not in steps`. Stray-Confirm assertion lists shift by one position. - `tests/test_setup_page_unified.py` + `tests/test_web_ui.py` — same step-number shifts in the rendered /setup preview assertions. The `claude mcp add` line is still the Atlassian Remote-MCP path that the 2026-05-10 init-report Fix C added — only its position in the flow changes. /home Atlassian tile copying continues to install the MCP too (the prompt body the tile pastes contains the same line). 112 tests pass. * feat(atlassian): operator-overrideable base URL via AGNES_ATLASSIAN_BASE_URL Adds an env var / YAML key the operator (Terraform module, customer-VM template, OSS instance.yaml) can set to bake the Atlassian Cloud site root into the connector prompt — so end users don't have to guess / paste their org's `https://.atlassian.net`. When set, the Atlassian connector prompt (rendered on both /home tile and inlined into the setup-script step 7 Atlassian sub-block) replaces step 1's "Ask me for my Atlassian Cloud site URL and email" with a one-line note that the URL is already provisioned by the operator and asks only for the email. Step 4's helper-script body has the `BASE_URL=''` placeholder substituted with the literal value. When unset (empty), the existing "ask the user" flow remains — no regression for OSS instances. Resolution + normalization in `get_atlassian_base_url()`: - env `AGNES_ATLASSIAN_BASE_URL` > yaml `instance.atlassian.base_url` > "" - strips trailing slash + trailing `/wiki` so the canonical value is the bare site root. Matches the per-user helper script's normalization at storage time (atlassian_prompt step 4 guard 2), so the literal baked in by the operator stays consistent with what the user's helper script would have computed from their input. Plumbing: - `app/instance_config.py`: new `get_atlassian_base_url()` resolver. - `app/web/connector_prompts.py`: - `atlassian_prompt(*, base_url: str = "")` — string-replace two explicit placeholder phrases when base_url is truthy; otherwise return the prompt unchanged. - `all_connector_prompts(..., atlassian_base_url: str = "")` — forwards the kwarg. - `app/web/router.py` (`_build_context`): reads `get_atlassian_base_url()` and passes it through to `all_connector_prompts(...)` so both the /home tile context AND the inlined-script `resolve_lines(...)` call use the same value. - `src/welcome_template.py` (`compute_default_agent_prompt`): same threading via the existing import-on-demand path. Tests (`tests/test_home_route_resolution.py`): - `get_atlassian_base_url` resolver: default empty, env override, trailing-slash strip, trailing-`/wiki` strip. - `atlassian_prompt(base_url=...)`: literal URL baked in, ask-step removed, placeholder replaced, operator-baked-in copy appears. - `atlassian_prompt(base_url="")`: existing ask-the-user flow unchanged. - `all_connector_prompts(atlassian_base_url=...)`: kwarg threads through to the rendered atlassian prompt. 135 tests pass. * feat(asana): register hosted Asana Remote MCP in connector prompt The Asana connector prompt only stored a PAT in the OS keychain + ran a curl verify against /api/1.0/users/me. That set Claude Code up for direct `curl` calls but didn't actually wire Asana into Claude's tool list — so the user couldn't ask Claude to "find my open Asana tasks" and have it work. Symmetric oversight to the Atlassian connector's original speculative `.claude/mcp/atlassian` line that this branch already replaced with `claude mcp add --transport sse atlassian https://mcp.atlassian.com/v1/sse`. Adds a new step 5 that registers Asana's hosted Remote MCP: claude mcp add --transport http asana https://mcp.asana.com/mcp || true This is the V2 endpoint (streamable HTTP transport, launched February 2026). The V1 SSE endpoint at https://mcp.asana.com/sse was deprecated 2026-05-11 (today) and must NOT be used — calling it out explicitly in the prompt body so a future operator who finds an old reference doesn't paste the dead URL. OAuth is handled by Claude Code at first use, same model as the Atlassian MCP step. The PAT stored in step 3 stays for direct `curl` calls (precheck + ad-hoc scripts) — the MCP path uses its own OAuth grant, not the PAT. Old step 5 (revoke instructions) renumbers to step 6 and adds the `claude mcp remove asana` cleanup hint. Same single-source-of-truth invariant holds: /home Asana tile + the inlined Asana sub-block in the setup script (step 7 connectors) both emit identical text from `asana_prompt()`. 71 tests pass. * feat(asana): drive MCP OAuth login + end-to-end validation post-register `claude mcp add --transport http asana ...` only registers the server in Claude Code's local config — it does NOT trigger OAuth. The browser tab opens the first time any `mcp__asana__*` tool gets invoked. So the previous step 5 left a user looking at a "registered" MCP that, in practice, hadn't authed yet and would fail on first real use. Same blind spot Atlassian's prompt also has, but Asana was the one called out in the latest review pass. Adds a new step 6 between MCP registration (step 5) and the revoke instructions (now step 7): a. Tell the user verbatim what's about to happen — a low-impact read through the MCP will pop the OAuth browser tab; sign in with the same account whose PAT they stored in step 3 and approve. Frames the OAuth as one-time so users don't wait for it on every later call. b. Drive an actual MCP read. Don't prescribe the exact tool name because the Asana MCP's exposed surface (`mcp__asana__*`) is versioned upstream and we don't want to pin to a name that gets renamed. Instead: tell Claude to pick the lightest read from its surfaced tool list (users-me / list-workspaces / equivalent). Document the recovery path when Claude Code times out waiting for the OAuth tool use: `claude mcp list` to confirm registration before retrying. c. Print a single one-line proof that combines wiring + auth: "Asana MCP connected as workspace(s) visible." Explicit anti-echo callout for tokens, task content, comments. On failure, surface the exact Claude-Code error and stop — no silent pass. d. Sanity-check that the MCP OAuth identity and the PAT identity reference the same Asana account. Easy mistake to make when the user has multiple Asana accounts — flag only on mismatch, keep quiet when they match. Recovery: `claude mcp remove asana && claude logout asana` then redo step 5. Step 7 (revoke) absorbs both the keychain delete + the `claude logout asana` line so users have a single place to undo everything. 43 tests pass. * fix(init): clear stale CA env vars on Windows before any TLS handshake Reported by the 2026-05-11 Windows test pass: after `agnes init` the gws connector failed with `UnknownIssuer` TLS errors because `SSL_CERT_FILE` and `REQUESTS_CA_BUNDLE` were still set in Windows User scope pointing at `C:\Users\localadmin\.config\agnes\ca-bundle.pem` — a file that did not exist on the test host. Past Agnes installs (the setup-prompt trust block + older bootstrap helpers) write those pointers when they materialize a combined Agnes-CA bundle; when the bundle file later disappears (re-init on a new VM, machine swap, the ~/.agnes dir wiped), the pointers go stale and every native Windows TLS handshake fails before Agnes itself runs. SSL_CERT_FILE in particular REPLACES (not appends to) the trust store, so a stale pointer is silently catastrophic. `agnes init` now clears stale pointers in two layers before the first server roundtrip: 1. Current-process env (os.environ) — what the immediately-following `api_get` to /api/catalog/tables actually reads. Without this, init itself blows up before it gets to step 2. 2. Windows User-scope env via PowerShell `[Environment]::SetEnvironmentVariable(name, $null, 'User')` — what every future shell + every native tool (gws, claude.exe, pip, uv) inherits. The 2026-05-11 reporter expected this exact cleanup ("init was supposed to clear these but they persisted"). The cleanup is best-effort and conservative: - Only deletes a var when its value points at a path that does NOT exist on disk. Intentional operator config (e.g. SSL_CERT_FILE pointing at a corp certifi bundle) stays put. - PowerShell missing / restricted execution policy / WSL-without-pwsh: swallowed silently. The current-process leg still runs, which unblocks init even on hosts where the User-scope leg cannot fire. Tests (`tests/test_init_ca_cleanup.py`, 6 cases): - Stale pointers → removed from process env. - Real-path pointers → preserved. - Non-Windows hosts: PowerShell is not invoked. - Windows hosts: PowerShell IS invoked with a script that checks all three vars + uses Test-Path + SetEnvironmentVariable. - PowerShell FileNotFoundError: cleanup swallows it, does not raise. - `_is_windows_host()` reflects sys.platform. * refactor(asana): MCP-first flow — drop PAT storage, precheck via `claude mcp list` The Asana hosted MCP at https://mcp.asana.com/mcp authenticates via OAuth (Claude Code holds the grant; browser tab pops on first tool use). The earlier prompt walked the user through creating + keychain- storing an Asana Personal Access Token AND registering the MCP — two parallel auth surfaces for one connector. Once the MCP works, the PAT has no consumer: the precheck/verify steps that used `curl $BASE/api/1.0/users/me` are just redundant proof that Asana itself is reachable, which the OAuth handshake already establishes. Removed: - Step 0 keychain probe + curl verify against /users/me with PAT. - Step 1 open developer-console / create PAT. - Step 2 click "+ New access token", warn shown-ONCE. - Step 3 helper-script for keychain-storage (per-OS bodies: macOS `security add-generic-password`, Linux `secret-tool store`, Windows `cmdkey /generic`). - Step 4 PAT-side `users/me` verify. - Step 5's split that kept the PAT around for direct curl scripts. - Step 6d's "MCP vs PAT identity sanity check" — there is no PAT anymore, nothing to mismatch against. New flow (3 steps total): - Step 0 precheck: `claude mcp list | grep ^asana` — if found, the server is registered AND Claude Code is holding its OAuth grant (otherwise prior failure would have removed it); print "Asana MCP already registered — skipping setup" and stop. Tells the user the explicit reset command (`claude mcp remove asana && claude logout asana`) so a re-register stays one paste. - Step 1: `claude mcp add --transport http asana https://mcp.asana.com/mcp` — no `|| true` because step 0 should have caught the "already exists" case. Step explains the V2-vs-V1 endpoint distinction (V1 SSE deprecated 2026-05-11) and the abort-clean recovery if the precheck somehow missed the existing server. - Step 2: same OAuth + low-impact-read validation pattern as before. - Step 3: revoke instructions (mcp remove + logout + Asana-side app revoke at app.asana.com/Settings → Apps). Both surfaces (the /home Asana tile and the inlined Asana sub-block in the setup script's step 7) emit the new text from the same asana_prompt() — single-source-of-truth invariant intact. 77 tests pass. --- app/instance_config.py | 28 +++++ app/web/connector_prompts.py | 87 +++++++++----- app/web/router.py | 4 +- app/web/setup_instructions.py | 66 +++-------- app/web/templates/_claude_setup_cta.jinja | 60 +++++++--- cli/commands/init.py | 115 +++++++++++++++++++ src/welcome_template.py | 2 + tests/test_home_route_resolution.py | 83 ++++++++++++++ tests/test_init_ca_cleanup.py | 131 ++++++++++++++++++++++ tests/test_setup_instructions.py | 55 +++++---- tests/test_setup_page_unified.py | 4 +- tests/test_web_ui.py | 2 +- 12 files changed, 513 insertions(+), 124 deletions(-) create mode 100644 tests/test_init_ca_cleanup.py diff --git a/app/instance_config.py b/app/instance_config.py index 1cef0a8..486ac1b 100644 --- a/app/instance_config.py +++ b/app/instance_config.py @@ -275,6 +275,34 @@ def get_instance_admin_email() -> str: return (raw or "").strip() +def get_atlassian_base_url() -> str: + """Operator-provisioned Atlassian Cloud site URL — baked into the + Atlassian connector prompt so end users don't have to guess / + paste their org's `https://.atlassian.net`. + + When set, the connector prompt's "ask me for the site URL" step + is replaced by a literal value the helper script substitutes + directly. When unset (empty string), the prompt falls back to + asking the user — same flow as today. + + Normalized: trailing slashes and a trailing ``/wiki`` are stripped + so the value is always the bare site root. Matches the + normalization the per-user helper script already does at storage + time (see atlassian_prompt step 4 guard 2). + + Resolution: ``AGNES_ATLASSIAN_BASE_URL`` env > ``instance.atlassian.base_url`` YAML > "". + Mirrors :func:`get_instance_admin_email` so Terraform overrides + work the same way. + """ + raw = os.environ.get("AGNES_ATLASSIAN_BASE_URL") + if raw is None: + raw = get_value("instance", "atlassian", "base_url", default="") + value = (raw or "").strip().rstrip("/") + if value.endswith("/wiki"): + value = value[: -len("/wiki")] + return value + + def get_sync_interval() -> str: """Human-readable refresh cadence shown in the analyst welcome prompt.""" return get_value("instance", "sync_interval", default="1 hour") diff --git a/app/web/connector_prompts.py b/app/web/connector_prompts.py index 1149460..cf1f56c 100644 --- a/app/web/connector_prompts.py +++ b/app/web/connector_prompts.py @@ -67,12 +67,14 @@ def all_connector_prompts( *, gws_oauth: dict | None = None, instance_admin_email: str = "", + atlassian_base_url: str = "", ) -> dict[str, str]: """Resolve every connector's prompt text with the operator's runtime config baked in. Caller (router._build_context, setup_instructions consumers) passes the already-resolved ``gws_oauth`` dict from - :func:`app.instance_config.get_gws_oauth_credentials` and the admin - email from :func:`get_instance_admin_email`. Returns a dict keyed by + :func:`app.instance_config.get_gws_oauth_credentials`, the admin + email from :func:`get_instance_admin_email`, and the Atlassian site + URL from :func:`get_atlassian_base_url`. Returns a dict keyed by connector slug so both the template (``{{ connector_prompts.asana }}``) and the setup script (``connector_prompts['asana']``) read the same shape. @@ -82,6 +84,11 @@ def all_connector_prompts( but is plumbed through anyway so a future GWS prompt branch that references the admin contact can add the string without changing the call sites. + + ``atlassian_base_url`` (when non-empty) bakes the operator-provisioned + Atlassian Cloud site root into the Atlassian connector prompt — the + user no longer has to guess / paste their org's URL. Empty value + falls back to the existing "ask the user" flow. """ gws_oauth = gws_oauth or {} return { @@ -96,7 +103,7 @@ def all_connector_prompts( ), instance_admin_email=instance_admin_email, ), - "atlassian": atlassian_prompt(), + "atlassian": atlassian_prompt(base_url=atlassian_base_url), } @@ -110,9 +117,21 @@ def all_connector_prompts( # --------------------------------------------------------------------------- def asana_prompt() -> str: - """Asana PAT setup. Stores token in OS keychain under - ``agnes-asana-pat``. Idempotent — re-running short-circuits when the - cached token still passes the Asana ``users/me`` probe.""" + """Asana MCP setup — registers Asana's hosted Remote MCP (V2 + streamable HTTP at https://mcp.asana.com/mcp) so Claude Code can + read tasks, comment, and create updates on demand. + + No PAT storage. The hosted Asana MCP handles auth via OAuth (Claude + Code opens a browser tab on first tool use; the user signs in once + with their Asana account; subsequent calls reuse the grant). + Earlier versions of this prompt walked the user through creating + + keychain-storing an Asana Personal Access Token, but the MCP path + has its own OAuth grant — the PAT had no consumer once the MCP + became the actual integration surface, so it's gone. + + Precheck short-circuits when `claude mcp list` already shows the + `asana` server registered — re-running setup on a connected machine + is a no-op.""" return _ASANA_PROMPT @@ -156,11 +175,34 @@ def gws_prompt( ) -def atlassian_prompt() -> str: +def atlassian_prompt(*, base_url: str = "") -> str: """Atlassian (Jira + Confluence) API token setup. Stores token in OS keychain under ``agnes-atlassian-api-token``, plus email + normalized base URL in ``~/.claude/agnes/secrets.env``. Jira-first / Confluence- - fallback verify so Confluence-only sites still onboard.""" + fallback verify so Confluence-only sites still onboard. + + ``base_url`` (when non-empty) is the operator-provisioned Atlassian + Cloud site root — typically ``https://.atlassian.net``. When + set, step 1's "ask me for the site URL" prompt is replaced by a + one-line note that the URL is already baked in, and step 4's + ``BASE_URL=''`` literal becomes + ``BASE_URL=''`` directly. Empty value (default) keeps the + existing flow — Claude asks the user. + + Caller is expected to normalize: strip trailing slash + ``/wiki`` + (handled by :func:`app.instance_config.get_atlassian_base_url`).""" + if base_url: + # The {{ }} pair in the f-string escapes a literal `{` — the + # `${ATLASSIAN_BASE_URL%/}` shell parameter expansion in step 0 + # precheck uses `${...}` which f-strings DON'T touch, so no + # additional escaping is needed there. Replace only the two + # explicit placeholder strings. + return _ATLASSIAN_PROMPT \ + .replace("", base_url) \ + .replace( + "1. Ask me for my Atlassian Cloud site URL (looks like https://.atlassian.net) and the email I sign in with. Site URL and email are NOT secrets — fine to type into chat. Don't proceed until I've given you both.", + f"1. Ask me only for the email I sign in with — the Atlassian Cloud site URL is already provisioned by the Agnes operator (``{base_url}``) and baked into the helper script. Email is not a secret — fine to type into chat. Don't proceed until I've given you the email.", + ) return _ATLASSIAN_PROMPT @@ -169,26 +211,17 @@ def atlassian_prompt() -> str: # any per-call allocation cost and trivially diffable. # --------------------------------------------------------------------------- -_ASANA_PROMPT = """Set up an Asana personal access token for Claude Code. Walk me through it step by step. +_ASANA_PROMPT = """Set up Asana access for Claude Code via Asana's hosted Remote MCP. Walk me through it step by step. -Ground rules: this is idempotent — safe to re-run, the precheck below short-circuits when Asana is already wired up. If any step fails with an unfamiliar error, paste the exact error back and stop. Do NOT improvise around TLS errors by disabling verification (`-k`, `NODE_TLS_REJECT_UNAUTHORIZED=0`, `git -c http.sslVerify=false`, etc.) — those hide the real problem. +Ground rules: this is idempotent — safe to re-run, the precheck below short-circuits when the `asana` MCP server is already registered. No Personal Access Token is created or stored; Asana's hosted MCP handles auth via OAuth, with Claude Code holding the grant. If any step fails with an unfamiliar error, paste the exact error back and stop. Do NOT improvise around TLS errors by disabling verification (`-k`, `NODE_TLS_REJECT_UNAUTHORIZED=0`, `git -c http.sslVerify=false`, etc.) — those hide the real problem. -0. Precheck — skip the rest if Asana is already connected. Detect my OS, then look up an existing keychain entry under the service name `agnes-asana-pat` and verify it against Asana's API. macOS: `t=$(security find-generic-password -s 'agnes-asana-pat' -w 2>/dev/null) && curl -fsS -H "Authorization: Bearer $t" https://app.asana.com/api/1.0/users/me | jq -r '.data | "Already connected as \\(.name) (\\(.workspaces | length) workspace(s)). Skipping setup."' && exit 0`. Linux: `t=$(secret-tool lookup service agnes-asana-pat username "$USER" 2>/dev/null) && ...same curl...`. Windows PowerShell: `$cred = cmdkey /list:agnes-asana-pat 2>$null; if ($LASTEXITCODE -eq 0) { Write-Host "Asana cred entry found — verify in your terminal before re-running setup." }` (Windows can't read the password back without a CredentialManager module — print a hint and let me confirm). If the verify call returns 200, print the one-line "Already connected" message and STOP. Only continue to step 1 when no cred exists OR the cached token returns 401. -1. Open the Asana developer tokens page in my default browser — use your Bash tool: `open https://app.asana.com/0/developer-console/tokens` on macOS, `xdg-open https://app.asana.com/0/developer-console/tokens` on Linux/WSL, or `Start-Process https://app.asana.com/0/developer-console/tokens` on Windows. Detect OS first. If that URL doesn't render the tokens UI (rare), tell me to click my avatar (top right) → Settings → "Apps" tab → "Manage Developer Apps" → Personal access tokens. -2. Tell me to click "+ New access token", name it "Claude Code — Agnes", and click "Create token". Warn me the token is shown ONCE and Asana PATs do not expire — I'd need to revoke it from the same page if it leaks. -3. Important: do NOT ask me to paste the token into the chat. Chat input is saved to ~/.claude/projects/.../*.jsonl. Instead, prepare a tiny helper script for me to run in my real terminal: - a. Detect my OS. Use the Write/Edit tool (NOT a shell here-doc that prints the body) to create ~/.claude/agnes/bin/store-asana.sh on macOS/Linux, or ~/.claude/agnes/bin/store-asana.ps1 on Windows. chmod 700 the file. Body for macOS: - #!/usr/bin/env bash - set -e - read -srp 'Paste Asana token (hidden): ' t; echo - security add-generic-password -U -s 'agnes-asana-pat' -a "$USER" -w "$t" - unset t - echo 'Stored in macOS Keychain.' - Linux variant: same shape but `printf %s "$t" | secret-tool store --label='Agnes Asana PAT' service agnes-asana-pat username "$USER"`. Windows .ps1: `$t = Read-Host 'Paste Asana token' -AsSecureString; $p = [Runtime.InteropServices.Marshal]::PtrToStringAuto([Runtime.InteropServices.Marshal]::SecureStringToBSTR($t)); cmdkey /generic:agnes-asana-pat /user:$env:USERNAME /pass:$p > $null; Remove-Variable p,t; 'Stored.'` - b. Tell me to open a real terminal (Terminal.app / iTerm / WSL / PowerShell — NOT Claude Code's `!` prefix, which has no TTY) and run `bash ~/.claude/agnes/bin/store-asana.sh` (or `pwsh ~/.claude/agnes/bin/store-asana.ps1` on Windows). The script will wait silently at the hidden prompt. - c. Walk me through the clipboard order: copy the launcher first, paste it in my terminal, press Enter (terminal now waiting). Switch to the Asana tab, copy the token from step 2. Switch back to terminal, paste at the silent prompt, press Enter. Token enters via stdin only — not shown on screen, not in shell history, not in clipboard at the moment Claude is involved. -4. After I report "Stored", verify by calling `curl -sS -H "Authorization: Bearer $(security find-generic-password -s 'agnes-asana-pat' -w)" https://app.asana.com/api/1.0/users/me | jq -r '.data | "\\(.name) — \\(.workspaces | length) workspace(s)"'` (macOS; Linux uses `secret-tool lookup` instead). Print only the one-line result. Never echo the token. -5. Remind me where the token is stored and how to revoke: in macOS Keychain Access search "agnes-asana-pat" or run `security delete-generic-password -s 'agnes-asana-pat'`; on Asana, revoke from the same developer-console page.""" +0. Precheck — skip the rest if Asana is already wired up. Run `claude mcp list` and grep for a line starting with `asana` (the server name we register in step 1). If it's there, the MCP is registered AND Claude Code is holding its OAuth grant (otherwise the server would have been removed by a previous failure). Print "Asana MCP already registered — skipping setup. To force re-register: `claude mcp remove asana && claude logout asana`, then re-run." and STOP. Continue to step 1 only when `claude mcp list` shows NO `asana` row. +1. Register Asana's hosted Remote MCP: `claude mcp add --transport http asana https://mcp.asana.com/mcp`. This is Asana's V2 MCP (streamable HTTP, launched February 2026); the V1 SSE endpoint at `https://mcp.asana.com/sse` was deprecated 2026-05-11 and must not be used. If `claude mcp add` errors with "server already exists", the precheck in step 0 missed it — abort cleanly with `claude mcp remove asana && claude mcp add --transport http asana https://mcp.asana.com/mcp` to force a clean state. No PAT is required: Asana's hosted MCP authenticates via OAuth on first tool use. +2. Log the user in through the Asana MCP, then validate end-to-end before declaring success. Claude Code's MCP OAuth opens a browser tab the FIRST time any tool from the `asana` MCP is invoked, not when the server is added — so the registration in step 1 alone proves nothing. Drive a real verification: + a. Tell the user verbatim: "I'm going to make a low-impact read through the Asana MCP. Your browser will open an Asana sign-in page — sign in with your Asana account and approve the consent screen. The approval is one-time; subsequent MCP calls reuse the grant." + b. Invoke the lightest read the Asana MCP exposes (typically a "list workspaces" / "users me" tool — call whatever shows up under the `mcp__asana__*` prefix in your tool list; pick the one that returns the caller's profile or a small list). Wait for the OAuth tab to come back. If Claude Code times out waiting for tool use, run `claude mcp list` to confirm the server registered, then retry the same MCP call. + c. On success, print ONE line that proves both the wiring AND the auth work: "Asana MCP connected as <display name from the MCP response> — <workspace count> workspace(s) visible." Never echo the OAuth token or any task / comment content. On failure, surface the exact error Claude Code returned (NotAuthenticated, NotFound, network) and stop — do not silently move on. +3. Remind me how to disconnect later: `claude mcp remove asana` removes the server from Claude Code's config; `claude logout asana` drops the OAuth grant. To revoke the grant on Asana's side, sign in at https://app.asana.com and revoke the "Claude Code" entry from Settings → Apps.""" _GWS_PROMPT_HEAD = """Set up Google Workspace access for Claude Code using the official `gws` CLI from https://github.com/googleworkspace/cli (install steps: README → Installation). The npm path is what we'll use because (a) it's the README's documented convenience path, (b) it works the same on macOS / Linux / WSL / Windows, and (c) it can run with zero admin rights when Node is managed by `nvm` (Unix) or `fnm` (Windows). @@ -346,6 +379,6 @@ Ground rules: this is idempotent — safe to re-run, the precheck below short-ci Linux variant: replace `security add-generic-password ...` with `printf %s "$t" | secret-tool store --label='Agnes Atlassian token' service agnes-atlassian-api-token username "$EMAIL"`. All three guards (length floor, URL normalization, Jira-then-Confluence verification) stay identical — they run before the storage call. Windows .ps1: same control flow — `Read-Host -AsSecureString`, convert via `Marshal::PtrToStringAuto`, check `$t.Length -lt 100`, then `$BASE_URL = $BASE_URL.TrimEnd('/').TrimEnd('/wiki')` (or `if ($BASE_URL.EndsWith('/wiki')) { $BASE_URL = $BASE_URL.Substring(0, $BASE_URL.Length - 5) }`), `try { Invoke-RestMethod -Uri "$BASE_URL/rest/api/3/myself" -Authentication Basic -Credential (New-Object PSCredential($EMAIL, $secureToken)) } catch { if ($_.Exception.Response.StatusCode.value__ -eq 404) { Invoke-RestMethod -Uri "$BASE_URL/wiki/rest/api/user/current" -Authentication Basic -Credential ... } else { throw } }` — write to `cmdkey` + `secrets.env` only after a 200 lands from either probe. b. Tell me to open a real terminal (not Claude Code's `!`) and run `bash ~/.claude/agnes/bin/store-atlassian.sh` (or `pwsh ~/.claude/agnes/bin/store-atlassian.ps1` on Windows). The script will wait silently at the hidden prompt. c. Walk me through clipboard order: copy the launcher first, paste in terminal, Enter (terminal waiting). Switch to the Atlassian tab, copy the token from step 3 — use the panel's "Copy" button, NOT click-and-drag (which often truncates). Switch back to terminal, paste at the silent prompt, Enter. The script will print "Stored. Verified as ." on success, or fail loudly with the exact reason (too short / HTTP 401 / etc.) without writing anything. -5. Register the on-demand Atlassian MCP under .claude/mcp/atlassian referencing the stored credentials (read token from keychain via `security find-generic-password -s 'agnes-atlassian-api-token' -w` at MCP startup). +5. Register the hosted Atlassian Remote MCP so Claude Code can read Jira tickets and Confluence pages on demand: `claude mcp add --transport sse atlassian https://mcp.atlassian.com/v1/sse || true`. Idempotent — the `|| true` swallows the "server already exists" error from re-runs. OAuth is handled by Claude Code the first time it actually queries the MCP (it'll open a browser tab; approve once). The PAT stored in step 4 stays for direct `curl` calls (e.g. the precheck) — the MCP path uses its own OAuth grant, not the PAT. 6. The store script already verified the token end-to-end. If I want a second redacted readback later, hit `GET $BASE_URL/rest/api/3/myself` (Jira) or `GET $BASE_URL/wiki/rest/api/user/current` (Confluence) — try Jira first, fall back to Confluence on 404, same shape as the store script's verify. Print just displayName + accountId — never the token. 7. Remind me how to revoke: same API tokens page on Atlassian, plus `security delete-generic-password -s 'agnes-atlassian-api-token'` locally (macOS) / `secret-tool clear service agnes-atlassian-api-token` (Linux) / `cmdkey /delete:agnes-atlassian-api-token` (Windows).""" diff --git a/app/web/router.py b/app/web/router.py index c8c3152..42479e2 100644 --- a/app/web/router.py +++ b/app/web/router.py @@ -23,7 +23,7 @@ from app.instance_config import ( get_instance_name, get_instance_subtitle, get_datasets, get_theme, get_corporate_memory_config, get_home_route, get_gws_oauth_credentials, get_home_automode_visibility, - get_instance_admin_email, + get_instance_admin_email, get_atlassian_base_url, ) from app.web.connector_prompts import all_connector_prompts from src.repositories.sync_state import SyncStateRepository @@ -412,6 +412,7 @@ def _build_context( _connector_prompts = all_connector_prompts( gws_oauth=get_gws_oauth_credentials(), instance_admin_email=get_instance_admin_email(), + atlassian_base_url=get_atlassian_base_url(), ) setup_instructions_lines = resolve_lines( @@ -456,6 +457,7 @@ def _build_context( "connector_prompts": all_connector_prompts( gws_oauth=get_gws_oauth_credentials(), instance_admin_email=get_instance_admin_email(), + atlassian_base_url=get_atlassian_base_url(), ), # Whether /home renders the "Step 3 — turn on auto-accept mode" # install-block. Operator can hide it via AGNES_HOME_SHOW_AUTOMODE=0 diff --git a/app/web/setup_instructions.py b/app/web/setup_instructions.py index 9fa9477..1c8ed62 100644 --- a/app/web/setup_instructions.py +++ b/app/web/setup_instructions.py @@ -650,44 +650,6 @@ def _marketplace_block( ] -def _mcp_servers_block(step_num: str) -> list[str]: - """Register the Atlassian Remote MCP unattended. - - Why only Atlassian here: - - Atlassian publishes a hosted SSE MCP at - https://mcp.atlassian.com/v1/sse with OAuth handled by Claude Code - automatically on first tool call. No PAT/keychain dance, no - per-user setup beyond clicking through OAuth once when an - operator first asks Claude to read a Jira ticket. Safe to - register unattended in the bootstrap script. - - Asana and Google Workspace need PAT/keychain flows that don't - survive non-interactive bootstrap, so those stay on the /home - connector cards (operator-driven). - - Idempotent across re-runs: `claude mcp add` returns non-zero when the - server name already exists, so we soft-fail with `|| true` and a - one-line note rather than tripping the `set -e` style operators - sometimes wrap the prompt in. Subsequent runs of the prompt are - no-ops. - - Reference: 2026-05-10 init-report — David's `claude mcp list` showed - only the pre-existing claude.ai Drive connector; Atlassian/Asana/GWS - weren't registered because the prompt had zero `claude mcp add` - lines. Fix C in the response plan. - """ - return [ - "", - f"{step_num}) Register the Atlassian MCP server (Jira + Confluence on demand):", - " # Hosted Remote MCP — Claude Code handles OAuth automatically the", - " # first time you ask it to read a Jira ticket or Confluence page.", - " # Idempotent: re-runs are a no-op (the `|| true` swallows the", - " # \"server already exists\" error from `claude mcp add`).", - " claude mcp add --transport sse atlassian https://mcp.atlassian.com/v1/sse || true", - "", - " Asana and Google Workspace use per-user PAT / CLI flows that don't", - " fit an unattended bootstrap — connect those from the /home connector", - " cards after this script finishes.", - ] def _preamble_lines(*, has_ca: bool) -> list[str]: @@ -728,19 +690,21 @@ def _step_numbers(*, has_connectors: bool = True) -> dict[str, str]: so call sites stay diff-minimal). Steps (always emitted): install (1), init (2), catalog (3), - preflight (4), marketplace (5), mcp_servers (6), diagnose (7), - connectors (8), confirm (9). Preflight + marketplace + mcp_servers - + connectors are all always-on: + preflight (4), marketplace (5), diagnose (6), connectors (7), + confirm (8). Preflight + marketplace + connectors are always-on: - Marketplace registration is useful even when the operator has zero plugin grants (SessionStart hook reconciles future grants automatically). - - Atlassian MCP registration is unattended-safe (hosted Remote MCP - with Claude Code-managed OAuth) and applies to every analyst - whose work touches Jira/Confluence — high enough hit rate to - justify default-on. - Connectors (Asana / GWS / Atlassian) are per-connector default-yes asks — the user can decline each individually, so always-emitting - the block costs nothing for users who skip everything. + the block costs nothing for users who skip everything. The + Atlassian Remote MCP registration (`claude mcp add ...`) used to + be its own step 6 but moved INTO the Atlassian connector's + prompt body (atlassian_prompt() in app.web.connector_prompts) so + everything Atlassian-related lives in one group — both the + /home Atlassian tile and the inlined Atlassian sub-block in the + setup script emit the same `claude mcp add` line as part of the + standard connector setup. The interactive "Skills" step that previously sat between diagnose and Confirm was deleted in #242 — on-demand `agnes skills show @@ -750,7 +714,7 @@ def _step_numbers(*, has_connectors: bool = True) -> dict[str, str]: `has_connectors` is kept as a parameter for future flexibility (default True). When set False, the connectors step is skipped and - Confirm shifts down to step 8. + Confirm shifts down to step 7. Step-0 (TLS trust block) sits outside this numbering — it is gated by has_ca and has its own "0)" header rendered inside the trust block @@ -759,7 +723,6 @@ def _step_numbers(*, has_connectors: bool = True) -> dict[str, str]: n = 4 preflight = str(n); n += 1 marketplace = str(n); n += 1 - mcp_servers = str(n); n += 1 diagnose = str(n); n += 1 connectors = str(n) if has_connectors else "" if has_connectors: @@ -768,7 +731,6 @@ def _step_numbers(*, has_connectors: bool = True) -> dict[str, str]: return { "preflight": preflight, "marketplace": marketplace, - "mcp_servers": mcp_servers, "diagnose": diagnose, "connectors": connectors, "confirm": confirm, @@ -836,8 +798,10 @@ def resolve_lines( lines.extend(_init_lines()) # 2, 3 lines.extend(_preflight_block(steps["preflight"])) # 4 lines.extend(_marketplace_block(names, step_num=steps["marketplace"])) # 5 - lines.extend(_mcp_servers_block(steps["mcp_servers"])) # 6 - lines.extend(_diagnose_lines(diagnose_num=steps["diagnose"])) # 7 + # Standalone MCP step used to live here — moved INTO the Atlassian + # connector's prompt body in step 7 so all Atlassian setup is grouped + # together. + lines.extend(_diagnose_lines(diagnose_num=steps["diagnose"])) # 6 # Connectors are the LAST interactive ask before Confirm. Per-connector # default-yes — empty/Enter is install, explicit "no" skips. lines.extend(_connectors_block( diff --git a/app/web/templates/_claude_setup_cta.jinja b/app/web/templates/_claude_setup_cta.jinja index 9bc1533..94a57df 100644 --- a/app/web/templates/_claude_setup_cta.jinja +++ b/app/web/templates/_claude_setup_cta.jinja @@ -103,24 +103,56 @@ {% include "_claude_setup_instructions.jinja" %} - function copyToClipboard(text) { - if (navigator.clipboard && window.isSecureContext) { - return navigator.clipboard.writeText(text); - } - var ta = document.createElement('textarea'); - ta.value = text; - ta.style.position = 'fixed'; - ta.style.left = '-9999px'; - ta.style.top = '-9999px'; - document.body.appendChild(ta); - ta.focus(); - ta.select(); + // Synchronous textarea + execCommand('copy'). The fallback path the + // Clipboard API replaced — still functional in all major browsers and, + // critically, NOT subject to the Clipboard API's transient-activation + // window. We use it whenever (a) the modern Clipboard API is + // unavailable / non-secure context, OR (b) it rejected at write time + // (NotAllowedError — Safari/Firefox enforce this when user-gesture is + // consumed across an `await fetch()`, which is exactly what + // setupNewClaude does between the button click and the copy call). + function copyViaTextarea(text) { return new Promise(function (resolve, reject) { - document.execCommand('copy') ? resolve() : reject(); - document.body.removeChild(ta); + var ta = document.createElement('textarea'); + ta.value = text; + ta.style.position = 'fixed'; + ta.style.left = '-9999px'; + ta.style.top = '-9999px'; + // readOnly + tabindex=-1 stops the keyboard focus stealing and + // virtual-keyboard popups on mobile. + ta.setAttribute('readonly', ''); + ta.setAttribute('tabindex', '-1'); + document.body.appendChild(ta); + try { + ta.focus(); + ta.select(); + ta.setSelectionRange(0, text.length); + var ok = false; + try { ok = document.execCommand('copy'); } catch (_e) { ok = false; } + if (ok) { resolve(); } else { reject(new Error('execCommand copy returned false')); } + } finally { + document.body.removeChild(ta); + } }); } + function copyToClipboard(text) { + // Prefer the modern Clipboard API but fall back to the textarea + // path on rejection — NOT just when the API is missing. Recent + // browsers reject `writeText` with NotAllowedError when transient + // user activation has been consumed by an intervening `await` + // (the /auth/tokens fetch in setupNewClaude does exactly that). + // Without this fallback the catch in setupNewClaude pops the + // manual-paste modal, which users perceive as "browser blocked + // the copy" — when actually the textarea path would have worked. + if (navigator.clipboard && window.isSecureContext) { + return navigator.clipboard.writeText(text).catch(function (_err) { + return copyViaTextarea(text); + }); + } + return copyViaTextarea(text); + } + function defaultTokenName() { var stamp = new Date().toISOString().slice(0, 16).replace("T", " "); return "Claude Code — " + stamp; diff --git a/cli/commands/init.py b/cli/commands/init.py index 66ec2a9..5d835eb 100644 --- a/cli/commands/init.py +++ b/cli/commands/init.py @@ -37,6 +37,9 @@ Task 18 will register `init_app` on the root Typer app. from __future__ import annotations import json +import os +import subprocess +import sys from datetime import datetime, timezone from pathlib import Path from typing import Optional @@ -58,6 +61,109 @@ from cli.lib.pull import PullResult, _override_server_env, run_pull _INIT_MARKER = "AI Data Analyst" +# Env vars that, when set to a non-existent path, cause every TLS handshake +# on the host to fail before Agnes itself runs. Past versions of the Agnes +# setup script's TLS trust block (and older bootstrap helpers) wrote +# pointers to ``~/.agnes/ca-bundle.pem`` into the user's persistent env +# (Windows User scope; shell rc files on POSIX). When the file goes away +# (re-init on a new VM, manual cleanup, machine swap) the pointers go +# stale — gws auth login, claude plugin marketplace add, even pip/uv, +# all fail with UnknownIssuer / FileNotFoundError. Reported by the +# Windows test user 2026-05-11. SSL_CERT_FILE in particular REPLACES +# (not appends to) the trust store, so a stale pointer is silently +# catastrophic. +_CA_ENV_VARS = ("SSL_CERT_FILE", "REQUESTS_CA_BUNDLE", "GIT_SSL_CAINFO") + + +def _is_windows_host() -> bool: + """True when the Python interpreter sees Windows underneath. + + Covers native Python on Windows (``sys.platform == 'win32'``) and + Git Bash / MSYS launchers (interpreter still reports win32; the + bash shell wrapper is irrelevant for User-scope env-var management). + POSIX-only edge cases (WSL with `windows` in /proc/version) stay on + the POSIX path — User-scope env vars don't exist there in the + Windows-registry sense, so the cleanup is a no-op. + """ + return sys.platform == "win32" + + +def _cleanup_stale_ca_env_vars() -> None: + """Clear stale SSL_CERT_FILE / REQUESTS_CA_BUNDLE / GIT_SSL_CAINFO + pointers from the current process AND (on Windows) from User scope. + + Two layers because the failure mode hits both: + 1. Current-process env — what the upcoming `api_get` call to + /api/catalog/tables actually reads. Without clearing it here, the + httpx call falls over with a FileNotFoundError before init can + finish step 2. + 2. Windows User-scope env — what every future shell + every native + Windows tool (gws, claude.exe, pip, uv) inherits. Without + clearing it there, the user re-hits the same wall the next time + they open PowerShell — exactly what the 2026-05-11 Windows test + user reported ("the init was supposed to clear these but they + persisted; fixed by removing both vars from User scope"). + + Best-effort. We only delete a var when it points at a path that does + NOT exist on disk — intentional operator config (e.g. SSL_CERT_FILE + pointing at a corporate certifi bundle) is preserved. PowerShell + invocation failures are swallowed silently because the init shouldn't + abort on a defensive cleanup helper. + """ + cleared_process: list[tuple[str, str]] = [] + for var in _CA_ENV_VARS: + cur = os.environ.get(var) + if cur and not Path(cur).exists(): + del os.environ[var] + cleared_process.append((var, cur)) + for name, path in cleared_process: + typer.echo( + f"agnes init: cleared stale process env {name}={path} " + f"(file does not exist)" + ) + + if not _is_windows_host(): + return + + # Build a single PowerShell invocation that checks + clears all three + # User-scope vars in one shot. Quoting strategy: pass the script via + # -Command with single-quoted strings inside so Python's f-string + # composition stays simple. We use [Environment]::SetEnvironmentVariable + # with $null (the documented way to delete a User-scope env var on + # Windows; setx has no delete verb). + statements = [] + for var in _CA_ENV_VARS: + statements.append( + "$cur = [Environment]::GetEnvironmentVariable('" + var + "', 'User'); " + "if ($cur -and -not (Test-Path -LiteralPath $cur)) { " + "[Environment]::SetEnvironmentVariable('" + var + "', $null, 'User'); " + "Write-Host ('agnes init: cleared stale User-scope " + var + "=' + $cur + ' (file does not exist)') " + "}" + ) + ps_script = "; ".join(statements) + try: + result = subprocess.run( + ["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", ps_script], + capture_output=True, + text=True, + timeout=15, + ) + except (FileNotFoundError, subprocess.TimeoutExpired, OSError): + # PowerShell missing (cygwin-only environments), or hung. Skip — + # the current-process cleanup above already covers the immediate + # `api_get` failure; persistent state cleanup is best-effort. + return + if result.stdout: + # Forward PowerShell's confirmation lines to the user so the + # cleanup is auditable. stderr from PowerShell (rare here) is + # swallowed — the worst it'd add is "execution policy" noise on + # restricted hosts, which isn't actionable. + for line in result.stdout.splitlines(): + line = line.strip() + if line: + typer.echo(line) + + init_app = typer.Typer(help="Bootstrap an analyst workspace in this directory") @@ -82,6 +188,15 @@ def init( workspace = Path(workspace_str).resolve() if workspace_str else Path.cwd() server_url = server_url.rstrip("/") + # Best-effort cleanup before ANY TLS handshake fires below — stale + # SSL_CERT_FILE / REQUESTS_CA_BUNDLE / GIT_SSL_CAINFO pointers from a + # previous Agnes install on this host (or its Windows User-scope + # registry entries) would otherwise blow up step 2's `api_get` with + # an opaque "UnknownIssuer" / "FileNotFoundError" before the user + # has any way to see what's wrong. Reported by the 2026-05-11 + # Windows test pass. + _cleanup_stale_ca_env_vars() + # ------------------------------------------------------------------ # Step 1: detect an existing workspace. # ------------------------------------------------------------------ diff --git a/src/welcome_template.py b/src/welcome_template.py index ef0e5a3..1b59e65 100644 --- a/src/welcome_template.py +++ b/src/welcome_template.py @@ -206,9 +206,11 @@ def compute_default_agent_prompt( from app.instance_config import ( get_gws_oauth_credentials, get_instance_admin_email, ) + from app.instance_config import get_atlassian_base_url connector_prompts = all_connector_prompts( gws_oauth=get_gws_oauth_credentials(), instance_admin_email=get_instance_admin_email(), + atlassian_base_url=get_atlassian_base_url(), ) except Exception: logger.exception("compute_default_agent_prompt: connector prompt resolution failed; using module defaults") diff --git a/tests/test_home_route_resolution.py b/tests/test_home_route_resolution.py index 2f8f32f..8976034 100644 --- a/tests/test_home_route_resolution.py +++ b/tests/test_home_route_resolution.py @@ -324,3 +324,86 @@ def test_navbar_home_link_uses_home_route(fresh_db, monkeypatch): # Navbar link href reflects the resolved home_route, not hard-coded /dashboard. # Label is "Home" (was "Dashboard" before the nav reorg). assert 'href="/home">Home' in resp.text + + +# --------------------------------------------------------------------------- +# Atlassian base URL — operator-provisioned site root, Terraform-overrideable +# via AGNES_ATLASSIAN_BASE_URL. +# --------------------------------------------------------------------------- + +def test_atlassian_base_url_default_empty(fresh_db, monkeypatch): + """Unset env + unset YAML → empty string. Connector prompt falls + back to asking the user for the site URL (the existing flow).""" + monkeypatch.delenv("AGNES_ATLASSIAN_BASE_URL", raising=False) + from app.instance_config import get_atlassian_base_url + assert get_atlassian_base_url() == "" + + +def test_atlassian_base_url_env_overrides(fresh_db, monkeypatch): + """Env var takes precedence over YAML / default.""" + monkeypatch.setenv("AGNES_ATLASSIAN_BASE_URL", "https://acme.atlassian.net") + from app.instance_config import get_atlassian_base_url + assert get_atlassian_base_url() == "https://acme.atlassian.net" + + +def test_atlassian_base_url_strips_trailing_slash(fresh_db, monkeypatch): + """`https://acme.atlassian.net/` → `https://acme.atlassian.net`. + Matches the per-user helper script's normalization at storage time + (atlassian_prompt step 4 guard 2). Without this, $BASE_URL/rest/... + becomes $BASE_URL//rest/... which some CDN paths reject.""" + monkeypatch.setenv("AGNES_ATLASSIAN_BASE_URL", "https://acme.atlassian.net/") + from app.instance_config import get_atlassian_base_url + assert get_atlassian_base_url() == "https://acme.atlassian.net" + + +def test_atlassian_base_url_strips_trailing_wiki(fresh_db, monkeypatch): + """`https://acme.atlassian.net/wiki` (the Confluence path) → + `https://acme.atlassian.net` (bare site root). The connector + prompt's verify step probes both Jira (root) and Confluence + (root + /wiki), so the canonical stored value is the root.""" + monkeypatch.setenv("AGNES_ATLASSIAN_BASE_URL", "https://acme.atlassian.net/wiki") + from app.instance_config import get_atlassian_base_url + assert get_atlassian_base_url() == "https://acme.atlassian.net" + + monkeypatch.setenv("AGNES_ATLASSIAN_BASE_URL", "https://acme.atlassian.net/wiki/") + assert get_atlassian_base_url() == "https://acme.atlassian.net" + + +def test_atlassian_prompt_uses_base_url_when_set(): + """The atlassian connector prompt bakes the operator's base URL into + the helper script instead of asking the user. Saves a chat round- + trip and avoids the "guess your org's Atlassian URL" footgun.""" + from app.web.connector_prompts import atlassian_prompt + + p = atlassian_prompt(base_url="https://acme.atlassian.net") + # The literal URL is baked into the prompt body. + assert "https://acme.atlassian.net" in p + # The "ask me for the site URL" step disappears. + assert "Ask me for my Atlassian Cloud site URL" not in p + # The placeholder in step 4's helper-script body is replaced with the literal. + assert "" not in p + # The new "operator baked it in" wording appears in step 1. + assert "already provisioned by the Agnes operator" in p + + +def test_atlassian_prompt_asks_user_when_base_url_empty(): + """When no operator override is set, prompt falls back to the + existing "ask me for the site URL" flow — no regression for OSS + instances that don't set the env var.""" + from app.web.connector_prompts import atlassian_prompt + + p = atlassian_prompt(base_url="") + assert "Ask me for my Atlassian Cloud site URL" in p + assert "" in p + assert "already provisioned by the Agnes operator" not in p + + +def test_all_connector_prompts_threads_atlassian_base_url(): + """all_connector_prompts() must forward the atlassian_base_url + kwarg to atlassian_prompt — otherwise the operator's Terraform + override never reaches the rendered text.""" + from app.web.connector_prompts import all_connector_prompts + + out = all_connector_prompts(atlassian_base_url="https://acme.atlassian.net") + assert "https://acme.atlassian.net" in out["atlassian"] + assert "Ask me for my Atlassian Cloud site URL" not in out["atlassian"] diff --git a/tests/test_init_ca_cleanup.py b/tests/test_init_ca_cleanup.py new file mode 100644 index 0000000..06d524a --- /dev/null +++ b/tests/test_init_ca_cleanup.py @@ -0,0 +1,131 @@ +"""Windows User-scope CA env-var cleanup at the top of `agnes init`. + +Past Agnes installs that wrote `SSL_CERT_FILE` / `REQUESTS_CA_BUNDLE` / +`GIT_SSL_CAINFO` to Windows User-scope env (via the setup-prompt trust +block or an older bootstrap helper) left those pointers behind when the +target file got cleaned up. Subsequent boots → every TLS handshake on +the host fails with UnknownIssuer / FileNotFoundError before Agnes +itself runs. The 2026-05-11 Windows test user fixed the wedge manually; +`agnes init` now does it for them. + +The cleanup is best-effort. Tests pin: + - Current-process env vars pointing at non-existent paths get removed. + - Real paths are preserved (no false positives). + - PowerShell invocation failures don't abort the helper. + - On non-Windows, the PowerShell branch is skipped. +""" + +from __future__ import annotations + +import os +import sys +from pathlib import Path +from unittest.mock import patch, MagicMock + +import pytest + + +def test_cleanup_removes_stale_process_env_pointing_at_missing_file(monkeypatch, tmp_path): + """Each of the three known-bad vars gets removed when its value + points at a path that doesn't exist on disk.""" + from cli.commands.init import _cleanup_stale_ca_env_vars + + bogus = str(tmp_path / "does-not-exist.pem") + monkeypatch.setenv("SSL_CERT_FILE", bogus) + monkeypatch.setenv("REQUESTS_CA_BUNDLE", bogus) + monkeypatch.setenv("GIT_SSL_CAINFO", bogus) + + # Force the Windows branch off so we don't try to shell out in test env. + with patch("cli.commands.init._is_windows_host", return_value=False): + _cleanup_stale_ca_env_vars() + + assert "SSL_CERT_FILE" not in os.environ + assert "REQUESTS_CA_BUNDLE" not in os.environ + assert "GIT_SSL_CAINFO" not in os.environ + + +def test_cleanup_preserves_env_pointing_at_real_file(monkeypatch, tmp_path): + """Operator-configured paths that DO exist must not be touched — + the cleanup must only remove dangling pointers.""" + from cli.commands.init import _cleanup_stale_ca_env_vars + + real = tmp_path / "ca.pem" + real.write_text("-----BEGIN CERTIFICATE-----\nfake\n-----END CERTIFICATE-----\n") + monkeypatch.setenv("SSL_CERT_FILE", str(real)) + monkeypatch.setenv("REQUESTS_CA_BUNDLE", str(real)) + + with patch("cli.commands.init._is_windows_host", return_value=False): + _cleanup_stale_ca_env_vars() + + assert os.environ["SSL_CERT_FILE"] == str(real) + assert os.environ["REQUESTS_CA_BUNDLE"] == str(real) + + +def test_cleanup_skips_powershell_on_non_windows(monkeypatch, tmp_path): + """The User-scope cleanup leg is a no-op outside Windows. Tests run + on macOS / Linux — confirm we never spawn PowerShell there.""" + from cli.commands.init import _cleanup_stale_ca_env_vars + + monkeypatch.setenv("SSL_CERT_FILE", str(tmp_path / "missing.pem")) + + with patch("cli.commands.init._is_windows_host", return_value=False), \ + patch("cli.commands.init.subprocess.run") as mock_run: + _cleanup_stale_ca_env_vars() + + assert mock_run.call_count == 0 + + +def test_cleanup_invokes_powershell_on_windows(monkeypatch, tmp_path): + """On Windows, the cleanup shells out to PowerShell with a script + that GetEnvironmentVariable's each var at User scope and clears + those pointing at non-existent paths.""" + from cli.commands.init import _cleanup_stale_ca_env_vars + + monkeypatch.delenv("SSL_CERT_FILE", raising=False) + monkeypatch.delenv("REQUESTS_CA_BUNDLE", raising=False) + monkeypatch.delenv("GIT_SSL_CAINFO", raising=False) + + fake_result = MagicMock() + fake_result.stdout = "agnes init: cleared stale User-scope SSL_CERT_FILE=C:\\stale\\ca.pem (file does not exist)\n" + fake_result.returncode = 0 + + with patch("cli.commands.init._is_windows_host", return_value=True), \ + patch("cli.commands.init.subprocess.run", return_value=fake_result) as mock_run: + _cleanup_stale_ca_env_vars() + + assert mock_run.call_count == 1 + args, kwargs = mock_run.call_args + cmd = args[0] + assert cmd[0] == "powershell.exe" + assert "-NoProfile" in cmd + # Script body must reference all three vars + check Test-Path before deletion. + script = cmd[-1] + for var in ("SSL_CERT_FILE", "REQUESTS_CA_BUNDLE", "GIT_SSL_CAINFO"): + assert var in script, f"PowerShell script must check {var}" + assert "Test-Path" in script + assert "SetEnvironmentVariable" in script + + +def test_cleanup_swallows_powershell_failures(monkeypatch): + """PowerShell missing / blocked by execution policy must not abort + init — the cleanup is best-effort. Verifies the FileNotFoundError / + OSError handler silently absorbs the exception.""" + from cli.commands.init import _cleanup_stale_ca_env_vars + + with patch("cli.commands.init._is_windows_host", return_value=True), \ + patch("cli.commands.init.subprocess.run", side_effect=FileNotFoundError("powershell.exe not on PATH")): + # Must not raise. + _cleanup_stale_ca_env_vars() + + +def test_is_windows_host_reflects_sys_platform(monkeypatch): + """Helper toggles on `sys.platform == 'win32'`. Covers native Python + on Windows + Git Bash launchers (still 'win32' under the hood).""" + from cli.commands.init import _is_windows_host + + monkeypatch.setattr(sys, "platform", "win32") + assert _is_windows_host() is True + + for plat in ("darwin", "linux", "cygwin"): + monkeypatch.setattr(sys, "platform", plat) + assert _is_windows_host() is False diff --git a/tests/test_setup_instructions.py b/tests/test_setup_instructions.py index 540ad63..6c6b998 100644 --- a/tests/test_setup_instructions.py +++ b/tests/test_setup_instructions.py @@ -66,10 +66,9 @@ def test_resolve_lines_no_plugins_unified_layout(): assert "3) Verify the data is queryable:" in joined assert "4) Make sure git and claude are installed" in joined assert "5) Register the Agnes Claude Code marketplace" in joined - assert "6) Register the Atlassian MCP server" in joined - assert "7) Run diagnostics:" in joined - assert "8) Connect the user's tools" in joined - assert "9) Confirm:" in joined + assert "6) Run diagnostics:" in joined + assert "7) Connect the user's tools" in joined + assert "8) Confirm:" in joined # No stray Confirms at other positions. assert "10) Confirm:" not in joined assert "6) Confirm:" not in joined @@ -287,14 +286,13 @@ def test_resolve_lines_with_plugins_uses_install_first_diagnose_last_layout(): assert "claude plugin install foo@agnes" not in executable assert "claude plugin install bar@agnes" not in executable # Step 6 — Atlassian MCP registration (Fix C in 2026-05-10 init-report response). - assert "6) Register the Atlassian MCP server" in joined # Step 7 — diagnose now AFTER marketplace + MCP wiring. - assert "7) Run diagnostics:" in joined + assert "6) Run diagnostics:" in joined # Step 8 — connectors, the LAST interactive step before Confirm # (skills step deleted in #242). - assert "8) Connect the user's tools" in joined - assert "9) Confirm:" in joined - for stray in ("4) Confirm:", "5) Confirm:", "6) Confirm:", "7) Confirm:", "8) Confirm:", "10) Confirm:"): + assert "7) Connect the user's tools" in joined + assert "8) Confirm:" in joined + for stray in ("4) Confirm:", "5) Confirm:", "6) Confirm:", "7) Confirm:", "9) Confirm:", "10) Confirm:"): assert stray not in joined # Crucial ordering invariants for the new layout. install_idx = joined.index("1) Install the CLI") @@ -302,11 +300,10 @@ def test_resolve_lines_with_plugins_uses_install_first_diagnose_last_layout(): catalog_idx = joined.index("3) Verify the data is queryable:") git_idx = joined.index("4) Make sure git and claude are installed") market_idx = joined.index("5) Register the Agnes Claude Code marketplace") - mcp_idx = joined.index("6) Register the Atlassian MCP server") - diag_idx = joined.index("7) Run diagnostics:") - conn_idx = joined.index("8) Connect the user's tools") - confirm_idx = joined.index("9) Confirm:") - assert install_idx < init_idx < catalog_idx < git_idx < market_idx < mcp_idx < diag_idx < conn_idx < confirm_idx + diag_idx = joined.index("6) Run diagnostics:") + conn_idx = joined.index("7) Connect the user's tools") + confirm_idx = joined.index("8) Confirm:") + assert install_idx < init_idx < catalog_idx < git_idx < market_idx < diag_idx < conn_idx < confirm_idx # Legacy `git config sslVerify=false` downgrade is gone — see CHANGELOG. assert "git config --global" not in joined # server_host is server-side substituted; the placeholder must be gone. @@ -637,7 +634,7 @@ def test_resolve_lines_ca_pem_works_without_plugins(): joined = "\n".join(resolve_lines("agnes.whl", ca_pem=_FAKE_CA_PEM)) assert "0) Trust the Agnes TLS certificate" in joined - assert "9) Confirm:" in joined + assert "8) Confirm:" in joined # Marketplace block is now emitted unconditionally; the bootstrap # one-liner does the `claude plugin marketplace add` internally so # the literal string isn't in the prompt text — the user-facing @@ -715,12 +712,12 @@ def test_no_plugins_layout_keeps_diagnose_before_connectors(): from app.web.setup_instructions import resolve_lines joined = "\n".join(resolve_lines("agnes.whl")) - assert "7) Run diagnostics:" in joined - assert "8) Connect the user's tools" in joined - assert "9) Confirm:" in joined - diag_idx = joined.index("7) Run diagnostics:") - conn_idx = joined.index("8) Connect the user's tools") - confirm_idx = joined.index("9) Confirm:") + assert "6) Run diagnostics:" in joined + assert "7) Connect the user's tools" in joined + assert "8) Confirm:" in joined + diag_idx = joined.index("6) Run diagnostics:") + conn_idx = joined.index("7) Connect the user's tools") + confirm_idx = joined.index("8) Confirm:") assert diag_idx < conn_idx < confirm_idx @@ -857,18 +854,20 @@ def test_connectors_block_uses_gws_manual_branch_when_oauth_unset(): def test_step_numbering_with_connectors_step(): - """_step_numbers must return diagnose=7, connectors=8, confirm=9. - Anchors the numeric expectations the rest of the test suite assumes - (skills step deleted in #242, connectors added in #243).""" + """_step_numbers must return diagnose=6, connectors=7, confirm=8. + Anchors the numeric expectations the rest of the test suite assumes. + (Skills step deleted in #242; connectors added in #243; standalone + `mcp_servers` step retired and folded into the Atlassian connector's + prompt body, so the layout drops by one.)""" from app.web.setup_instructions import _step_numbers steps = _step_numbers() assert steps["preflight"] == "4" assert steps["marketplace"] == "5" - assert steps["mcp_servers"] == "6" - assert steps["diagnose"] == "7" - assert steps["connectors"] == "8" - assert steps["confirm"] == "9" + assert "mcp_servers" not in steps + assert steps["diagnose"] == "6" + assert steps["connectors"] == "7" + assert steps["confirm"] == "8" assert "skills" not in steps # deleted in #242 diff --git a/tests/test_setup_page_unified.py b/tests/test_setup_page_unified.py index 7ecc8b0..34ec9d2 100644 --- a/tests/test_setup_page_unified.py +++ b/tests/test_setup_page_unified.py @@ -51,7 +51,7 @@ def test_setup_page_renders_unified_layout(client): assert "agnes auth import-token" not in text # Always-on layout (preflight + marketplace + MCP + connectors block all # unconditional; skills step deleted in #242): Confirm = step 9. - assert "9) Confirm:" in text + assert "8) Confirm:" in text def test_setup_page_ignores_role_query_param(client): @@ -115,7 +115,7 @@ def test_setup_page_renders_marketplace_for_user_with_grants(client, monkeypatch assert "agnes refresh-marketplace --bootstrap" in text # Layout shift: Confirm is now step 9 (preflight + marketplace + MCP + # connectors all always-on; skills step deleted in #242). - assert "9) Confirm:" in text + assert "8) Confirm:" in text # Pre-flight is in the rendered prompt at step 4. assert "Make sure git and claude are installed" in text # Atlassian MCP registration is at step 6. diff --git a/tests/test_web_ui.py b/tests/test_web_ui.py index f68ea2d..c1e2089 100644 --- a/tests/test_web_ui.py +++ b/tests/test_web_ui.py @@ -236,7 +236,7 @@ class TestClaudeSetupPreview: # Step 1 install, step 4 preflight, step 5 marketplace, step 6 MCP, # step 7 diagnose. assert "1) Install the CLI" in body - assert "7) Run diagnostics" in body + assert "6) Run diagnostics" in body assert "agnes diagnose" in body # `agnes init` is now the mandatory bootstrap step. assert "agnes init" in body