Commit Graph

3 Commits

Author SHA1 Message Date
anhtahaylove
fde6241d41 test: close the database before removing temp DATA_DIR (#13290) (#13292)
* test: close the database before removing temp DATA_DIR (#13290)

Tests that set their own DATA_DIR and removed it in test.after() failed on
Windows with EPERM: nothing closed the SQLite connection, so the directory
still had an open handle and the -shm/-wal sidecars kept it locked. maxRetries
could not help because every retry hit the same open handle.

Adds tests/_setup/tempDataDir.ts with cleanupTempDataDir()/createTempDataDir(),
which close the DB singleton (lazily imported, so tests that never touch the
database do not pull in the DB layer) and then remove the directory
best-effort. Applies it to the five suites confirmed failing.

The helper's own test proves the ordering matters: skipping the close makes it
fail with 'cleanup must remove the directory'.

* test: close the database before removing temp DATA_DIR (15 more suites)

Converts the suites that measurably emitted EPERM during a full run to the
shared cleanupTempDataDir helper from #13292.

Measured on the same 15 files:
  base   -> 22 fail, 40 EPERM lines
  branch ->  7 fail, 10 EPERM lines

The 7 remaining failures are pre-existing and unrelated to teardown:
rtk-learn-discover-routes and executor-map-golden already fail on a clean
base (6 and 3 failures respectively).

* test: close the database before removing temp DATA_DIR (final 9 suites)

Completes the #13290 sweep. Two teardown shapes needed the helper:

- after()/t.after() hooks that removed DATA_DIR directly
- beforeEach() hooks that wiped DATA_DIR between tests while the previous
  test's connection was still open. These failed *before* the test body ran,
  so every test in the file reported the same EPERM path.

Three of them already called core.resetDbInstance() right before rmSync and
still leaked, which is the product-side connection leak tracked in #13303.

Measured per file, EPERM lines now 0 across all nine. Remaining failures are
pre-existing on a clean base (firefly 4->1, driverFactory 1, responses-* 1
each) and unrelated to teardown.

* test: add the missing cleanupTempDataDir import to two responses suites

The previous commit swapped rmSync for cleanupTempDataDir in these two files but
did not add the import, so both suites died with
ReferenceError: cleanupTempDataDir is not defined before running any test.

responses-parse-once-4041:            0 pass / 1 fail -> 4 pass / 0 fail
responses-route-early-keepalive-wiring: 0 pass / 1 fail -> 3 pass / 0 fail

Both now report 0 EPERM.

* test: close SQLite handles in three silently-leaking suites

These three suites requested DATA_DIR cleanup but the delete failed on
Windows because a SQLite connection was still open. They pass today, so
the leak is invisible: they carry state between tests and would surface
later as an unrelated-looking assertion, as #13303 already did in the
Firefly suite (a 500 instead of a 401).

agentbridge-mitm-router-key-6403 and agent-bridge-bypass-flow removed
their own temp dir in test.after() without closing the DB first; both now
use the shared cleanupTempDataDir helper, which closes the singleton
before removing the directory.

issue-agent-route-execution is a different case: it has no teardown at
all, so the connection stayed open until process exit and the
isolateDataDir cleanup hook then hit EPERM. It now closes the DB in
test.after().

Verified with a probe on fs.rmSync: all three reported a failed delete
before, and zero across three consecutive runs after, while the same
probe still reports four leaks in the Firefly suite.

* test: remove temp DATA_DIR in five suites that never cleaned up

These five suites create their own mkdtemp DATA_DIR, open the SQLite DB and
never remove the directory, so every run leaves a storage.sqlite behind in the
OS temp dir. Each dir is private to its suite, so this leaked disk space rather
than corrupting results - but the churn is pointless.

Each now closes the DB and removes its directory through the shared
cleanupTempDataDir helper.

Verified with an exit-time probe that lists storage.sqlite* still present in
DATA_DIR: it fired for these suites before the change and is silent after,
with the same test counts (22/14/5/3/3 passing).
2026-09-17 02:31:53 -03:00
Diego Rodrigues de Sa e Souza
3d4f3e4960 test(infra): retry recursive temp-dir removal instead of failing a shard on ENOTEMPTY (#11966) (#11968)
* test(infra): retry recursive temp-dir removal instead of failing a shard on ENOTEMPTY (#11966)

Two shards on release/v3.8.51 went red in one day with the same signature —
"ENOTEMPTY, Directory not empty: /tmp/omniroute-<test>-XXXXXX" — from
combo-same-provider-cascade (Unit Tests fast-path 4/4, on a PR that touches only
.github/) and auth-policy-embeddings-webfetch-7785 (the 20k-test TIA step). Both pass
alone and on re-run: the cleanup races something still writing into the directory
(SQLite WAL/-shm checkpoint, a worker, the backup) and under a loaded hosted runner
the window opens. 1154 test files do their own cleanup with
fs.rmSync(dir, { recursive: true, force: true }); 57 already asked for retries.

One-shot codemod (scripts/ad-hoc/codemod-rm-maxretries.mjs, kept for the record):
every rm / rmSync / rmdirSync option object with `recursive: true` and no
`maxRetries` gains `maxRetries: 5, retryDelay: 100` — Node itself then retries
ENOTEMPTY/EBUSY/EPERM for up to ~0.5 s before giving up. 2243 call sites in 1292
files under tests/, the shared tests/_setup/isolateDataDir.ts exit hook included.
Only the option object changes: no call site, assertion or import is touched.

Validation: prettier and ESLint (with the frozen suppressions) clean on all 1292
files; a random 20-file sample runs green (quota-redis-store hangs identically on
the untouched tree — it needs a Redis on localhost, an environment matter). The
four unit shards on this PR are the full run.

* fix(quality): let check-forgotten-sibling-tests read a 1,000-file diff

The gate shells out to `git diff` through execFileSync with Node's default 1 MB
maxBuffer; the 1,292-file codemod in this PR is the first diff large enough to
overflow it, and the gate died with `spawnSync git ENOBUFS` before comparing
anything. 64 MB is far above any real PR and costs nothing when unused.
2026-08-29 01:17:40 -03:00
stanley
4540d303d7 fix(oauth): send required CLI headers in claude-auth import bootstrap call (#10144)
* fix(oauth): send required CLI headers in claude-auth import bootstrap call

enrichWithBootstrap() in claudeAuthImport.ts was missing the
User-Agent and anthropic-beta headers that the two other callers of
the same /api/claude_cli/bootstrap endpoint (claudeIdentity.ts and
src/lib/oauth/providers/claude.ts) always send. Without them,
Anthropic doesn't recognize the request as coming from a CLI client
and the bootstrap call fails, silently returning a null identity
(accountUUID/organizationUUID/organizationType all null).

createConnectionFromAuthFile()'s identity-verification refusal then
gets bypassed via overwriteExisting: true (the only way imports
currently succeed, since first attempts fail with
identity_unverified because of this same bug), so every imported
Claude connection ends up with unverified identity.

Downstream, resolveAccountUUID() in claudeIdentity.ts falls back to
a hash-derived fake UUID when providerSpecificData.accountUUID is
null. That fake UUID is shape-valid but was never associated with
the real account by Anthropic, so requests carrying it get
classified as unrecognized third-party traffic and routed to the
separate extra-usage pool instead of the account's plan limits --
producing an intermittent (~50% observed) 400:
"Third-party apps now draw from your extra usage, not your plan
limits." on an otherwise perfectly valid, imported subscription
token.

Fixes the header mismatch so bootstrap succeeds and imported
connections get a real, Anthropic-recognized account identity from
the start, same as connections created via the native OAuth flow.

Fixes #10143

* fix(oauth): persist cliUserID device identity on claude-auth import

createConnectionFromAuthFile() in claudeAuthImport.ts never set
providerSpecificData.cliUserID, unlike the native OAuth setup flow in
src/lib/oauth/providers/claude.ts which always mints one. cliUserID is
read by resolveCliUserID() (open-sse/executors/claudeIdentity.ts) as
the request's device_id; when absent it falls back to a lazy-random
device id regenerated fresh every process restart (in-memory Map,
process-lifetime only), so every restart of an imported connection
presents as a brand-new device to Anthropic for the same account --
a second, independent contributor (alongside Part 1's bootstrap
header fix in this same PR) to the intermittent third-party-usage 400
on valid imported subscription tokens.

- "create new connection" branch: always mint a fresh cliUserID.
- "update existing connection" branch: preserve any already-persisted
  cliUserID from existing.providerSpecificData (don't rotate a working
  device identity on re-import); only mint a fresh one if absent.

Adds changelog.d/fixes/10144-claude-import-cli-user-id.md per
CONTRIBUTING.md.

Fixes #10143

* test(oauth): cover claude-auth import bootstrap headers + cliUserID persistence

Adds tests/unit/claudeAuthImport-bootstrap-headers-10144.test.ts (Rule #18
regression guard for #10143):

1. enrichWithBootstrap() sends the required CLI headers on the
   /api/claude_cli/bootstrap call — a claude-cli User-Agent (now sourced
   from CLAUDE_CODE_CLIENT_VERSION, matching the two working call-sites)
   and anthropic-beta: oauth-2025-04-20 — and still falls back to null
   identity fields on non-OK upstream responses.
2. createConnectionFromAuthFile() mints a 64-hex cliUserID device
   identity on create, preserves an already-persisted cliUserID on
   overwrite re-import (no rotation), and mints a fresh one when the
   existing connection has none.

Also aligns the hardcoded claude-cli/1.0.0 User-Agent in the import
bootstrap with the version constant the two working call-sites
(claudeIdentity.ts, oauth/providers/claude.ts) already use.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* refactor(oauth): source claude-auth import UA from canonical constant (#10144 review nit)

Addresses the hardcoded-version nit from review: the bootstrap User-Agent was
re-typed as `claude-cli/${CLAUDE_CODE_CLIENT_VERSION}` instead of importing
getClaudeCodeUserAgent() — the single source of truth the two working
call-sites (claudeIdentity.ts, oauth/providers/claude.ts) use.

- claudeAuthImport.ts: use getClaudeCodeUserAgent("cli") for the bootstrap call
- test: import the same canonical helper instead of a local copy of the pinned
  version, and assert the outbound UA byte-for-byte against it, so a future
  version bump can't silently desync the wire identity.

Verified: node --import tsx/esm --test on the new test file -> 5/5 pass;
sibling claudeAuthImport.test.ts -> pass; eslint on both changed files ->
no new findings (only the pre-existing @/lib/localDb barrel-import restriction
on an untouched import line).

* test(oauth): exercise claude auth import implementation

Replace copied helper tests with real implementation coverage for bootstrap headers and persistent cliUserID behavior.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Co-authored-by: stanleytejakusuma <stanleytejakusuma@users.noreply.github.com>
2026-08-17 08:01:45 -03:00