Skip to content

🥁 test: Stabilize Host Edit Responsiveness Assertions - #16727

Merged
danny-avila merged 1 commit into
devfrom
lia/skill-edit-timer
Oct 4, 2026
Merged

danny-avila merged 1 commit into
devfrom
lia/skill-edit-timer

Conversation

@lia-by-librechat

@lia-by-librechat lia-by-librechat Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

API shard 4/4 intermittently fails an existing skill-edit assertion across unrelated PRs. Introduced in #16677, the assertion treats a worker reply arriving before setTimeout(0) as timer starvation, even when the edit is correctly rejected without writing.

Separate atomicity from responsiveness: skill handler tests keep real amplification rejection and no-write assertions. Worker lifecycle tests hold a reply pending and check timer progress with both cold and reused workers. Existing real-amplification timer coverage remains intact. No production code changes.

Related to #16707, #16719, #16715.

How it works

handler tests: real amplification → budget error → no writes
worker tests: held reply → timer fires while unsettled → abort and close

Type of change

  • Tests / tooling / CI

Testing

Environment: Node.js 24.16.0, repository npm/Jest configuration.

Automated checks at c935cf5b7530619266cb35f6da01f015e0ee3b1e:

  • Handler and worker suites: 376 tests passed with coverage scoped to handlers.ts, processing.ts and matching.ts.
  • Timer/atomicity regression cases: 4 passed in each of 5 successive runs.
  • npm exec --workspace packages/api -- tsc --noEmit: passed.
  • Touched-file ESLint, Prettier, import ordering and npm run static-checks -- --against origin/dev: passed.
  • Data-provider, data-schemas and API builds: passed.

The default whole-workspace coverage collection hit the local command limit after both suites passed. The scoped coverage run completed successfully. Full suites and Lighthouse were not run locally; this changes tests only.

Investigation: Original handler suite passed locally. A disposable Node inspector set a conditional breakpoint before clearTimeout(timer); 200 repeated original cases passed without hitting it. The CI failure was not reproduced locally.

Screenshots / recordings

No user-facing change.

Risk / compatibility

Test-only. Worker budgets, matching, persistence and runtime behavior are unchanged.

@lia-by-librechat

Copy link
Copy Markdown
Contributor Author

Head: c935cf5

Separates skill-edit atomicity from timer ordering. Adds cold/reused-worker timer checks with a deliberately pending reply. Existing real-amplification timer coverage is unchanged. No production changes.

@lia-by-librechat

Copy link
Copy Markdown
Contributor Author

Verified head: c935cf5

  • 376 focused tests passed with edit-code coverage.
  • 4 timer/atomicity cases passed in each of 5 successive runs.
  • API typecheck, touched-file lint, formatting, import order and static checks passed.
  • Backend package builds passed.

Default whole-workspace coverage collection exceeded the local command limit; scoped coverage completed. Full suites and Lighthouse were not run locally. CI is running.

@lia-by-librechat

Copy link
Copy Markdown
Contributor Author

Reviewed head: c935cf5

Independent review complete: no findings. Confirmed amplification rejection, no-write assertions and existing real-amplification timer coverage remain intact.

CI: all four @librechat/api shards and TypeScript checks passed. Remaining checks are running with no failures reported.

Local evidence: 376 focused tests passed with scoped coverage; four regression cases passed across five runs. API typecheck and touched-file static checks passed. Independent reviewer did not rerun Jest.

@danny-avila
danny-avila force-pushed the lia/skill-edit-timer branch from c935cf5 to 18db092 Compare October 4, 2026 00:02
@danny-avila
danny-avila merged commit 6298bfc into dev Oct 4, 2026
35 checks passed
@danny-avila
danny-avila deleted the lia/skill-edit-timer branch October 4, 2026 01:22
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