Skip to content

test(agent-mesh): make approval timeout test deterministic - #3839

Open
Buriro Chimodoi Ezekia (buriro-ezekia) wants to merge 2 commits into
microsoft:mainfrom
buriro-ezekia:test/fix-windows-approval-timeout-flake
Open

test(agent-mesh): make approval timeout test deterministic#3839
Buriro Chimodoi Ezekia (buriro-ezekia) wants to merge 2 commits into
microsoft:mainfrom
buriro-ezekia:test/fix-windows-approval-timeout-flake

Conversation

@buriro-ezekia

Copy link
Copy Markdown

Related Issue

No dedicated issue. The affected regression was introduced as part of #3106.

Problem & Solution

Problem: TestCallbackTimeoutDeprecation.test_timeout_always_denies relied on time.sleep(0.005) and a subsequent time.monotonic() delta. On Windows, the monotonic clock in the local test environment reported a resolution of 15.625 ms, so a 5 ms sleep could still produce an observed elapsed time of 0.0. With timeout_seconds=0, the test could therefore return the callback's approval instead of exercising the timeout-denial branch.

This made the security regression test intermittently fail even though the implementation remained fail-closed when an elapsed timeout was actually observed.

Solution: replace the real-clock sleep with deterministic monotonic values in the test. The test patches the exact CallbackApproval.request_approval function-global clock for the duration of the test, so it always observes a positive elapsed interval and verifies the intended system:timeout denial without depending on host timer granularity.

Production approval code is unchanged.

Impact on Your Work

The flaky test blocked a clean AgentMesh approval-test baseline on Windows and made repeated local verification unreliable. This change preserves the existing security assertion while making the regression deterministic and portable across platforms.

Timeline

None.

Alternatives Considered

  • Increase the sleep duration. This would reduce the likelihood of failure but would still depend on operating-system scheduling and clock resolution, while making the test slower.
  • Patch time.monotonic through a separately imported module alias. This was tested first, but the consolidated package layout can load the implementation through a different module object in editable or local installations. Patching the function's own globals targets the clock actually resolved by the implementation.
  • Change the production timeout comparison. This was rejected because the failure was caused by the test's timing assumption rather than by the production fail-closed behaviour.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Maintenance (dependency updates, CI/CD, refactoring)
  • Security fix

Package(s) Affected

Core & runtime:

  • agent-governance-toolkit-core
  • agent-primitives
  • agent-os
  • agent-mesh
  • agent-runtime
  • agent-sre
  • agent-compliance

Governance & security:

  • agent-mcp-governance
  • agent-rag-governance
  • agent-sandbox
  • agent-discovery
  • agt-policies
  • policy-engine

Platform & tooling:

  • agent-hypervisor
  • agent-lightning
  • agent-marketplace
  • agent-governance-toolkit-cli
  • agent-governance-toolkit-integrations
  • agent-governance-toolkit-protocols
  • agentmesh-integrations (framework integrations)

CLI plugins:

  • agent-governance CLI plugins (copilot-cli / claude-code / opencode / antigravity-cli)

Shared / other:

  • schemas
  • action (GitHub Action)
  • examples
  • docs / root

Testing

Unit Testing

  • AgentMesh approval-focused suite: 76 passed.
  • The previously flaky timeout-denial regression now passes with a deterministic clock.
  • Three existing package-consolidation deprecation warnings remain unchanged.

Manual Testing

Verified locally on Windows with Python 3.12:

  • python -m pip checkNo broken requirements found.
  • python -m ruff check agent-governance-python/agent-mesh/tests/test_approval_strict.pyAll checks passed!
  • Working tree clean after verification.

Checklist

  • I have linked a related issue above, or completed "Problem & Solution", "Impact on Your Work", and "Alternatives Considered"
  • My code follows the project style guidelines (ruff check)
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass (pytest)
  • I have updated documentation as needed
  • I have signed the Microsoft CLA

The full AgentMesh package suite has not been run locally; the focused approval suite passed cleanly. The CLA checkbox is intentionally left open for the repository's CLA bot to confirm.

Attribution & Prior Art

  • This contribution does not contain code copied or derived from other projects without attribution
  • Any external projects that inspired this design are credited in code comments or documentation
  • If this PR implements functionality similar to an existing open-source project, I have listed it below

Prior art / related projects (if any):

None.

AI Assistance

  • I can explain every meaningful change in this PR: what it does, why, and what tradeoffs were considered
  • I have run tests and verification appropriate for this change
  • No part of this PR was autonomously submitted by an AI agent without my review
  • I have not used AI to generate review comments on others' PRs

ChatGPT was used to help diagnose the Windows timer-resolution failure and draft the deterministic test change. The exact diff was reviewed and the validation commands were run locally before submission.

IP, Patents, and Licensing

  • This contribution does not implement patent-pending or patent-encumbered techniques
  • This contribution does not require an NDA or licensing agreement to understand or use
  • Any AI tools used have terms compatible with the MIT License

Signed-off-by: Buriro Chimodoi Ezekia <93260631+buriro-ezekia@users.noreply.github.com>
Signed-off-by: Buriro Chimodoi Ezekia <93260631+buriro-ezekia@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@github-actions

Copy link
Copy Markdown

Welcome to the Agent Governance Toolkit! Thanks for your first pull request.
Please ensure tests pass, code follows style (ruff check), and you have signed the CLA.
See our Contributing Guide.

@github-actions github-actions Bot added tests agent-mesh agent-mesh package size/S Small PR (< 50 lines) labels Aug 26, 2026
@github-actions

Copy link
Copy Markdown

PR Review Summary

Check Status Details
🔍 Code Review ⚠️ Missing No current-run comment
🛡️ Security Scan ⚠️ Missing No current-run comment
🔄 Breaking Changes ⚠️ Missing No current-run comment
📝 Docs Sync ⚠️ Missing No current-run comment
🧪 Test Coverage ⚠️ Missing No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

@buriro-ezekia

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-mesh agent-mesh package size/S Small PR (< 50 lines) tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant