Skip to content

test(cursor): isolate cursor-models shared-cache tests to stop parallel-run flake - #1268

Merged
heavygee merged 1 commit into
tiann:mainfrom
heavygee:fix/cursor-models-shared-cache-test-isolation
Jul 31, 2026
Merged

test(cursor): isolate cursor-models shared-cache tests to stop parallel-run flake#1268
heavygee merged 1 commit into
tiann:mainfrom
heavygee:fix/cursor-models-shared-cache-test-isolation

Conversation

@heavygee

Copy link
Copy Markdown
Collaborator

Summary

Fixes a flaky failure in the CLI test suite:

cursorModels.test.ts > buildCursorModelsSeedPayload > inherits cliModelSkus from shared cache when ACP snapshot has none
AssertionError: expected undefined to deeply equal [ 'gpt-5.5-medium', 'gpt-5.5-high' ]

The four cursorModels* test files share one on-disk cache path ($HAPI_HOME/cache/cursor-models.json, defaulting to /tmp/hapi when HAPI_HOME is unset). Two of them already isolate it - cursorModelsStaleLock.test.ts (PID-namespaced home) and handlers/cursorModels.test.ts (unique temp home with save/restore) - but cursorModels.test.ts and cursorModelsSharedCache.test.ts do not.

Under vitest's parallel file execution, cursorModelsSharedCache's afterEach(_resetSharedCursorModelsCacheForTests) rmSyncs that shared file between the other file's write and read, so the read returns null and cliModelSkus is undefined. It passes in isolation and fails at random in the full parallel suite.

Root cause (deterministic proof)

Both un-isolated files resolve to the same path with HAPI_HOME unset; one file's reset deletes the other's write:

after A write, read.cliModelSkus = [ 'gpt-5.5-medium', 'gpt-5.5-high' ]
after B reset (rmSync same path), A reads = null   => cliModelSkus undefined

Parallel scheduling just decides whether B's reset lands inside A's write->read window.

Fix

Give the two un-isolated files their own mkdtemp HAPI_HOME at module load, matching the pattern the other two already use. mkdtempSync guarantees a unique dir regardless of worker-process reuse, so no two files collide.

Post-fix, the same interleaving leaves A's write intact:

A reads after B reset = [ 'gpt-5.5-medium', 'gpt-5.5-high' ]   => intact

Test-isolation only; no product/behavior change (source cursorModelsSharedCache.ts is untouched).

Test plan

  • bun typecheck clean (cli + web + hub)
  • Full cli suite: cursorModels* green across 2 runs + 25 targeted parallel runs of all four cache files
  • Deterministic pre/post interleaving proof (above)
  • Diff is exactly the two test files

Note: the pre-existing 1-minute runner.integration.test.ts > should detect version mismatch and kill old runner failure is environment-dependent and unrelated to this change (no runner code touched).

The four cursorModels* CLI test files share one on-disk cache path
($HAPI_HOME/cache/cursor-models.json, defaulting to /tmp/hapi when
HAPI_HOME is unset). Two files already isolate it (cursorModelsStaleLock
via a PID-namespaced home; handlers/cursorModels via a unique temp home),
but cursorModels.test.ts and cursorModelsSharedCache.test.ts do not.

Under vitest's parallel file execution, cursorModelsSharedCache's
afterEach(_resetSharedCursorModelsCacheForTests) rmSyncs that shared file
between the other file's write and read, so the read returns null and
"inherits cliModelSkus from shared cache" fails with
`expected undefined to deeply equal [...]`. Passes in isolation; fails at
random in the full parallel suite.

Give both un-isolated files their own mkdtemp HAPI_HOME at module load so
each test file's cache path is unique regardless of worker-process reuse.

Fixes #101

Co-authored-by: Cursor <cursoragent@cursor.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Clean up each per-file temporary HAPI home — both new mkdtempSync calls create roots outside the suite-managed tmpHome, while the added files only remove the cache file. The global teardown removes only its own tmpHome, so every run leaves these directories behind. Save/restore HAPI_HOME and recursively remove each per-file root in afterAll. Evidence: cli/src/modules/common/cursorModels.test.ts:10, cli/src/modules/common/cursorModelsSharedCache.test.ts:14.

    Suggested fix:

    import { afterAll, afterEach, describe, expect, test } from "vitest"
    import { mkdtempSync, rmSync } from "node:fs"
    
    const previousHapiHome = process.env.HAPI_HOME
    const testHapiHome = mkdtempSync(join(tmpdir(), "hapi-cursor-models-"))
    process.env.HAPI_HOME = testHapiHome
    
    afterAll(() => {
        if (previousHapiHome === undefined) delete process.env.HAPI_HOME
        else process.env.HAPI_HOME = previousHapiHome
        rmSync(testHapiHome, { recursive: true, force: true })
    })

Summary

Review mode: initial. One minor test-isolation cleanup issue found.

Testing

Not run (automation); repository CI test check was pending at review time.

HAPI Bot

// Isolate the on-disk cursor-models cache to this file's own HAPI_HOME so
// parallel vitest workers don't race on the shared $HAPI_HOME/cache path
// (see heavygee/hapi#101).
process.env.HAPI_HOME = mkdtempSync(join(tmpdir(), 'hapi-cursor-models-'))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MINOR] Remove the per-file temp home after the suite

mkdtempSync creates a new root on every run, but afterEach only removes cache/cursor-models.json; the global teardown removes only the separate suite tmpHome. This root therefore remains in the system temp directory. The same issue exists at cli/src/modules/common/cursorModelsSharedCache.test.ts:14.

Suggested fix:

const previousHapiHome = process.env.HAPI_HOME
const testHapiHome = mkdtempSync(join(tmpdir(), "hapi-cursor-models-"))
process.env.HAPI_HOME = testHapiHome

afterAll(() => {
    if (previousHapiHome === undefined) delete process.env.HAPI_HOME
    else process.env.HAPI_HOME = previousHapiHome
    rmSync(testHapiHome, { recursive: true, force: true })
})

@heavygee

Copy link
Copy Markdown
Collaborator Author

Hey @tiann - went ahead and merged this one myself: trivial test-only isolation fix (2 test files, +16 lines, zero source/runtime change). Thinking I could take low-impact PRs like this off your plate so only the meatier stuff needs your eyes - that work for you? Tell me if this is me 越俎代庖 and I'll happily back off.

@heavygee
heavygee merged commit 743c0a0 into tiann:main Jul 31, 2026
2 checks passed
@tiann

tiann commented Jul 31, 2026 via email

Copy link
Copy Markdown
Owner

heavygee added a commit that referenced this pull request Jul 31, 2026
…tes (#1269)

Follow-up to #1268. The per-file `mkdtempSync`/`join(tmpdir(), ...)` HAPI
homes in the cursor-models test cluster were never removed, so every run
leaked a directory into the system temp dir (afterEach only cleared the
cache file inside them). Save/restore HAPI_HOME and recursively remove
each per-file temp root in teardown across all three cursor-models test
files (cursorModels, cursorModelsSharedCache, and the stale-lock test that
had the same pattern). No source/runtime change.

Co-authored-by: Cursor <cursoragent@cursor.com>
heavygee added a commit to heavygee/hapi that referenced this pull request Jul 31, 2026
… upstream jump

Upstream jumped (743c0a0..c7afc50, incl tiann#1268) and the driver soup
re-materialized with new conflict shapes:

- 67-unseen-count-sessionchat.patch: a soup layer now inserts
  onCancelLoadMore between onLoadMore and unseenCount in SessionChat.tsx;
  add it as hunk context so git apply -3 stops 3-way conflicting.
- 68-router-import-picker-fixups.patch (new): the feat/agent-session-import-picker
  rerere merge dropped the local CodexImportIcon() icon fn and the SessionsPage
  const queryClient = useQueryClient(); heal re-adds both (5 tsc errors).

Co-authored-by: Cursor <cursoragent@cursor.com>
heavygee added a commit to heavygee/hapi that referenced this pull request Jul 31, 2026
Add pr-merge-policy lib + Meta WAIT TIANN / SELF-MERGE queue sections after
tiann's tiann#1268 low-impact blessing. Agents still prepare-only; promote via
low-impact/self-merge-ok labels or allowlist.

Co-authored-by: Cursor <cursoragent@cursor.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.

2 participants