fix(gateway): Settings extension button label reflects auth state (#2235) (#2709)

* fix(gateway): label Settings extension button Setup vs Reconfigure by auth state

Closes nearai/ironclaw#2235. Extracted from #2375.

The Settings → Extensions card fallback branch unconditionally labeled
the action button "Reconfigure", so users opening the settings for a
chat-installed channel saw "Reconfigure" even though credentials had
never been entered — and clicking it opened the credential popup,
matching the QA repro on the 2026-04-09 bug bash.

Pick the label from `ext.authenticated`: "Setup" when no credentials
are on file, "Reconfigure" once they are. `setup_required` /
`installed` keep the legacy label because the inline setup form below
already provides the same action — preserves the no-duplicate-setup
invariant guarded by `test_wasm_channel_setup_states`.

Tests:
- `test_extensions_list_reports_authenticated_after_setup_submit`
  drives POST setup-submit → GET list and asserts `authenticated`
  flips on the wire (the field the JS branch reads).
- `test_settings_extensions_labels.py` (Playwright) covers both
  label states, the no-duplicate-setup invariant, and that clicking
  Reconfigure on an authenticated channel does not fire /activate.

Does not touch `classify_wasm_channel_activation` (keeps the
`has_paired` axis the #1921 truth-table tests guard) or introduce an
`owner_bound` wire field.

Co-Authored-By: Nige <G7CNF@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(gateway): use ext.reconfigure i18n key consistently (PR #2709 review)

Address gemini-code-assist review on #2709: the `inlineSetupCoversIt`
branch was the sole remaining caller of `extensions.reconfigure`. All
other Reconfigure buttons in this file already use `ext.reconfigure`
(lines 177, 218, 354). Both keys resolve to the same string in en/ko/
zh-CN locales, so this is a no-op for users — it removes the odd key
out and aligns with the project's `extensions.*` → `ext.*` migration.

Declined the paired suggestion to swap `var` → `const`: the surrounding
wasm-channel branch consistently uses `var` (lines 309/311/317/325),
and partial modernization inside the same conditional is worse than
matching the existing style.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(gateway): drop 'installed' from inlineSetupCoversIt + fix Playwright mock (PR #2709 review)

Copilot review on #2709 flagged two real bugs:

1. `inlineSetupCoversIt` treated `fallbackStatus === 'installed'` as if
   an inline setup form was present, but the inline form only renders
   when effective status is `setup_required` (see `loadInlineChannelSetup`
   branch at line 380). A production `installed` wire shape
   (`activation_status='installed'`, `onboarding_state=null` —
   `derive_onboarding` only emits non-null for `Pairing`) therefore kept
   the `Reconfigure` label with no inline form, which is exactly the
   #2235 QA repro. Drop `installed` from the conditional.

2. `test_reconfigure_click_does_not_send_auth_event` mocked the
   setup-fetch with empty `secrets`/`fields`, which makes
   `showConfigureModal` short-circuit with a `noConfigNeeded` toast
   and never render `.configure-modal`. The wait-for would have timed
   out on first CI run. Return a non-empty `secrets` array so
   `renderConfigureModal` actually fires.

Also adds `test_fallback_button_says_setup_on_production_installed_wire_shape`
— pins the exact #2235 wire shape (activation_status='installed',
onboarding_state=null) so this class of bug has a named regression
test going forward.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: reuse `status` and drop redundant presence assert (PR #2709 review)

Copilot second-round review on #2709:

- extensions.js: `fallbackStatus` recomputed the expression already
  stored in `status` at line 309. Reuse `status` directly; drop the
  one-use `inlineSetupCoversIt` alias while we are here — the
  `status === 'setup_required'` branch is short enough to read inline.

- features/extensions/mod.rs: the `telegram.get("authenticated").is_some()`
  assertion is redundant with the preceding `assert_eq!(..., true)` —
  a missing field indexes to `Value::Null` and trips the equality check.
  Folded the "must stay on the wire" rationale into the equality
  assertion's message so the diagnostic still documents why the field
  matters.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Nige <G7CNF@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Illia Polosukhin
2026-04-20 15:27:30 +09:00
committed by GitHub
parent f2c4c258dd
commit 0476a3d8e9
3 changed files with 469 additions and 1 deletions

View File

@@ -322,9 +322,20 @@ function renderExtensionCard(ext) {
} else if (status === 'failed') {
actions.appendChild(createReconfigureButton(ext.name));
} else {
// Only `setup_required` has an inline setup form below (see the
// `loadInlineChannelSetup` branch), so keep the legacy label there
// to avoid a duplicate setup action. Every other fallback state —
// including the default `installed` when no `onboarding_state` is
// set — renders no inline form, so pick the label from
// `authenticated`: "Setup" before credentials are on file,
// "Reconfigure" after. Closes #2235.
var reconfigureBtn = document.createElement('button');
reconfigureBtn.className = 'btn-ext configure';
reconfigureBtn.textContent = I18n.t('extensions.reconfigure');
if (status === 'setup_required') {
reconfigureBtn.textContent = I18n.t('ext.reconfigure');
} else {
reconfigureBtn.textContent = ext.authenticated ? I18n.t('ext.reconfigure') : I18n.t('ext.setup');
}
reconfigureBtn.addEventListener('click', function() { showConfigureModal(ext.name); });
actions.appendChild(reconfigureBtn);
}

View File

@@ -1261,6 +1261,151 @@ mod tests {
assert_eq!(telegram["activation_status"], "installed");
}
/// Caller-level wire-contract regression for nearai/ironclaw#2235.
///
/// The Settings → Extensions UI picks the WASM-channel fallback button
/// label ("Setup" vs "Reconfigure") from `ExtensionInfo.authenticated`
/// on the `/api/extensions` response. A backend regression that left
/// `authenticated=false` after credentials were written — or dropped
/// the field off the wire entirely — would silently re-show the
/// credential popup on an already-configured install. The unit-level
/// classifier tests above cannot catch this because they do not
/// exercise the `configure()` → `list()` wire round-trip.
///
/// This test drives the real pair of handlers — POST setup then GET
/// list — against a stub channel whose WASM binary intentionally
/// fails to activate (so `active=false`). The `authenticated` flag
/// must still flip to `true` once the required secret lands in the
/// secrets store.
#[tokio::test]
async fn test_extensions_list_reports_authenticated_after_setup_submit() {
use axum::body::Body;
use tower::ServiceExt;
let secrets = test_secrets_store();
let (ext_mgr, _wasm_tools_dir, wasm_channels_dir) = test_ext_mgr(secrets);
let channel_name = "telegram";
std::fs::write(
wasm_channels_dir
.path()
.join(format!("{channel_name}.wasm")),
b"\0asm fake",
)
.expect("write fake wasm");
let caps = serde_json::json!({
"type": "channel",
"name": channel_name,
"setup": {
"required_secrets": [
{"name": "BOT_TOKEN", "prompt": "Enter bot token"}
]
}
});
std::fs::write(
wasm_channels_dir
.path()
.join(format!("{channel_name}.capabilities.json")),
serde_json::to_string(&caps).expect("serialize caps"),
)
.expect("write capabilities");
let state = test_gateway_state(Some(ext_mgr));
let app = Router::new()
.route("/api/extensions", get(extensions_list_handler))
.route(
"/api/extensions/{name}/setup",
post(extensions_setup_submit_handler),
)
.with_state(state);
// Pre-setup: authenticated must be false.
let mut req = axum::http::Request::builder()
.method("GET")
.uri("/api/extensions")
.body(Body::empty())
.expect("request");
req.extensions_mut().insert(UserIdentity {
user_id: "test".to_string(),
role: "admin".to_string(),
workspace_read_scopes: Vec::new(),
});
let resp = ServiceExt::<axum::http::Request<Body>>::oneshot(app.clone(), req)
.await
.expect("pre-setup response");
assert_eq!(resp.status(), StatusCode::OK);
let body = axum::body::to_bytes(resp.into_body(), 1024 * 64)
.await
.expect("body");
let parsed: serde_json::Value = serde_json::from_slice(&body).expect("json");
let telegram = parsed["extensions"]
.as_array()
.and_then(|items| items.iter().find(|item| item["name"] == channel_name))
.expect("telegram entry pre-setup");
assert_eq!(
telegram["authenticated"], false,
"pre-setup: authenticated must start false (the JS Settings card shows \
'Setup' in this state — regressing it to true would flip the button to \
'Reconfigure' before credentials exist, re-introducing #2235)"
);
// Submit credentials via the real setup-submit handler.
let submit_body = serde_json::json!({
"secrets": {
"BOT_TOKEN": "dummy-token"
}
});
let mut req = axum::http::Request::builder()
.method("POST")
.uri(format!("/api/extensions/{channel_name}/setup"))
.header("content-type", "application/json")
.body(Body::from(submit_body.to_string()))
.expect("setup-submit request");
req.extensions_mut().insert(UserIdentity {
user_id: "test".to_string(),
role: "admin".to_string(),
workspace_read_scopes: Vec::new(),
});
let resp = ServiceExt::<axum::http::Request<Body>>::oneshot(app.clone(), req)
.await
.expect("setup-submit response");
assert_eq!(resp.status(), StatusCode::OK);
// Post-setup: the wire contract the Settings UI depends on.
let mut req = axum::http::Request::builder()
.method("GET")
.uri("/api/extensions")
.body(Body::empty())
.expect("request");
req.extensions_mut().insert(UserIdentity {
user_id: "test".to_string(),
role: "admin".to_string(),
workspace_read_scopes: Vec::new(),
});
let resp = ServiceExt::<axum::http::Request<Body>>::oneshot(app, req)
.await
.expect("post-setup response");
assert_eq!(resp.status(), StatusCode::OK);
let body = axum::body::to_bytes(resp.into_body(), 1024 * 64)
.await
.expect("body");
let parsed: serde_json::Value = serde_json::from_slice(&body).expect("json");
let telegram = parsed["extensions"]
.as_array()
.and_then(|items| items.iter().find(|item| item["name"] == channel_name))
.expect("telegram entry post-setup");
// A missing field indexes to `Value::Null` and trips this assertion
// too, so the single check covers both "field stripped from the wire"
// and "field present but wrong value".
assert_eq!(
telegram["authenticated"], true,
"post-setup: the `authenticated` flag must be present and true once \
the required secret is written — the Settings card's \
Setup/Reconfigure branch reads this field directly, and a regression \
here reopens #2235."
);
}
#[test]
fn apply_extension_readiness_preserves_install_success_for_auth_followup() {
let mut resp = ActionResponse::ok("Installed notion");

View File

@@ -0,0 +1,312 @@
"""Scenario: Settings → Extensions fallback button label regression.
Closes nearai/ironclaw#2235 — clicking the action button on an already-
authenticated WASM channel previously re-prompted for credentials
because the fallback branch labeled it "Reconfigure" unconditionally.
The Settings UI now picks the label from `ext.authenticated`: "Setup"
when no credentials are on file yet, "Reconfigure" once they are.
The inline setup form already provides a setup action when
`onboarding_state === 'setup_required'`, so we keep the legacy
"Reconfigure" label in that one state to preserve the
no-duplicate-setup-button invariant guarded by
`test_wasm_channel_setup_states` in `test_extensions.py`. Those two
invariants are both exercised here — the first (the #2235 fix) and the
second (no regression) — so a future edit that re-introduces
duplication or re-breaks the label will fail a named test rather than
slip through.
Every assertion runs against route-mocked `/api/extensions` responses
so we exercise the production JS render path without needing a real
WASM channel binary.
"""
import json
from helpers import SEL
_WASM_CHANNEL_BASE = {
"name": "test-channel-labels",
"display_name": "Label Channel",
"kind": "wasm_channel",
"description": "A WASM channel used to assert Settings card button labels.",
"url": None,
"tools": [],
"activation_error": None,
"has_auth": False,
"needs_setup": True,
}
async def _mock_and_go(page, *, installed, setup_secrets=None):
"""Mock /api/extensions with the given installed list and open the tab.
`setup_secrets` is the `secrets` array returned by
`/api/extensions/{name}/setup`. Defaults to empty — that makes
`showConfigureModal` short-circuit with a toast and skip rendering
the configure modal, which is fine for tests that only assert button
labels. Pass a non-empty list to exercise the full
`renderConfigureModal` path.
"""
ext_body = json.dumps({"extensions": installed})
async def handle_ext(route):
path = route.request.url.split("?")[0]
if path.endswith("/api/extensions"):
await route.fulfill(status=200, content_type="application/json", body=ext_body)
else:
await route.continue_()
await page.route("**/api/extensions*", handle_ext)
async def handle_registry(route):
await route.fulfill(
status=200,
content_type="application/json",
body=json.dumps({"entries": []}),
)
await page.route("**/api/extensions/registry", handle_registry)
secrets_payload = setup_secrets if setup_secrets is not None else []
async def handle_setup_fetch(route):
await route.fulfill(
status=200,
content_type="application/json",
body=json.dumps(
{
"name": _WASM_CHANNEL_BASE["name"],
"kind": "wasm_channel",
"secrets": secrets_payload,
"fields": [],
"onboarding_state": None,
"onboarding": None,
}
),
)
await page.route(
f"**/api/extensions/{_WASM_CHANNEL_BASE['name']}/setup",
handle_setup_fetch,
)
await page.locator(SEL["tab_button"].format(tab="settings")).click()
await page.locator(SEL["settings_subtab"].format(subtab="channels")).click()
await page.locator(SEL["settings_subpanel"].format(subtab="channels")).wait_for(
state="visible", timeout=5000
)
def _channel_with(**overrides):
return {**_WASM_CHANNEL_BASE, **overrides}
async def test_fallback_button_says_setup_when_not_authenticated(page):
"""Unauthenticated WASM channel in `configured` state shows Setup, not Reconfigure.
This is the core #2235 regression: the old build unconditionally said
"Reconfigure" here, leading users to click it expecting a configured
channel only to be shown a credential entry form.
"""
await _mock_and_go(
page,
installed=[
_channel_with(
active=False,
authenticated=False,
activation_status="configured",
onboarding_state="activation_in_progress",
onboarding=None,
)
],
)
card = page.locator(
SEL["channels_ext_card"], has_text=_WASM_CHANNEL_BASE["display_name"]
).first
await card.wait_for(state="visible", timeout=5000)
setup_btn = card.locator(SEL["ext_configure_btn"], has_text="Setup")
reconfig_btn = card.locator(SEL["ext_configure_btn"], has_text="Reconfigure")
assert await setup_btn.count() == 1, (
"fallback button must say 'Setup' when no credentials are on file "
"(authenticated=false) — regresses #2235 if it says 'Reconfigure'"
)
assert await reconfig_btn.count() == 0, (
"fallback button must not say 'Reconfigure' for an unauthenticated channel"
)
async def test_fallback_button_says_reconfigure_when_authenticated(page):
"""Authenticated WASM channel in `configured` state shows Reconfigure."""
await _mock_and_go(
page,
installed=[
_channel_with(
active=False,
authenticated=True,
activation_status="configured",
onboarding_state="activation_in_progress",
onboarding=None,
)
],
)
card = page.locator(
SEL["channels_ext_card"], has_text=_WASM_CHANNEL_BASE["display_name"]
).first
await card.wait_for(state="visible", timeout=5000)
reconfig_btn = card.locator(SEL["ext_configure_btn"], has_text="Reconfigure")
setup_btn = card.locator(SEL["ext_configure_btn"], has_text="Setup")
assert await reconfig_btn.count() == 1, (
"fallback button must say 'Reconfigure' when credentials are on file "
"(authenticated=true)"
)
assert await setup_btn.count() == 0, (
"fallback button must not say 'Setup' once authenticated"
)
async def test_fallback_button_says_setup_on_production_installed_wire_shape(page):
"""Exact #2235 production wire shape: `activation_status='installed'`,
`onboarding_state=null` — `derive_onboarding` only emits a non-null
onboarding state for the `Pairing` variant, so real clients never
receive `setup_required` alongside `activation_status='installed'`.
The inline setup form only renders when the effective status is
`setup_required`, so this card shows no inline form — the action-area
button is the ONLY setup affordance. Under the bug the label read
'Reconfigure' here, which is the precise shape Copilot flagged and
the precise shape the QA bug-bash repro hit.
"""
await _mock_and_go(
page,
installed=[
_channel_with(
active=False,
authenticated=False,
activation_status="installed",
onboarding_state=None,
onboarding=None,
)
],
)
card = page.locator(
SEL["channels_ext_card"], has_text=_WASM_CHANNEL_BASE["display_name"]
).first
await card.wait_for(state="visible", timeout=5000)
assert await card.locator(SEL["ext_onboarding"]).count() == 0, (
"production `installed` wire shape must not render an inline setup "
"form — if this ever changes, revisit the `inlineSetupCoversIt` rule"
)
setup_btn = card.locator(SEL["ext_configure_btn"], has_text="Setup")
reconfig_btn = card.locator(SEL["ext_configure_btn"], has_text="Reconfigure")
assert await setup_btn.count() == 1, (
"fallback button must say 'Setup' for an unauthenticated channel in "
"the default `installed` state — this is the exact wire shape of #2235"
)
assert await reconfig_btn.count() == 0, (
"fallback button must not say 'Reconfigure' when credentials are not "
"yet on file and no inline setup form covers the action"
)
async def test_fallback_button_preserves_no_duplicate_setup_invariant(page):
"""`setup_required` + unauthenticated keeps the legacy label so the action
button does not duplicate the inline setup form's call-to-action.
The sibling invariant also asserted by `test_wasm_channel_setup_states`.
Kept here so a future refactor that inlines the Setup-label change into
this branch trips a named regression rather than quietly duplicating the
UI element.
"""
await _mock_and_go(
page,
installed=[
_channel_with(
active=False,
authenticated=False,
activation_status="installed",
onboarding_state="setup_required",
onboarding=None,
)
],
)
card = page.locator(
SEL["channels_ext_card"], has_text=_WASM_CHANNEL_BASE["display_name"]
).first
await card.wait_for(state="visible", timeout=5000)
setup_btn = card.locator(SEL["ext_configure_btn"], has_text="Setup")
assert await setup_btn.count() == 0, (
"the action-area button must not say 'Setup' while the inline setup "
"form covers the same action (no-duplicate-setup-button invariant)"
)
async def test_reconfigure_click_does_not_send_auth_event(page):
"""Clicking Reconfigure on an already-authenticated channel must not
reissue a credential-prompt SSE event or trigger a reactivation request.
Covers the runtime half of the QA repro — clicking the button should
open the configure modal locally, not fire a handshake that the backend
would translate into a credential popup again.
"""
# Pass a non-empty `secrets` so `showConfigureModal` actually renders
# `.configure-modal` — with an empty list it short-circuits with a
# "no config needed" toast and the modal selector never appears.
await _mock_and_go(
page,
installed=[
_channel_with(
active=True,
authenticated=True,
activation_status="active",
onboarding_state="ready",
onboarding=None,
)
],
setup_secrets=[
{
"name": "BOT_TOKEN",
"prompt": "Bot token",
"provided": True,
"optional": False,
"auto_generate": False,
}
],
)
activate_calls = {"count": 0}
async def handle_activate(route):
activate_calls["count"] += 1
await route.fulfill(
status=200,
content_type="application/json",
body=json.dumps({"success": True, "activated": True}),
)
await page.route(
f"**/api/extensions/{_WASM_CHANNEL_BASE['name']}/activate",
handle_activate,
)
card = page.locator(
SEL["channels_ext_card"], has_text=_WASM_CHANNEL_BASE["display_name"]
).first
await card.wait_for(state="visible", timeout=5000)
reconfig_btn = card.locator(SEL["ext_configure_btn"], has_text="Reconfigure")
assert await reconfig_btn.count() == 1
await reconfig_btn.click()
# Modal should open locally.
await page.locator(SEL["configure_modal"]).wait_for(state="visible", timeout=3000)
assert activate_calls["count"] == 0, (
"Reconfigure must not call /activate — the button opens the configure "
"modal only. A non-zero call count means the click path regressed to "
"trigger activation (the shape of the #2235 repro)."
)