mirror of https://github.com/Gitlawb/openclaude.git synced 2026-08-24 02:34:15 -05:00

Files

T

JATMNandGitHub 9190bd0c50 Harden test isolation and smoke checks (#1440 )

* fix(test): isolate provider-related attribution and preconnect tests

Remove process-global provider mocks from apiPreconnect tests and exercise real env-based provider resolution with hermetic first-party setup.

Reset bootstrap/settings state around attribution tests and reload the attribution module per test so provider and client state cannot leak across suites.

Verification: bun test --max-concurrency=1 src/utils/apiPreconnect.test.ts src/utils/attribution.test.ts

* Fix full local check failures

Add a check script that runs smoke plus the full single-concurrency Bun test suite, and wire it into CONTRIBUTING, the PR template, and PR checks.

Fix Windows/full-suite failures by preferring Git Bash over the WSL bash launcher, normalizing settings paths before source matching, making path and warning-glyph tests platform-aware, and restoring persistent Bun module mocks for AgentTool and hook-chain tests.

Verified with bun test src\tools\BashTool\BashTool.errorOutput.test.ts --max-concurrency=1 and bun run check.

* fix(test): eliminate mock.module() leaks and platform-specific test failures

## Problem

The full test suite (bun test --max-concurrency=1) had 10 failing tests on
Windows. Investigation revealed 4 distinct root causes, all stemming from
bun's mock.module() not being fully reversible by mock.restore(). When a
test file replaces a shared module via mock.module(), stale bindings persist
in already-imported modules even after mock.restore() is called. This is a
known bun limitation.

The CI (Ubuntu) only showed 1 consistent failure (the attribution test),
but the Windows-local failures exposed real bugs that could surface in CI
under different test ordering.

## Changes

### src/utils/hookChains.integration.test.ts (root polluter)

This file was the biggest source of test pollution with 9 mock.module()
calls replacing shared modules (analytics, growthbook, policyLimits,
teammateMailbox, teammate, AgentTool, replBridge, etc.) with partial
surfaces. For example, the teammateMailbox mock only exported writeToMailbox
but the real module has 20+ exports including isIdleNotification,
createIdleNotification, readMailbox, etc. When mock.restore() didn't fully
undo these mocks, downstream tests got undefined for missing exports.

Fix: Import real modules via cache-busted dynamic imports before setting up
mocks, then spread the real module surface into each mock.module() call.
This way even if the mock leaks, downstream tests see the full module
surface with only the intended overrides. All 9 mock.module calls now
spread their real module counterparts.

Also fixed: the test was failing in isolation with SyntaxError because
attachments.ts transitively imports isIdleNotification from
teammateMailbox.js, which was missing from the partial mock.

### src/utils/settings/changeDetector.test.ts (Windows path normalization)

4 tests failed because getSourceForPath() normalizes paths using
path.normalize() which converts forward slashes to backslashes on Windows.
The test hardcoded Unix-style paths (/tmp/openclaude/user/settings.json)
but path.normalize produces \tmp\openclaude\user\settings.json on
Windows. The path comparison always failed, so handleChange() returned
early without triggering any callbacks or debounce timers.

Fix: Import normalize from 'path' and apply it to all test path constants
(pathsBySource, getManagedSettingsDropInDir). This matches what the
production code does.

### src/utils/exportFormats.test.ts (Windows path separator)

resolveExportFilepath() uses path.join() which produces backslash-separated
paths on Windows. The test expected forward-slash paths.

Fix: Import join from 'path' and use it in the expected value so the
assertion is platform-agnostic.

### src/utils/file.test.ts (growthbook mock leak)

importFileModuleWithKillswitchEnabled() mocked growthbook.js with only
getFeatureValue_CACHED_MAY_BE_STALE: () => killswitchEnabled. When
killswitchEnabled was false, this poisoned isAgentSwarmsEnabled() for all
downstream tests because agentSwarmsEnabled.ts has a static import of
getFeatureValue_CACHED_MAY_BE_STALE that captured the mock binding.

Fix: Import the real growthbook module and spread it into the mock, so
all exports remain available even if the mock leaks.

### src/utils/plugins/officialMarketplaceStartupCheck.test.ts (same pattern)

Same growthbook mock leak pattern. Top-level mock.module with only
getFeatureValue_CACHED_MAY_BE_STALE: () => true.

Fix: Import real growthbook module and spread into mock.

### src/tools/AgentTool/AgentTool.teammateModel.test.ts (transitive mock binding)

4 tests failed with 'Agent Teams is not yet available on your plan' because
isAgentSwarmsEnabled() returned false. The function checks
getFeatureValue_CACHED_MAY_BE_STALE('tengu_amber_flint', true) from
growthbook.js, but the static import binding in agentSwarmsEnabled.ts was
captured from a leaked mock that returned false.

Cache-busting the AgentTool.js import doesn't help because
agentSwarmsEnabled.ts is a transitive dependency that keeps its
already-loaded (mocked) growthbook binding.

Fix: Add mock.module for agentSwarmsEnabled.js in importAgentToolWithSpawnMock()
to pin isAgentSwarmsEnabled to true, matching the test's intent (it sets
CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1).

## Verification

- bun run smoke: passes
- bun test --max-concurrency=1: 3019 pass, 0 fail (verified twice)
- No skipped tests (test.skip/it.skip/describe.skip), no test.todo,
  no flaky markers, no test exclusions in config

## Known remaining risks

6 test files still have partial mock.module() calls on providers.js
(withRetry, officialRegistry, domainCheck, conversationRecovery, fastMode)
that don't spread the real module. These don't cause failures under current
test ordering but are latent risks if bun changes file execution order.

* Fix remaining provider mock leak risks

Address the known remaining risks from 7583157 by making provider mocks in withRetry, officialRegistry, and fastMode tests spread and restore the real providers module surface.

Verified with the targeted provider-mock test group and bun run check.

* Harden smoke test coverage

Remove the CI-only skip and unrelated error swallowing from the SDK query lifecycle tests so fork/resume behavior is asserted in CI and local runs.

Isolate test-suite global state by disabling built-in SDK agents for the lifecycle test, restoring MACRO presence exactly, clearing the agent cache, restoring axios mocks, and protecting xAI loopback tests from proxy/fetch leakage.

Make the provider API test script run serially to match the shared env/proxy mutation surface.

Validation: bun run check; CI=1 bun test tests\\sdk\\query-lifecycle.test.ts --max-concurrency=1; bun run test:provider; npm run test:provider-recommendation; bun run security:pr-scan -- --base upstream/main; bun run web:typecheck; bun run web:build; python -m pytest -q -p no:cacheprovider python/tests.

* Expose hidden SDK test failures

Tighten SDK test drains so they only suppress expected lifecycle abort errors instead of swallowing arbitrary init and bootstrap failures.

Replace no-op test assertions with real checks and add V2 lifecycle isolation for MACRO, built-in agents, and agent cache state.

Fix SDK V2 sendMessage to fast-exit when the caller-provided AbortController is already aborted, preventing aborted sessions from submitting work and producing result messages.

Validation: bun test scripts\\feature-flags-source-guard.test.ts tests\\sdk\\query-concurrency.test.ts tests\\sdk\\sdk-v2-lifecycle.test.ts --max-concurrency=1; bun test tests\\sdk\\query-concurrency.test.ts tests\\sdk\\query-lifecycle.test.ts tests\\sdk\\sdk-v2-lifecycle.test.ts --max-concurrency=1; bun run check.

* Fix CI smoke test failures

Respect SDK context null session project directories so regenerated SDK sessions do not fall back to global project state.

Isolate attribution tests from CI provider/model environment and replace nondeterministic live query permission checks with direct assertions against the SDK permission machinery.

Validation: bun test src\\utils\\attribution.test.ts tests\\sdk\\query-lifecycle.test.ts tests\\sdk\\permissions.test.ts --max-concurrency=1; bun test tests\\sdk\\sdk-context-isolation.test.ts tests\\sdk\\query-concurrency.test.ts --max-concurrency=1; bun run check.

* Stabilize attribution contract test

Assert that includeCoAuthoredBy emits the default co-author trailer without pinning the active provider's model label, which can legitimately differ in CI provider environments.

Validation: bun test src\\utils\\attribution.test.ts --max-concurrency=1; ANTHROPIC_MODEL=claude-sonnet-4-5-20250929 CLAUDE_CODE_USE_BEDROCK=1 bun test src\\utils\\attribution.test.ts --max-concurrency=1; bun run check.

2026-05-30 14:40:33 +08:00

3.1 KiB

Raw Blame History

Contributing to OpenClaude

Thanks for contributing.

OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.

Before You Start

Search existing issues and discussions before opening a new thread.
Use issues for confirmed bugs and actionable feature work.
Use discussions for setup help, ideas, and general community conversation.
For larger changes, open an issue first so the scope is clear before implementation.
For security reports, follow SECURITY.md.

Local Setup

Install dependencies:

bun install

Build the CLI:

bun run build

Smoke test:

bun run smoke

Full local check:

bun run check

Run the app locally:

bun run dev

If you are working on provider setup or saved profiles, useful commands include:

bun run profile:init
bun run dev:profile

Development Workflow

Keep PRs focused on one problem or feature.
Avoid mixing unrelated cleanup into the same change.
Preserve existing repo patterns unless the change is intentionally refactoring them.
Add or update tests when the change affects behavior.
Update docs when setup, commands, or user-facing behavior changes.

Validation

At minimum, run the most relevant checks for your change.

Common checks:

bun run check

Focused tests:

bun test ./path/to/test-file.test.ts

When working on provider/runtime setup, this can also help:

bun run doctor:runtime

Pull Requests

Good PRs usually include:

a short explanation of what changed
why it changed
the user or developer impact
the exact checks you ran

If the PR touches UI, terminal presentation, or the VS Code extension, include screenshots when useful.

If the PR changes provider behavior, mention which provider path was tested.

Code Style

Follow the existing code style in the touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files just because they are nearby.
Keep comments useful and concise.

Provider Changes

OpenClaude supports multiple provider paths. If you change provider logic:

be explicit about which providers are affected
avoid breaking third-party providers while fixing first-party behavior
test the exact provider/model path you changed when possible
call out any limitations or follow-up work in the PR description
if you are adding or changing descriptor-era integrations, start with docs/integrations/overview.md
use the focused how-to guides under docs/integrations/how-to/ for new vendors, gateways, models, anthropic proxies, and /usage support

Community

Please be respectful and constructive with other contributors.

Maintainers may ask for:

narrower scope
focused follow-up PRs
stronger validation
docs updates for behavior changes

That is normal and helps keep the project reviewable as it grows.

3.1 KiB Raw Blame History