Skip to content

Use async context manager for initializing gateway#7

Merged
crivetimihai merged 1 commit intomainfrom
fix-connection-close
May 28, 2025
Merged

Use async context manager for initializing gateway#7
crivetimihai merged 1 commit intomainfrom
fix-connection-close

Conversation

@madhav165
Copy link
Copy Markdown
Collaborator

No description provided.

@crivetimihai crivetimihai merged commit f66a5e9 into main May 28, 2025
0 of 3 checks passed
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 14, 2025
Use async context manager for initializing gateway

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 14, 2025
Use async context manager for initializing gateway
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 16, 2025
Use async context manager for initializing gateway
Signed-off-by: Vicky Kuo <vicky.kuo@ibm.com>
yiannis2804 added a commit to yiannis2804/mcp-context-forge that referenced this pull request Feb 19, 2026
…BM#7)

Address code review suggestion from @jonpspri:

Problem: The _has_permission static method handles *, admin.*, and exact
matches but had no dedicated tests covering edge cases.

Solution:
- Added 5 comprehensive wildcard permission tests
- Tests cover: exact match, *, namespace.*, multiple permissions
- Discovered and fixed bug: admin.* incorrectly matched 'admin'

Bug Fixed:
- admin.* should only match admin.SOMETHING (e.g., admin.system_config)
- It should NOT match just 'admin' (no dot)
- Fixed by removing 'required == prefix' check
- Now correctly requires dot: required.startswith(prefix + '.')

Test Coverage:
✅ test_exact_match - exact permission matching
✅ test_wildcard_star_matches_all - * matches everything
✅ test_namespace_wildcard - admin.* matches admin.anything
✅ test_wildcard_does_not_match_namespace_only - admin.* ≠ admin
✅ test_multiple_permissions - combined permission lists

All 21 PolicyEngine tests passing (16 existing + 5 new).

Related: PR IBM#2682 Phase 1 Code Review Item IBM#7
Signed-off-by: yiannis2804 <yiannis2804@gmail.com>
crivetimihai pushed a commit that referenced this pull request Feb 24, 2026
)

Address code review suggestion from @jonpspri:

Problem: The _has_permission static method handles *, admin.*, and exact
matches but had no dedicated tests covering edge cases.

Solution:
- Added 5 comprehensive wildcard permission tests
- Tests cover: exact match, *, namespace.*, multiple permissions
- Discovered and fixed bug: admin.* incorrectly matched 'admin'

Bug Fixed:
- admin.* should only match admin.SOMETHING (e.g., admin.system_config)
- It should NOT match just 'admin' (no dot)
- Fixed by removing 'required == prefix' check
- Now correctly requires dot: required.startswith(prefix + '.')

Test Coverage:
✅ test_exact_match - exact permission matching
✅ test_wildcard_star_matches_all - * matches everything
✅ test_namespace_wildcard - admin.* matches admin.anything
✅ test_wildcard_does_not_match_namespace_only - admin.* ≠ admin
✅ test_multiple_permissions - combined permission lists

All 21 PolicyEngine tests passing (16 existing + 5 new).

Related: PR #2682 Phase 1 Code Review Item #7
Signed-off-by: yiannis2804 <yiannis2804@gmail.com>
aidbutlr referenced this pull request in aidbutlr/mcp-context-forge Mar 3, 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.

2 participants