Skip to content

test: fix timeouts#3

Merged
rhuanbarreto merged 5 commits into
mainfrom
fix-tests
Feb 23, 2026
Merged

test: fix timeouts#3
rhuanbarreto merged 5 commits into
mainfrom
fix-tests

Conversation

@rhuanbarreto

@rhuanbarreto rhuanbarreto commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR fixes two root causes of test failures observed in CI and codifies the fixes as governance rules in ARCH-005.

1. MCP server event loop retention (Linux hang)

McpServer from @modelcontextprotocol/sdk internally creates an AjvJsonSchemaValidator (backed by ajv + ajv-formats) that holds references keeping Bun's event loop alive after all tests complete. On Linux this caused bun test to hang indefinitely — all tests passed but the process never exited. On macOS the event loop drained normally, making this a CI-only failure that was hard to reproduce locally.

Fix: Moved McpServer instantiation from individual test bodies into beforeEach/afterEach hooks across all MCP test files, with await server.close() called in afterEach to release internal references. Applied to 7 test files: resources, server, tools, check, list-adrs, review-context, session-context.

2. Git identity missing in temp repos (CI commit failures)

Tests that created a temporary git repo and ran git commit passed locally (where developers have a global git identity) but failed silently in CI with a cryptic ShellPromise error — the root cause being that CI runners have no global user.email/user.name configured.

Fix: Added explicit git config user.email and git config user.name to the git-files test immediately after git init, before any commit is made.

3. CI safety nets

Added timeout-minutes to all workflow jobs (code-pull-request.yml, release.yml, release-binaries.yml) to cap any future event loop or test hangs at the job level. Also set a per-test timeout via bun test --timeout 60000 in package.json so individual slow tests fail fast rather than blocking the suite indefinitely.

4. ADR and agent memory updates

Both fixes are codified in ARCH-005 (Testing Standards):

  • New Do's: close SDK instances in afterEach; configure local git identity before git commit in temp repos
  • New Don'ts: don't leave SDK instances open; don't rely on global git config in CI
  • New good/bad code examples for both patterns
  • New risk entry documenting third-party SDK event loop retention on Linux

Agent memory (MEMORY.md) updated with both patterns for future sessions.

Test plan

  • bun run validate passes locally (lint → typecheck → format → test → ADR check)
  • CI validate job completes without hanging on Linux
  • All MCP tests pass and the process exits cleanly
  • git-files test passes in CI

@rhuanbarreto rhuanbarreto merged commit 410fa13 into main Feb 23, 2026
2 of 3 checks passed
@rhuanbarreto rhuanbarreto deleted the fix-tests branch February 24, 2026 22:51
rhuanbarreto added a commit that referenced this pull request Feb 28, 2026
* test: fix timeouts

* fix: add git credentials to test

* docs: capture learnings

* ci: update workflow name to comply with org setup

* refactor: format fix
rhuanbarreto added a commit that referenced this pull request Mar 3, 2026
Resolves code scanning alerts #1, #2, #3 (actions/missing-workflow-permissions)
by adding `permissions: contents: read` to the three workflows that lacked
a top-level permissions block, following the principle of least privilege.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rhuanbarreto added a commit that referenced this pull request Mar 3, 2026
fix(ci): add explicit permissions to workflows missing them

Resolves code scanning alerts #1, #2, #3 (actions/missing-workflow-permissions)
by adding `permissions: contents: read` to the three workflows that lacked
a top-level permissions block, following the principle of least privilege.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@archgatebot archgatebot Bot mentioned this pull request Mar 3, 2026
rhuanbarreto added a commit that referenced this pull request Mar 3, 2026
* test: fix timeouts

* fix: add git credentials to test

* docs: capture learnings

* ci: update workflow name to comply with org setup

* refactor: format fix
rhuanbarreto added a commit that referenced this pull request Mar 3, 2026
fix(ci): add explicit permissions to workflows missing them

Resolves code scanning alerts #1, #2, #3 (actions/missing-workflow-permissions)
by adding `permissions: contents: read` to the three workflows that lacked
a top-level permissions block, following the principle of least privilege.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
rhuanbarreto added a commit that referenced this pull request Mar 4, 2026
* test: fix timeouts

* fix: add git credentials to test

* docs: capture learnings

* ci: update workflow name to comply with org setup

* refactor: format fix
rhuanbarreto added a commit that referenced this pull request Mar 4, 2026
fix(ci): add explicit permissions to workflows missing them

Resolves code scanning alerts #1, #2, #3 (actions/missing-workflow-permissions)
by adding `permissions: contents: read` to the three workflows that lacked
a top-level permissions block, following the principle of least privilege.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Mar 4, 2026
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.

1 participant