test(mcp): pin the auth-tool expiresAt to the effective wait timeout - #2616
test(mcp): pin the auth-tool expiresAt to the effective wait timeout#2616xpzouying wants to merge 1 commit into
Conversation
Both suites construct the tool with a 100ms timeout override, but the assertions only bounded expiresAt by the 15-minute default — and the lower bound compared against Date.now() taken after the run, which turns into a flake the moment the test body outlives the override. Bound by the window around the run instead: expiresAt must equal emit time + the effective timeout.
|
|
Closing — this is a test-only precision tweak; folding it into a future substantive PR instead of spending review bandwidth on a standalone one. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d21c0f95b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // The deadline is absolute (emit time + the effective wait timeout — the | ||
| // 100ms override here, not the 15-minute default), so hosts render from | ||
| // data instead of mirroring the engine constant. Bounding by the time | ||
| // window around the run keeps the assertion exact without depending on | ||
| // how long the test body takes. |
There was a problem hiding this comment.
This added explanatory comment is inside a test body, but the package guidance for agent-core-v2 only allows comments in the top-of-file header and says not to place them beside functions, methods, or statements. Please move this rationale into the file header or make the assertions self-explanatory without an inline comment.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L15-L17
Useful? React with 👍 / 👎.
1 participant
Related Issue
Follow-up to #2609 — tightens the tests it added; no separate issue.
Problem
The v1/v2 auth-tool suites construct the tool with a
timeoutMs: 100override, but theexpiresAtassertions added in #2609 only bound the value by the 15-minute default — they pass even if the implementation ignored the override. Worse, the lower bound compares againstDate.now()taken after the run completes, so the assertion becomes flaky the moment the test body takes longer than the 100ms override.What changed
Test-only. Both suites capture
before = Date.now()ahead of the run and assertexpiresAt ∈ [before + 100, Date.now() + 100]— pinning the deadline to emit time + the effective wait timeout exactly, with bounds that hold regardless of how long the test body takes.No changeset: tests only.
Verification
Both auth-tool suites pass (5 tests each);
oxlintclean.