From 5058a1cf0cf573caa39b846ad1271f411eb7ffdf Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Sat, 18 Apr 2026 18:37:58 +0900 Subject: [PATCH] =?UTF-8?q?fix(ci):=20three=20staging=20regressions=20?= =?UTF-8?q?=E2=80=94=20skill=20chain-load,=20duplicate=20Jobs=20tab,=20onb?= =?UTF-8?q?oarding=20E2E=20(#2637)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scheduled batched CI on staging was red across three unrelated paths. All three are fixed in-place; the existing tests become the regression coverage. 1. `tests/support/test_rig.rs`: rebuild the skill registry against the test's `with_skills_dir()` tempdir and actually run `discover_all()`. `AppBuilder::init_database()` reloads `config` from DB/TOML/env at the top of `build_all()`, which clobbered `config.skills.local_dir` back to the default (`~/.ironclaw/skills/`). Any registry `build_all()` constructed therefore pointed at the user's real skills dir, not the tempdir the test had laid down — so `loaded_skill_names()` came back empty and the v1 chain-load assertion panicked. Write the tempdir paths back onto `components.config.skills.*` so `AgentDeps::skills_config` agrees with the registry. `skill_chain_load_lifecycle::v1_chain_load_pulls_in_required_companions` now passes. 2. `crates/ironclaw_gateway/static/index.html`: drop the duplicate right-side `status-logs-btn` Jobs button added in #2353. The main tab-bar already has ``, and the duplicate had no `data-v1-only`/`data-v2-only` marker, so both rendered simultaneously. That broke `test_connection.py` (Playwright strict-mode rejected `.tab-bar button[data-tab="jobs"]` resolving to two elements) and also left both buttons visually `active` when the Jobs tab was open. 3. `tests/e2e/scenarios/test_extensions.py`: align `test_onboarding_failed_sse_shows_error_toast_and_reloads_extensions` with every other auth-card test in the file — resolve the real thread id via `_active_thread_id(page)` before calling `_show_auth_card`. `showAuthCard` short-circuits on `isCurrentThread(data.thread_id)`, and the synthetic `"thread-fail"` id fails that check once `currentThreadId` is populated after `go_to_extensions(page)`. The auth card was never rendered, so the follow-up `wait_for` for `.auth-card` hit its 5s timeout. Verified: `cargo test --features libsql --test skill_chain_load_lifecycle` and `--test skill_setup_marker_lifecycle` pass; `cargo clippy --tests --features libsql` is clean. Co-authored-by: Claude Opus 4.7 (1M context) --- crates/ironclaw_gateway/static/index.html | 2 -- tests/e2e/scenarios/test_extensions.py | 6 +++++- tests/support/test_rig.rs | 24 ++++++++++++++++------- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/crates/ironclaw_gateway/static/index.html b/crates/ironclaw_gateway/static/index.html index 7f2349a7fe..493358089c 100644 --- a/crates/ironclaw_gateway/static/index.html +++ b/crates/ironclaw_gateway/static/index.html @@ -163,8 +163,6 @@
- - Docs diff --git a/tests/e2e/scenarios/test_extensions.py b/tests/e2e/scenarios/test_extensions.py index eba3a6c99a..73de24638c 100644 --- a/tests/e2e/scenarios/test_extensions.py +++ b/tests/e2e/scenarios/test_extensions.py @@ -1282,7 +1282,11 @@ async def test_onboarding_failed_sse_shows_error_toast_and_reloads_extensions(pa await go_to_extensions(page) count_before = len(reload_count) - await _show_auth_card(page, extension_name="gmail", auth_url="https://example.com/oauth", request_id="req-fail", thread_id="thread-fail") + # Use the real active thread id so showAuthCard's isCurrentThread gate + # lets the card render. A synthetic "thread-fail" id is rejected once + # the page has initialized currentThreadId. + thread_id = await _active_thread_id(page) + await _show_auth_card(page, extension_name="gmail", auth_url="https://example.com/oauth", request_id="req-fail", thread_id=thread_id) assert await page.locator(SEL["auth_card"] + '[data-extension-name="gmail"]').count() == 1 # Inject a counter to confirm refreshCurrentSettingsTab is called diff --git a/tests/support/test_rig.rs b/tests/support/test_rig.rs index fa855e4b64..770ca917cf 100644 --- a/tests/support/test_rig.rs +++ b/tests/support/test_rig.rs @@ -1161,14 +1161,24 @@ impl TestRigBuilder { .register_routine_tools(Arc::clone(db_arc), engine); } - // Skills tools: use the config-resolved skills dirs so that a - // custom `with_skills_dir()` path propagates all the way to - // the registry (instead of always pointing at an empty temp dir). + // Skills tools: rebuild the registry against the test's tempdir. + // + // `AppBuilder::init_database()` re-resolves `config` from + // DB/TOML/env, which clobbers `config.skills.local_dir` back + // to the default (`~/.ironclaw/skills/`). Any registry + // `build_all()` already constructed therefore points at the + // user's real skills dir, not the tempdir the test laid + // down. Rebuild here from the in-scope `skills_dir` / + // `installed_skills_dir`, actually run discovery, and write + // the paths back onto `components.config` so downstream + // consumers (AgentDeps::skills_config) see the same dirs. if enable_skills { - let registry = Arc::new(std::sync::RwLock::new( - ironclaw_skills::SkillRegistry::new(components.config.skills.local_dir.clone()) - .with_installed_dir(components.config.skills.installed_dir.clone()), - )); + components.config.skills.local_dir = skills_dir.clone(); + components.config.skills.installed_dir = installed_skills_dir.clone(); + let mut registry = ironclaw_skills::SkillRegistry::new(skills_dir.clone()) + .with_installed_dir(installed_skills_dir.clone()); + let _loaded = registry.discover_all().await; + let registry = Arc::new(std::sync::RwLock::new(registry)); let catalog = ironclaw_skills::catalog::shared_catalog(); components .tools