Skip to content

Make the connectors test cleanup stable (ENOTEMPTY flake) - #533

Closed
HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:feat/stable-test-cleanup
Closed

Make the connectors test cleanup stable (ENOTEMPTY flake)#533
HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:feat/stable-test-cleanup

Conversation

@HugoXK

@HugoXK HugoXK commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Make the connectors test cleanup stable (ENOTEMPTY flake)

The connectors test flaked: a test's temp home is still written by a background save that outlives dispose(), so rmSync in the cleanup hook sometimes failed with ENOTEMPTY, failing a whole npm run verify / CI run at random. The cleanup now settles and retries, and leaves the temp dir if it is still racing (the assertions have already passed). Test-only.

Ran at the tip: npm run verify (green, TZ=UTC), the connectors suite 5× and the full core suite 2× (all green). Not run: smoke, Desktop, real-model steward.

@HugoXK
Make the connectors test cleanup stable (ENOTEMPTY flake)
e4d3c7f 
A test's temp home can still be written by a background save that
outlives dispose(), so a plain rmSync sometimes raced and failed the run
with ENOTEMPTY. Settle and retry; if it is still racing, leave the temp
dir (the assertions already passed). Test-only.
@github-actions github-actions Bot added the risk:T1 Risk tier T1 (client, tests, generated files), set by the controller from the diff label Oct 20, 2026
@Birfy

Birfy commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

Review (Birfy, 2026-10-20 20:07 Asia/Shanghai): PASS

Tip SHA: e4d3c7f
Ran:

  • node --import ./test/harness-resolve.mjs --test test/connectors.test.js (core/, TZ=UTC) x5 → pass (28/28 each run)
  • node scripts/ds-bot-patches.mjs check → pass
  • npm --prefix core run lint (oxlint + arch-docs) → pass
  • CI tests-ok at this tip → green
    Not run:
  • full npm run verify locally — only core/test/connectors.test.js (and its patch copy) changed; CI ran the full suite at this tip
  • smoke / Desktop — test-only change, no host code touched
    Verdict: PASS

Findings: no actionable findings. Test-only: the raw rmSync cleanups become cleanHome() (settle + retry on ENOTEMPTY/EBUSY, then leave the temp dir). No product behavior changes; the patch change is the mirrored test file. Non-blocking: the root cause (a background save outliving dispose()) is papered over here; if it shows up outside tests it deserves its own bug.

Merge: risk:T1, green, mergeable. Held for now: it touches patches/ds-bot/0001-core-fixes.patch, which open awaiting-approval PRs #515/#494/#493 also change, so it waits until those land (or the owner says to go first). This is not merge authority.

@Birfy Birfy added bug Something isn't working area:automation Scheduled tasks and automation labels Oct 20, 2026
@Birfy Birfy mentioned this pull request Oct 20, 2026
Closed
@Birfy

Birfy commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

@HugoXK Owner decision (2026-10-20 21:09 Asia/Shanghai): accepted (test-only, T1). No change required. It will be queued only after #515 and #493 have merged; both are still open, so it is not queued yet.

@Birfy

Birfy commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

This PR is dirty: it conflicts with main, so it cannot merge as it stands.

Please merge main into the branch (merge, not force-push), regenerate the patches/ds-bot/files/test/connectors.test.js mirror from core/test/connectors.test.js, then re-run npm run verify. The ENOTEMPTY fix itself reads correctly and it is T1/test-only.

@Birfy Birfy mentioned this pull request Oct 21, 2026
Merged
@Birfy

Birfy commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

Superseded by #560: the maintenance round recaptured this on a fresh branch from main (105ec65b) because this PR's head had not moved since 2026-10-20 15:06 and it conflicts with main. The fix is the same cleanHome retry, plus a test that pins the contract.

@Birfy Birfy closed this Oct 21, 2026
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 21, 2026
@Birfy
Make the connectors test cleanup stable (ENOTEMPTY flake) (ThinkFlowL…
4da6a0d 
…ab#560)

Fixes ThinkFlowLab#532

`core/test/connectors.test.js` flaked: a test's temp home can still be
written by a background save that outlives `dispose()`, so the raw
`rmSync` cleanup sometimes failed the whole run with `ENOTEMPTY`. Every
cleanup now goes through `cleanHome`, which settles and retries (5
tries, 50 ms apart, `rmSync` itself with `maxRetries`/`retryDelay`) and
leaves the temp dir when it is still racing — the assertions have
already passed by then. Any other error still surfaces. Because the
failure is a race, a test pins the retry contract instead of waiting for
a flake: ENOTEMPTY/EBUSY settle, a directory that never settles is left
behind, EACCES still rejects.

This recaptures ThinkFlowLab#533 on a fresh branch from `main` (`105ec65b`); that PR
had not moved since 2026-10-20 15:06 and conflicts with `main`, so it is
closed as superseded.

Tier: **T1** (test-only, no product behavior).

Ran at the tip: connectors 29/29 four runs in a row (no flake);
`ds-bot-patches check` (pin v0.1.5, 10 patches, core == ds-bot +
patches); `arch-docs OK (2 changed file(s))`; core lint 0 errors, 41
warnings, none in the touched file. `TZ=UTC`.

Not run: one-shot `npm run verify` (2 GiB container OOMs it), the full
core suite (CI's `tests-ok` covers it), smoke, Desktop.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

area:automation Scheduled tasks and automation bug Something isn't working risk:T1 Risk tier T1 (client, tests, generated files), set by the controller from the diff

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@HugoXK @Birfy