Skip to content

fix(e2e-sanity): normalize unexpected curl exit codes in cleanup trap (#2159) - #2162

Merged
HongmingWang-Rabbit merged 1 commit into
stagingfrom
fix/e2e-sanity-rc-normalization
Apr 27, 2026
Merged

fix(e2e-sanity): normalize unexpected curl exit codes in cleanup trap (#2159)#2162
HongmingWang-Rabbit merged 1 commit into
stagingfrom
fix/e2e-sanity-rc-normalization

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Summary

Closes #2159 — false-positive priority-high "🚨 E2E teardown safety net broken" issue opened by the weekly sanity run on 2026-04-27.

Root cause

When E2E_INTENTIONAL_FAILURE=1, tests/e2e/test_staging_full_saas.sh poisons the per-tenant admin token before step 5/11. The provision call then hits HTTP 401, and curl exits 22 under --fail-with-body. set -e propagates rc=22 unchanged.

The script's docstring contracts exit codes {0, 1, 2, 3, 4}, and e2e-staging-sanity.yml only matches those — rc=22 falls through to:

##[error]Unexpected rc=22 — neither clean-failure nor leak. Investigate harness.

…which then files this issue. The teardown trap actually fired correctly (✅ Teardown clean — no orphan resources), but the rc-mapping bug masked that.

Fix

cleanup_org now:

  1. Captures $? as its first statement (before any command can clobber it)
  2. After teardown + leak check complete, normalizes any non-contract code to 1

The leak-detection exit 4 path is unchanged — that branch still returns directly without reaching the normalization tail.

Tests

tests/e2e/test_harness_rc_normalization.sh — self-contained regression test that builds a stub harness with the same trap pattern and exercises:

Trigger rc Expected Reason
0 0 happy path
1, 2, 3 1, 2, 3 contracted fail(), env-missing, timeout
4 4 leak detected (cleanup's own exit)
22 1 the bug — curl HTTP error normalizes
6, 28, 35 1 DNS / timeout / SSL — other realistic curl failures
139 1 sigsegv'd child

10/10 pass locally. Shellcheck clean.

Why a unit test, not an E2E reproduce

The actual reproduction needs network + a poisoned staging tenant token; impractical to gate per-PR. The stub-harness approach pins the trap-pattern's behavior in isolation so a future refactor that drops the case statement fails fast.

🤖 Generated with Claude Code

…#2159)

When E2E_INTENTIONAL_FAILURE=1 poisons the tenant token, step 5/11's
`tenant_call POST /workspaces` curl exits 22 (HTTP error under
--fail-with-body). `set -e` propagates rc=22 directly, but the
script's documented contract emits only {0,1,2,3,4}, and the sanity
workflow's case statement only matches those. rc=22 falls through
to "Unexpected rc — investigate harness" and opens a false-positive
priority-high "safety net broken" issue (#2159, weekly run on
2026-04-27).

The trap now captures $? at entry (must be the first statement
before any command clobbers it) and at the end normalizes any
non-contract code to 1 (generic failure). Leak detection continues
to exit 4 directly, so its semantics are preserved.

Adds tests/e2e/test_harness_rc_normalization.sh — a self-contained
regression test that builds a stub harness with the same trap
pattern, triggers controlled exit codes, and asserts the
normalization. Covers the 5 contracted codes + curl-22 (the bug) +
3 representative network-failure codes + sigsegv-139.

Verification:
  - 10/10 regression tests pass
  - shellcheck clean on both modified files
  - production teardown path unchanged for legitimate {1,2,3,4}
    failures and the leak-detection exit 4

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@HongmingWang-Rabbit
HongmingWang-Rabbit added this pull request to the merge queue Apr 27, 2026
Merged via the queue into staging with commit 6f0774c Apr 27, 2026
15 checks passed
@molecule-ai
molecule-ai Bot deleted the fix/e2e-sanity-rc-normalization branch May 20, 2026 06:21
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
…roxy env absent (#2162)

applyPlatformManagedLLMEnv falsely reported HasUsableLLMCred:true when
MOLECULE_LLM_BASE_URL + MOLECULE_LLM_USAGE_TOKEN were empty, causing
claude-code workspaces to boot credential-less and hit the 600s
provision-timeout sweep (adk-demo dark-wedge class).

Fix:
- Empty-proxy-env path returns HasUsableLLMCred:false (was true).
- Caller aborts with MISSING_PLATFORM_PROXY, symmetric to the BYOK
  MISSING_BYOK_CREDENTIAL hard-fail.
- User-facing error message explains the boot-race and retry path.

Regression tests:
- TestApplyPlatformManagedLLMEnv_MissingProxyEnvFailClosed: asserts
  HasUsableLLMCred=false when proxy env absent.
- TestApplyPlatformManagedLLMEnv_ProxyEnvPresentInjectsCredential:
  asserts ANTHROPIC_API_KEY + ANTHROPIC_BASE_URL injected when proxy
  env present.

Refs: #2162, #711 (BYOK fail-closed pattern), #1994
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
…ed default

The #2162 fix adds a MISSING_PLATFORM_PROXY abort when a platform-managed
workspace has no CP proxy env. Five existing tests call prepareProvisionContext
or provisionWorkspaceCP with a payload that resolves to platform_managed but
do not set MOLECULE_LLM_BASE_URL / MOLECULE_LLM_USAGE_TOKEN, causing them to
abort early and fail their assertions.

Add the proxy env to:
- TestPrepareProvisionContext_ParentIDInjected
- TestPrepareProvisionContext_InjectsGitHTTPCredsFromPersonaToken
- TestPrepareProvisionContext_WorkspaceSecretWinsOverPersonaToken
- TestProvisionWorkspaceCP_NoInternalErrorsInBroadcast
- TestProvisionWorkspaceCP_ConcurrentBurst_NoSilentDrop

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
Three auto-routing tests (TestProvisionWorkspaceAuto_RoutesToCPWhenSet,
TestRestartWorkspaceAuto_RoutesToCPWhenSet,
TestProvisionWorkspaceAutoSync_RoutesToCPWhenSet) use
models.CreateWorkspacePayload with Runtime="claude-code" and empty Model.
This now derives to platform_managed billing mode, which fails closed
with MISSING_PLATFORM_PROXY when the CP proxy env is absent.

Supply the proxy env via t.Setenv so the tests reach the CP provisioner
stub instead of aborting early.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
…ail-closed when CP proxy env absent (#2162)' (#2164) from fix/2162-platform-managed-fail-closed-missing-proxy into main
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
…re + stale-head regression tests (#2159)

1. DOC - runbooks/dev-sop.md:
   - Documents the Gitea PR-head workflow-selection rule (workflows load
     from PR head, not base).
   - Describes the standard core-PR flow: auto-fire for fresh heads,
     slash-refire fallback for stale heads.
   - Provides quick-check curl command and rebase vs. slash-refire guidance.

2. LIVE-FIRE TEST - test_gate_auto_fire_live.py:
   - Runtime verification that submitting an APPROVED review to a PR whose
     head contains the current gate workflows causes Gitea Actions to queue
     qa-review + security-review and POST the BP-required contexts.
   - Fix: handle string trigger form in addition to list/dict.

3. STALE-HEAD DIAGNOSTIC - test_gate_stale_head_diagnostic.py:
   - Local-checkout baseline + optional PR_NUMBER mode.
   - Fix: avoid double exc.read() on HTTPError (always returned empty).
   - Fix: handle string trigger form.

CR round-2 fixes:
- Reverted out-of-scope Go changes that accidentally reverted the #2162
  platform-managed fail-closed guard.
- Restored regression tests and env-mocking that were removed from Go tests.
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
…2199)

E2E Staging Canvas (Playwright) / "Canvas tabs E2E" went red on main HEAD
b9d2f02. The actual failure (runner-6 task 258160) is in the Playwright
globalSetup, NOT in any spec assertion:

  [staging-setup] Workspace created: 8e5c7354-...
  Error: Workspace failed: (no last_sample_error) full body:
    {... "runtime":"hermes","status":"failed","uptime_seconds":0,
     "last_sample_error":null ...}
    at canvas/e2e/staging-setup.ts:272 (waitFor "workspace online")

Root cause — NOT a canvas/test regression and NOT timing fragility. It is
a deterministic consequence of workspace-server #2162 (merged 2026-06-03,
"platform-managed workspace must fail-closed when CP proxy env absent"),
which is a correct production safety fix. The canvas E2E creates a bare
hermes/gpt-4o workspace that defaults closed to platform_managed; on a
staging tenant without MOLECULE_LLM_BASE_URL / MOLECULE_LLM_USAGE_TOKEN,
the agent now aborts at boot with MISSING_PLATFORM_PROXY — surfacing as
the pre-start credential-abort shape (status:"failed", uptime_seconds:0,
no last_sample_error). Pre-#2162 the same workspace booted credential-less
(the bug #2162 fixed) so the old harness happened to pass.

The fix is in the harness, because this test does not need a booted agent:
staging-tabs.spec.ts only opens the 13 side-panel tabs and asserts no hard
crash / no "Failed to load" toast. It makes zero LLM calls and even mocks
/cp/auth/me + 401→200. All it needs is a workspace ROW so the node + tabs
render.

So step 6 now waits for RENDERABLE instead of strictly online:
  - online                                 -> happy path (staging with proxy env)
  - failed + uptime_seconds==0 + no sample -> pre-start credential-abort:
      agent never ran, row still renders -> proceed, with a loud console.warn
  - any other failed (last_sample_error present, OR uptime_seconds>0 i.e.
      the agent started then crashed)      -> still hard-throws (no masking)

Real infra/provision failure stays loud one step earlier at the org level
(instance_status === "failed", unchanged).

Verification: tsc clean for canvas/e2e/staging-* (pre-existing tsc errors
are all in unrelated __tests__ files); `playwright test --list` resolves
globalSetup + the single spec. Full live run needs staging CP creds not
available locally; the changed branch is the globalSetup readiness gate,
verified by inspection against the captured failing-run body.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant