mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 10:14:19 -05:00
c30578819ef79007896330258f0e9002d052fa52
7
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
c30578819e |
diagnostics(query): trace interruption causality (#2111)
* diagnostics(issue-1830): trace interruption causality * test(issue-1830): lock interruption ownership matrix * fix(codex): preserve stream deadline contract * fix(diagnostics): harden interruption trace lifecycle Refs #1830 * fix(diagnostics): harden interruption trace settlement Refs #1830 * fix(diagnostics): preserve interruption causality * fix(diagnostics): address interruption trace review * fix(diagnostics): preserve tracing observer contracts * fix(diagnostics): preserve interruption trace contracts * test(permissions): cover interactive hook interrupts |
||
|
|
a3dc345f12 |
test(user): restore real modules from a pre-mock snapshot (#2031)
* test(user): restore real modules from a pre-mock snapshot
This suite's teardown re-installed its own mocks instead of undoing them.
`import * as realExeca from 'execa'` is a live namespace binding, and
mock.module repoints it. By the time afterEach ran, `realExeca` WAS the
mock, so `mock.module('execa', () => realExeca)` reinstalled the stub -- and
mock.module lasts for the life of the process, so every test file loaded
afterwards got it.
The stub returns { exitCode, stdout } with no stderr, which is what made it
visible elsewhere: collectTaskReportGitMetadata does
`inside.stderr.trim()` and threw "undefined is not an object". The two
task-report CLI handler tests and the two /ads command tests failed on any
run where this file happened to be ordered before them, which is why the
same four went red on unrelated PRs and intermittently on main itself
(
|
||
|
|
701b68c215 |
fix(query): prevent spurious Windows interruption prompt by passing 'interrupt' reason (#1733)
* fix(query): prevent spurious Windows interruption prompt by passing 'interrupt' reason
* fix(cli): pass 'interrupt' abort reason and add stop-hook regression tests
* test(query): exercise handleStopHooks abort-reason branch directly
Adds focused coverage that imports and runs handleStopHooks() while a
Stop-hook generator is being consumed, asserting abort('interrupt')
suppresses the synthetic '[Request interrupted by user]' message and a
default abort still yields it. Closes jatmn's P2 regression request.
|
||
|
|
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
|
||
|
|
f12eb1c9e8 |
Harden test isolation for smoke stability (#1192)
* Fix flaky smoke build checks Replace feature-flag build preprocessing with a Bun onLoad transform so smoke/build no longer rewrites tracked src files while tests may be reading them. Keep telemetry stubs ahead of the feature transform in both CLI and SDK builds, preserve non-empty text token counts in hybrid context splitting, and make the corrupted Orama stress test assert against the actual project directory used by the test. Verified with bun run smoke, bun test src/utils/hybridContextStrategy.test.ts, bun test src/utils/knowledgeGraph.stress.test.ts --rerun-each 3, and bun test --max-concurrency=1. * Harden KnowledgeGraph smoke stress isolation Give each KnowledgeGraph stress test its own temporary config directory and remove it during teardown so Orama, SQLite, and corrupted-file state cannot bleed between stress cases or later PR test runs. Reviewed at least 20 open PRs and found the recurring smoke-and-tests failure cluster is the full unit suite, especially KnowledgeGraph corrupted Orama recovery. Verified with bun test src/utils/knowledgeGraph.stress.test.ts --rerun-each 5, bun test --max-concurrency=1, and bun run smoke. * Harden smoke test isolation Audit and harden broad smoke-adjacent test suites for process-global leaks, including env/config restoration, shared registry/module mock cleanup, fetch/axios/mock restoration, and global MACRO/platform/sandbox mutations. Replace fragile render sleeps in interactive tests with output-driven waits, and isolate provider/model/profile tests behind the shared mutation lock so unrelated PRs do not inherit stale process state. Make SQLite knowledge graph cleanup clear closed on-disk databases before best-effort file cleanup, with coverage for the stale database reset path. Verified: bun run smoke; bun test --max-concurrency=1; python -m pytest -q python/tests; bun run security:pr-scan -- --base origin/main; bun run test:provider; npm run test:provider-recommendation. * Harden test isolation across smoke suite Guard process-global test mutations with the shared mutation lock across env, module mock, config cache, and storage tests.\n\nDeep-copy global config snapshots, restore transient globals precisely, and make plugin/LSP mocks expose compatible export surfaces so concurrent test loading does not poison unrelated suites.\n\nReplace fixed SDK cleanup sleeps with call polling to remove timing sensitivity.\n\nVerification:\n- bun test --max-concurrency=1\n- bun run smoke\n- python -m pytest -q python/tests\n- bun run test:provider\n- npm run test:provider-recommendation\n- bun run security:pr-scan -- --base origin/main\n- git diff --check * Close remaining test global-state leaks Guard remaining cache, plugin, console, and VS Code module-mock tests with the shared mutation lock.\n\nThis follow-up audit covers non-env process-global state that can leak across test files: tool schema cache, cache stats tracker state, plugin loader caches, console.error replacement, and VS Code mock.module usage.\n\nVerification:\n- leak-surface scans for env/global/mock.module/cache outliers\n- duplicate top-level mock collision cluster\n- affected tests cluster\n- bun test --max-concurrency=1\n- bun run smoke\n- git diff --check * Guard remaining mock restore cleanup Lock tests that call bun:test mock.restore without installing module mocks themselves.\n\nmock.restore is process-global, so these cleanup hooks can still tear down another test file's active module mocks when files run concurrently.\n\nVerification:\n- expanded leak scans for env, globals, module mocks, mock.restore, timers, argv, and caches\n- bun test src/components/useCodexOAuthFlow.test.tsx src/services/github/deviceFlow.test.ts\n- bun test --max-concurrency=1\n- bun run smoke\n- git diff --check * Harden test isolation for smoke stability Serialize tests that mutate process-global state behind the shared mutation lock, including process.env, transient globals, global config/cache state, storage mocks, and Bun module mocks. Add isolated env mutex instances for SDK mutex tests so timeout coverage no longer manipulates the live process-global mutex. Move top-level mock.module setup behind lock acquisition and restore mocks before releasing locks to prevent cross-file leakage under parallel smoke runs. Verified with: bun test --max-concurrency=1; bun test; bun run smoke. |
||
|
|
1f66d322ad | fix flaky tests in full test run (#1020) | ||
|
|
60c76b6599 |
feat: SDK Runtime — Query Engine, Sessions, and Build Pipeline (#984)
* feat(sdk): add SDK foundation — type declarations, errors, and utilities Adds standalone SDK building blocks with no SDK source dependencies: - sdk.d.ts: ambient type declarations for SDK bundle - coreSchemas.ts + coreTypes.generated.ts: Zod schemas and generated types - errors.ts: SDK-specific error classes - validation.ts: input validation utilities - messageFilters.ts: extracted message filter logic - handlePromptSubmit.ts: imports from messageFilters - 16 generated-types tests * fix(sdk): narrow assertFunction type from broad Function to callable signature Code review finding: assertFunction used `asserts value is Function` which accepts any function-like value without narrowing. Changed to `(...args: any[]) => any` for better type safety. * fix(sdk): update sdk.d.ts header — manually maintained, not generated Reviewer noted the header said "Generated from index.ts" but no generator produces this file. Updated to "Manually maintained — keep in sync with index.ts". Drift detection added in validate-externals.ts (PR 3). * fix(sdk): align sdk.d.ts types with canonical coreTypes.generated.ts Tighten SDK public type contract to resolve reviewer blockers: - PermissionResult: unknown[] → precise 6-shape discriminated union (addRules/replaceRules/removeRules/setMode/addDirectories/removeDirectories) - SDKSessionInfo: snake_case → camelCase (sessionId, lastModified, etc.) - ForkSessionResult: session_id → sessionId - SDKPermissionRequestMessage: uuid + session_id now required - SDKPermissionTimeoutMessage: added uuid + session_id - SessionMessage: parent_uuid → parentUuid - SDKMessage/SDKUserMessage/SDKResultMessage: replaced loose inline definitions with re-exports from coreTypes.generated.ts * feat(sdk): wire existing code modules + SDK shared utilities Modifies core modules for SDK integration: - QueryEngine, tools, state, commands: SDK type hooks - SDK shared utilities (shared.ts, permissions.ts) - 21 SDK tests (shared-utils, permissions) Stack: main ← pr1-foundation ← pr2-sdk-core * feat(sdk): add snake_case ↔ camelCase key mapping utilities casing.ts provides recursive key transformation for the SDK boundary layer. Internal runtime uses snake_case; public API exposes camelCase. Will be used by shared.ts, sessions.ts, query.ts at export boundaries. * test(sdk): add tests for snake_case ↔ camelCase mapping utilities Covers snakeToCamel, camelToSnake, mapKeysToCamel, mapKeysToSnake including nested objects, arrays, null/undefined, and round-trips. * feat(sdk): add SDK runtime — query engine, sessions, build pipeline Completes the SDK implementation: - SDK build target (dist/sdk.mjs) with TUI dependency stubbing - External dependency lists (scripts/externals.ts) - SDK type generation from Zod schemas (scripts/generate-sdk-types.ts) - External validation (scripts/validate-externals.ts) - SDK source: index, query, v2, sessions modules - agentSdkTypes: re-exports SDK functions (query, createSession, etc.) - 136 SDK tests + 7 build scanner tests Stack: main ← pr1-foundation ← pr2-sdk-core ← pr3-sdk-runtime * fix(sdk): align internal SDK types with camelCase public contract shared.ts: SDKSessionInfo, ForkSessionResult, SessionMessage fields now use camelCase matching sdk.d.ts. SDKPermissionRequestMessage and SDKPermissionTimeoutMessage gain required uuid + session_id fields. permissions.ts: onPermissionRequest/onTimeout callbacks now include uuid and session_id in emitted messages. * fix(sdk): update runtime modules to use camelCase field names sessions.ts: toSDKSessionInfo outputs camelCase keys, entryToSessionMessage uses parentUuid, forkSession returns sessionId. query.ts: reads sessionId from listSessions/forkSession results instead of snake_case session_id. * fix(test): update session tests to use camelCase field names session_id → sessionId in forkSession result assertions and getSessionMessages calls. * fix(sdk): prevent permission timeout race condition with once-only resolve wrapper Add createOnceOnlyResolve utility to prevent double-resolution of promises when timeout and host response happen simultaneously. This ensures deterministic behavior in the permission handling flow. * fix(sdk): improve race condition test robustness * fix(sdk): handle consecutive underscores in snakeToCamel conversion Changes: - Use _+([a-z]) regex to match multiple consecutive underscores before letters - Add lookahead (?=. ) to preserve underscore-letter pairs at string end - Handle dunder names (__proto__, __typename) by stripping wrapper and capitalizing - Add tests for consecutive underscores and trailing underscore preservation * fix(sdk): include original error message in permission callback denial When a canUseTool callback throws an error, the catch block now includes the original error message in the denial message, making debugging easier for SDK consumers. * feat(sdk): add optional timeout to env mutex for deadlock prevention Add timeout parameter to acquireEnvMutex() to prevent infinite waits in deadlock scenarios. The timeout is optional and defaults to no timeout (wait forever) for backward compatibility. Returns a MutexAcquireResult object with acquired status and optional timeout reason for failed acquisitions. * fix(sdk): remove timed-out callback from mutex queue to prevent deadlock * test(sdk): add missing error path and timeout scenario tests Add tests for timeout scenarios when host doesn't respond to permission requests, fallback behavior when no onPermissionRequest callback, and MCP connection edge cases for undefined/empty config. * fix(sdk): address code review issues - race conditions, validation, error handling - Add createPermissionTarget() factory that applies onceOnlyResolve at registration time, fixing race condition where timeout and host response could both try to resolve the same promise - Add try-catch to releaseEnvMutex() to prevent permanent lock if callback throws - Extract DEFAULT_PERMISSION_TIMEOUT_MS constant (30 seconds) - Add MCP config validation rejecting null, non-objects, and arrays - Preserve error stack traces in MCP connection failures - Add runtime validation to mapMessageToSDK for null/non-object/invalid type - Update tests to use createPermissionTarget and add validation tests * fix(sdk): syntax fixes and MCP connection error handling - Remove extra closing parenthesis in permissions.ts - Remove extra closing braces in shared.ts type definitions - Wrap MCP connection in try/catch to continue without MCP tools on failure * fix(sdk): syntax fixes, MCP error handling, and logic clarity - Remove extra closing parenthesis in permissions.ts - Remove extra closing braces in shared.ts type definitions - Wrap MCP connection in try/catch to continue without MCP tools on failure - Clarify thinkingConfig logic: use ?? true instead of !== false - Add explanatory comment about thinkingEnabled default behavior - Apply createOnceOnlyResolve wrapper in QueryImpl.registerPendingPermission * fix(sdk): comprehensive error handling and resource cleanup - Add try-catch around injectAgents() to gracefully handle plugin agent tool validation failures (prevents test crashes from unknown 'LS' tool) - Add console.warn logging to agent loading/injection catch blocks for debugging visibility (matches v2.ts pattern) - Add pendingPermissionPrompts.clear() to close() and interrupt() methods in both query.ts and v2.ts to prevent memory accumulation - Add close() method to SDKSession interface and SDKSessionImpl - Wrap MCP connection in query.ts with try-catch (matches v2.ts behavior) - Add timeoutQueue cleanup in finally blocks (query.ts + v2.ts) - Remove error.stack from MCP error messages to prevent internal path leak All 208 SDK tests pass. TypeScript errors are pre-existing. * fix(sdk): address code review non-blocking issues - Add SDKAgentLoadFailureMessage type for agent load failure events - Emit agent definition/injection failures to SDK message stream - Add tool name to permission timeout denial message - Replace 'as any' casts with proper typed state access - Fix supportedCommands to use correct mcp.commands/plugins.commands paths - Update test for correct AppState structure * fix(sdk): address code review blocking and non-blocking issues Blocking Issues Fixed: - MCP cleanup missing on session/query close - now disconnects MCP clients to prevent resource leaks in long-running processes with multiple sessions - Engine reference not cleared on close - now sets _engine = null to prevent memory leaks - Added MCP cleanup tests (9 new tests covering cleanup scenarios) Non-Blocking Issues Fixed: - Removed redundant catch block that just rethrew errors (query.ts) - Fixed inconsistent timeout denial message format (permissions.ts) - Fixed hardcoded tool name 'Bash' in test (permissions.test.ts) - Exported PermissionResolveDecision type for SDK consumers (index.ts) All 217 SDK tests pass. * fix(sdk): address code review type consistency issues - Add close() method to SDKSession interface (documented but missing from type) - Fix SDKSessionInfo, ForkSessionResult, SessionMessage field naming: snake_case → camelCase to match sdk.d.ts public contract and implementation - Add uuid and session_id to SDKPermissionTimeoutMessage for correlation - Fix JSDoc comment in forkSession to use sessionId (not session_id) These changes align internal types (shared.ts) with the public SDK contract (sdk.d.ts) and actual implementation output. The merge from origin/main introduced snake_case types that mismatched camelCase implementation and tests. * fix: restore openclaude.json comment in REPL.tsx Merge |