fix(staging): repair 4 categories of CI test failures by henrypark133 · Pull Request #2091 · nearai/ironclaw

henrypark133 · 2026-04-07T00:23:13Z

Summary

Fixes multiple CI test failures from run #24048967956:

Telegram token unit test (flaky): add env mutex guard to prevent race with concurrent tests that override IRONCLAW_TEST_TELEGRAM_API_BASE_URL
SSE/connection E2E tests (stale): #sse-status element was removed from HTML — update tests to use #sse-dot CSS class; add SSE-ready wait to page fixture
Tool approval E2E tests (stale): API unified legacy pending_approval into pending_gate field — update all test helpers
WASM tar.gz extraction (real bug): canonicalized names use underscores (web_search) but release archives use hyphens (web-search.wasm) — accept both forms

Test plan

cargo check — compiles cleanly
cargo clippy --all --all-features — zero warnings
cargo test --lib test_telegram_token_colon — passes
CI: unit tests (Tests default) should pass
CI: E2E SSE/connection/chat tests should pass
CI: E2E tool approval + owner scope tests should pass
CI: E2E WASM lifecycle tests should pass

🤖 Generated with Claude Code

1. Telegram token test flaky race: add env mutex guard so concurrent tests that override IRONCLAW_TEST_TELEGRAM_API_BASE_URL don't pollute the unguarded read in the colon-preservation test. 2. SSE/connection E2E tests: #sse-status element was removed from HTML and replaced with #sse-dot colored indicator. Update tests to check the dot's CSS class instead of text content. Add SSE-ready wait to the page fixture so chat tests don't race against connection setup. 3. Tool approval E2E tests: API unified legacy pending_approval and engine v2 gates into a single pending_gate response field. Update all E2E test helpers to use the correct field name. 4. WASM tar.gz extraction bug: canonicalized extension names use underscores (web_search) but release archives use hyphens (web-search.wasm). Accept both filename forms when extracting. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

gemini-code-assist

Code Review

This pull request introduces several updates to improve compatibility and align E2E tests with recent UI and API changes. Key changes include adding support for both underscored and hyphenated filenames in extension archives within the ExtensionManager, and renaming pending_approval to pending_gate across multiple test scenarios. Additionally, the E2E testing suite was updated to use the new #sse-dot element for connection status verification following the removal of the #sse-status text element. A thread-safety improvement was also added to the Telegram API tests using an environment lock. I have no feedback to provide as no review comments were submitted.

Copilot

Pull request overview

This PR addresses multiple CI failures by updating E2E tests to match recent API/UI changes, reducing test flakiness, and fixing a real bug in WASM bundle extraction.

Changes:

Update E2E approval-related tests to use the unified pending_gate history field instead of legacy pending_approval.
Update SSE/connection E2E tests to use the new #sse-dot connection indicator and add an SSE-ready wait in the Playwright page fixture.
Fix tar.gz WASM extraction to accept both underscore and hyphen variants of extension filenames; serialize env access in the Telegram token unit test.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
tests/e2e/scenarios/test_v2_engine_error_handling.py	Switch approval polling to `pending_gate`.
tests/e2e/scenarios/test_v2_engine_approval_flow.py	Switch approval polling to `pending_gate` throughout the scenario.
tests/e2e/scenarios/test_tool_approval.py	Update tool-approval E2E helpers/assertions to `pending_gate`.
tests/e2e/scenarios/test_sse_reconnect.py	Update SSE “connected” checks from removed `#sse-status` to `#sse-dot`.
tests/e2e/scenarios/test_skill_oauth_flow.py	Switch auto-approve logic to `pending_gate`.
tests/e2e/scenarios/test_owner_scope.py	Switch pending approval polling to `pending_gate` and update related messaging.
tests/e2e/scenarios/test_oauth_refresh.py	Switch auto-approve logic to `pending_gate`.
tests/e2e/scenarios/test_connection.py	Update connection assertion to use `#sse-dot`.
tests/e2e/conftest.py	Add SSE-ready wait to the shared `page` fixture.
src/extensions/manager.rs	Accept hyphenated filenames in WASM bundle extraction; add env mutex guard in Telegram token URL unit test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- SSE wait: use sseHasConnectedBefore JS flag (set in onopen) instead of checking #sse-dot CSS class, which defaults to connected state before SSE actually connects - Rename _wait_for_pending_approval → _wait_for_pending_gate and _wait_for_no_pending_approval → _wait_for_no_pending_gate - Update all docstrings/error messages to say pending_gate - Deduplicate name.replace('_', '-') in tar.gz extraction and include both accepted filenames in the error message Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Add comment documenting invariant: canonical names use underscores, archives may use hyphens, reverse is not supported - Fix quote wrapping in single-name error case - Remove dead SEL["sse_status"] selector from helpers.py - Simplify SSE wait: use window.sseHasConnectedBefore === true (fails fast on rename instead of silent 10s timeout) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Staging already introduced ArchiveFilenames in naming.rs to handle the hyphen/underscore tar.gz extraction issue. Take the upstream solution and drop our inline variant. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sseHasConnectedBefore is declared with let at global scope, which does not create a window property. window.sseHasConnectedBefore would always be undefined. Use typeof guard + direct reference instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The tar header's declared size is attacker-controlled. Without capping, Vec::with_capacity could attempt a huge allocation and OOM before the read_to_end take() limit kicks in. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

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

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

tests/e2e/scenarios/test_v2_engine_approval_flow.py:313

This docstring still refers to pending_approval, but the API field and the rest of the test logic were migrated to pending_gate. Updating this text will avoid confusion when diagnosing CI failures and keep terminology consistent with the current history payload.

    """Test the v2 engine tool approval lifecycle.

    Uses text-based approval ("yes"/"no"/"always" as chat messages) rather
    than the /api/chat/approval endpoint, since the v2 engine's pending_approval
    metadata uses engine thread IDs that differ from the v1 session thread IDs

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…g reset - test_sse_status_shows_connected now checks #sse-dot CSS class (visual indicator) instead of re-checking sseHasConnectedBefore which the page fixture already guarantees - Add comment to test_sse_reconnect_after_disconnect explaining why sseHasConnectedBefore is reset and that the history-reload path is covered by test_sse_reconnect_preserves_chat_history Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nickpismenkov

lgtm

Resolved conflicts: - src/extensions/manager.rs test_telegram_token_colon_preserved_in_validation_url: staging's PR #2091 added a one-line `lock_env()` guard to fix the same flake my previous commit fixed defensively via `ScopedEnvVar::set(...,"")`. Both approaches work; took staging's simpler version to minimize churn. Drive-by fix needed to keep CI green: - src/channels/web/server.rs test_extensions_setup_submit_returns_failure_when_not_activated: staging PR #2129 changed `ExtensionManager::configure` to canonicalize the extension name before file I/O. The unit test wrote `test-failing-channel.wasm` and the canonicalization rewrote the lookup name to `test_failing_channel`, so configure() returned Err before reaching the activation step the test wanted to exercise. Renamed the test channel to `test_failing_channel` so the canonical form matches the file on disk and the test once again exercises the intended "wasm activation fails" branch (returning a JSON body with `activated: false`). PR #2129's own changes did not update this Rust unit test alongside the Python e2e test renames. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(staging): repair 4 categories of CI test failures 1. Telegram token test flaky race: add env mutex guard so concurrent tests that override IRONCLAW_TEST_TELEGRAM_API_BASE_URL don't pollute the unguarded read in the colon-preservation test. 2. SSE/connection E2E tests: #sse-status element was removed from HTML and replaced with #sse-dot colored indicator. Update tests to check the dot's CSS class instead of text content. Add SSE-ready wait to the page fixture so chat tests don't race against connection setup. 3. Tool approval E2E tests: API unified legacy pending_approval and engine v2 gates into a single pending_gate response field. Update all E2E test helpers to use the correct field name. 4. WASM tar.gz extraction bug: canonicalized extension names use underscores (web_search) but release archives use hyphens (web-search.wasm). Accept both filename forms when extracting. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address review feedback — stronger SSE signal, consistent naming - SSE wait: use sseHasConnectedBefore JS flag (set in onopen) instead of checking #sse-dot CSS class, which defaults to connected state before SSE actually connects - Rename _wait_for_pending_approval → _wait_for_pending_gate and _wait_for_no_pending_approval → _wait_for_no_pending_gate - Update all docstrings/error messages to say pending_gate - Deduplicate name.replace('_', '-') in tar.gz extraction and include both accepted filenames in the error message Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address second round of review feedback - Add comment documenting invariant: canonical names use underscores, archives may use hyphens, reverse is not supported - Fix quote wrapping in single-name error case - Remove dead SEL["sse_status"] selector from helpers.py - Simplify SSE wait: use window.sseHasConnectedBefore === true (fails fast on rename instead of silent 10s timeout) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use global binding for sseHasConnectedBefore, not window property sseHasConnectedBefore is declared with let at global scope, which does not create a window property. window.sseHasConnectedBefore would always be undefined. Use typeof guard + direct reference instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(security): cap tar.gz entry pre-allocation to MAX_ENTRY_SIZE The tar header's declared size is attacker-controlled. Without capping, Vec::with_capacity could attempt a huge allocation and OOM before the read_to_end take() limit kicks in. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: cargo fmt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: make test_sse_status_shows_connected non-redundant, document flag reset - test_sse_status_shows_connected now checks #sse-dot CSS class (visual indicator) instead of re-checking sseHasConnectedBefore which the page fixture already guarantees - Add comment to test_sse_reconnect_after_disconnect explaining why sseHasConnectedBefore is reset and that the history-reload path is covered by test_sse_reconnect_preserves_chat_history Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings April 7, 2026 00:23

github-actions Bot added scope: extensions Extension management size: M 50-199 changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Apr 7, 2026

Copilot started reviewing on behalf of henrypark133 April 7, 2026 00:23 View session

gemini-code-assist Bot reviewed Apr 7, 2026

View reviewed changes

henrypark133 requested a review from serrrfirat April 7, 2026 00:28

Copilot AI reviewed Apr 7, 2026

View reviewed changes

serrrfirat mentioned this pull request Apr 7, 2026

Flaky test: test_telegram_token_colon_preserved_in_validation_url leaks across parallel runs #2102

Open

serrrfirat reviewed Apr 7, 2026

View reviewed changes

Comment thread src/extensions/manager.rs

serrrfirat reviewed Apr 7, 2026

View reviewed changes

Comment thread src/extensions/manager.rs Outdated

serrrfirat reviewed Apr 7, 2026

View reviewed changes

Comment thread tests/e2e/scenarios/test_connection.py

serrrfirat reviewed Apr 7, 2026

View reviewed changes

Comment thread tests/e2e/conftest.py

Copilot AI review requested due to automatic review settings April 7, 2026 14:46

Copilot started reviewing on behalf of henrypark133 April 7, 2026 14:47 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

Comment thread tests/e2e/scenarios/test_connection.py Outdated

Comment thread src/extensions/manager.rs

henrypark133 and others added 2 commits April 7, 2026 08:14

henrypark133 requested review from Copilot and serrrfirat April 7, 2026 16:05

Copilot started reviewing on behalf of henrypark133 April 7, 2026 16:06 View session

style: cargo fmt

2ca817f

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

Copilot AI reviewed Apr 7, 2026

View reviewed changes

serrrfirat reviewed Apr 7, 2026

View reviewed changes

Comment thread tests/e2e/scenarios/test_sse_reconnect.py

serrrfirat reviewed Apr 7, 2026

View reviewed changes

Comment thread tests/e2e/scenarios/test_sse_reconnect.py

henrypark133 requested review from serrrfirat and zmanian April 7, 2026 16:36

nickpismenkov approved these changes Apr 7, 2026

View reviewed changes

henrypark133 merged commit a1ef85f into staging Apr 7, 2026
14 checks passed

henrypark133 deleted the fix/staging-ci-failures branch April 7, 2026 22:19

ironclaw-ci Bot mentioned this pull request Apr 10, 2026

chore: release #2075

Merged

ironclaw-ci Bot mentioned this pull request Apr 18, 2026

chore: release #2606

Merged

ilblackdragon mentioned this pull request Apr 18, 2026

feat(ci): AI-first CI workflows — review, interact, health monitor, smoke test #2459

Closed

6 tasks

Conversation

henrypark133 commented Apr 7, 2026

Summary

Test plan

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

nickpismenkov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants