diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index 221934f..3e34de0 100644 --- a/SPECS/ARCHIVE/INDEX.md +++ b/SPECS/ARCHIVE/INDEX.md @@ -1,6 +1,6 @@ # mcpbridge-wrapper Tasks Archive -**Last Updated:** 2026-02-20 (P14-T2_Align_release_metadata_and_changelog_for_0.4.0) +**Last Updated:** 2026-02-20 (P14-T5_Stabilize_broker_Unix-socket_permission_test_against_path-length_limits) ## Archived Tasks @@ -134,6 +134,7 @@ | P14-T3 | [P14-T3_Reconcile_declared_Python_support_with_tested_matrix/](P14-T3_Reconcile_declared_Python_support_with_tested_matrix/) | 2026-02-20 | PASS | | P14-T4 | [P14-T4_Replace_deprecated_setuptools_license_metadata_with_SPDX_format/](P14-T4_Replace_deprecated_setuptools_license_metadata_with_SPDX_format/) | 2026-02-20 | PASS | | P14-T2 | [P14-T2_Align_release_metadata_and_changelog_for_0.4.0/](P14-T2_Align_release_metadata_and_changelog_for_0.4.0/) | 2026-02-20 | PASS | +| P14-T5 | [P14-T5_Stabilize_broker_Unix-socket_permission_test_against_path-length_limits/](P14-T5_Stabilize_broker_Unix-socket_permission_test_against_path-length_limits/) | 2026-02-20 | PASS | ## Historical Artifacts @@ -233,6 +234,7 @@ | [REVIEW_P14-T3_python_support_matrix.md](_Historical/REVIEW_P14-T3_python_support_matrix.md) | Review report for P14-T3 | | [REVIEW_P14-T4_spdx_license_metadata.md](_Historical/REVIEW_P14-T4_spdx_license_metadata.md) | Review report for P14-T4 | | [REVIEW_P14-T2_release_metadata_changelog.md](_Historical/REVIEW_P14-T2_release_metadata_changelog.md) | Review report for P14-T2 | +| [REVIEW_P14-T5_broker_socket_path_limit.md](_Historical/REVIEW_P14-T5_broker_socket_path_limit.md) | Review report for P14-T5 | ## Archive Log @@ -421,3 +423,5 @@ | 2026-02-20 | P14-T4 | Archived REVIEW_P14-T4_spdx_license_metadata report | | 2026-02-20 | P14-T2 | Archived Align_release_metadata_and_changelog_for_0.4.0 (PASS) | | 2026-02-20 | P14-T2 | Archived REVIEW_P14-T2_release_metadata_changelog report | +| 2026-02-20 | P14-T5 | Archived Stabilize_broker_Unix-socket_permission_test_against_path-length_limits (PASS) | +| 2026-02-20 | P14-T5 | Archived REVIEW_P14-T5_broker_socket_path_limit report | diff --git a/SPECS/ARCHIVE/P14-T5_Stabilize_broker_Unix-socket_permission_test_against_path-length_limits/P14-T5_Stabilize_broker_Unix-socket_permission_test_against_path-length_limits.md b/SPECS/ARCHIVE/P14-T5_Stabilize_broker_Unix-socket_permission_test_against_path-length_limits/P14-T5_Stabilize_broker_Unix-socket_permission_test_against_path-length_limits.md new file mode 100644 index 0000000..3ad439c --- /dev/null +++ b/SPECS/ARCHIVE/P14-T5_Stabilize_broker_Unix-socket_permission_test_against_path-length_limits/P14-T5_Stabilize_broker_Unix-socket_permission_test_against_path-length_limits.md @@ -0,0 +1,58 @@ +# PRD — P14-T5: Stabilize broker Unix-socket permission test against path-length limits + +## 1. Context + +`pytest -q` can fail on macOS with: + +- `OSError: AF_UNIX path too long` + +The failure occurs in `TestSocketPermissions.test_socket_created_with_0600_permissions`, where the test binds a Unix socket under `tmp_path / "broker.sock"`. On systems where pytest's temporary directory path is long, the resulting AF_UNIX path exceeds platform limits. + +## 2. Objective + +Make the socket-permission test deterministic across environments while preserving the original behavior check (`0600` permissions). + +## 3. Deliverables + +- Update `tests/unit/test_broker_transport.py` so socket-permission testing uses a short, deterministic socket base path under `/tmp`. +- Keep existing permission assertion unchanged in meaning (`0o600`). +- Add cleanup handling for temporary directories used by the test fixture/helper. + +## 4. Dependencies + +- FU-P13-T12 (socket permission/security behavior already implemented) + +## 5. Implementation Plan + +1. Add a dedicated helper for short socket paths in tests (or update existing test helper) using `tempfile.mkdtemp(dir="/tmp", prefix="mcp...")`. +2. Route `TestSocketPermissions.test_socket_created_with_0600_permissions` through that helper. +3. Ensure helper uses `try/finally` cleanup to avoid residue in `/tmp`. +4. Keep socket permission assertion logic exactly as before. + +## 6. Acceptance Criteria + +- `tests/unit/test_broker_transport.py::TestSocketPermissions::test_socket_created_with_0600_permissions` passes with default pytest temp paths. +- Full `pytest -q` passes without `--basetemp` workaround. +- Test still verifies mode equals `0o600`. + +## 7. Validation + +Required quality gates per FLOW: + +- `pytest` +- `ruff check src/` +- `mypy src/` +- `pytest --cov` (coverage >= 90%) + +Plus explicit check: + +- `pytest -q tests/unit/test_broker_transport.py::TestSocketPermissions::test_socket_created_with_0600_permissions` + +## 8. Out of Scope + +- Runtime broker socket path behavior in production code. +- Changes to non-test socket configuration defaults. + +--- +**Archived:** 2026-02-20 +**Verdict:** PASS diff --git a/SPECS/ARCHIVE/P14-T5_Stabilize_broker_Unix-socket_permission_test_against_path-length_limits/P14-T5_Validation_Report.md b/SPECS/ARCHIVE/P14-T5_Stabilize_broker_Unix-socket_permission_test_against_path-length_limits/P14-T5_Validation_Report.md new file mode 100644 index 0000000..8e34492 --- /dev/null +++ b/SPECS/ARCHIVE/P14-T5_Stabilize_broker_Unix-socket_permission_test_against_path-length_limits/P14-T5_Validation_Report.md @@ -0,0 +1,45 @@ +# Validation Report — P14-T5 + +**Task:** P14-T5 — Stabilize broker Unix-socket permission test against path-length limits +**Date:** 2026-02-20 +**Verdict:** PASS + +## Scope + +- Ensure Unix-socket permission regression test remains stable on macOS path limits. +- Preserve existing assertion that broker socket file mode is `0o600`. + +## Implemented Changes + +- Updated `tests/unit/test_broker_transport.py`: + - Added a short-directory test setup via `tempfile.mkdtemp(dir="/tmp", prefix="mcpb")`. + - Kept permission assertion semantics unchanged (`mode == 0o600`). + - Added cleanup with `shutil.rmtree(..., ignore_errors=True)`. + +## Validation Commands + +1. `pytest -q tests/unit/test_broker_transport.py::TestSocketPermissions::test_socket_created_with_0600_permissions` +- Result: PASS + +2. `pytest` +- Result: PASS (`626 passed, 5 skipped`) + +3. `ruff check src/` +- Result: PASS (`All checks passed!`) + +4. `mypy src/` +- Result: PASS (`Success: no issues found in 18 source files`) + +5. `pytest --cov` +- Result: PASS (`626 passed, 5 skipped`) +- Coverage: `91.33%` (threshold: `>= 90%`) + +## Acceptance Criteria Check + +- [x] `TestSocketPermissions.test_socket_created_with_0600_permissions` passes with default pytest temp paths. +- [x] Full `pytest -q`/`pytest` passes without AF_UNIX path overflow failures. +- [x] Test still verifies socket mode is exactly `0o600`. + +## Notes + +- Existing warning output from `websockets` deprecations remains unchanged and is unrelated to this task. diff --git a/SPECS/ARCHIVE/_Historical/REVIEW_P14-T5_broker_socket_path_limit.md b/SPECS/ARCHIVE/_Historical/REVIEW_P14-T5_broker_socket_path_limit.md new file mode 100644 index 0000000..e078601 --- /dev/null +++ b/SPECS/ARCHIVE/_Historical/REVIEW_P14-T5_broker_socket_path_limit.md @@ -0,0 +1,30 @@ +## REVIEW REPORT — Broker Socket Path Limit + +**Scope:** origin/main..HEAD +**Files:** 6 + +### Summary Verdict +- [x] Approve +- [ ] Approve with comments +- [ ] Request changes +- [ ] Block + +### Critical Issues +- None. + +### Secondary Issues +- None. + +### Architectural Notes +- The fix is intentionally test-only and does not modify runtime broker behavior. +- Using a short `/tmp` path in the socket-permissions test removes environment-dependent AF_UNIX path overflow failures while preserving the original security assertion (`0600` mode). + +### Tests +- `pytest` passed (`626 passed, 5 skipped`). +- `ruff check src/` passed. +- `mypy src/` passed. +- `pytest --cov` passed with `91.33%` coverage (>=90% threshold). + +### Next Steps +- No follow-up tasks required. +- Proceed to ARCHIVE-REVIEW. diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index b64dcbf..ac7a49a 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -2,8 +2,8 @@ ## Recently Archived +- **P14-T5** — Stabilize broker Unix-socket permission test against path-length limits (2026-02-20, PASS) - **P14-T2** — Align release metadata and changelog for 0.4.0 (2026-02-20, PASS) -- **P14-T4** — Replace deprecated setuptools license metadata with SPDX format (2026-02-20, PASS) ## Suggested Next Tasks diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index fd2c093..a28b851 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -2311,6 +2311,36 @@ Phase 9 Follow-up Backlog ### Phase 14: Release 0.4.0 Readiness +#### ✅ P14-T5: Stabilize broker Unix-socket permission test against path-length limits — Completed (2026-02-20, PASS) +- **Description:** Make the socket-permission regression test deterministic across local environments by avoiding Unix domain socket path overflows in pytest temporary directories, while preserving verification of `0600` permissions. +- **Priority:** P1 +- **Dependencies:** FU-P13-T12 +- **Parallelizable:** yes +- **Outputs/Artifacts:** + - Updated `tests/unit/test_broker_transport.py` socket-permission test setup to use a safe short socket path + - Validation evidence that `pytest -q` passes without requiring `--basetemp` +- **Acceptance Criteria:** + - [x] `tests/unit/test_broker_transport.py::TestSocketPermissions::test_socket_created_with_0600_permissions` passes on macOS with default pytest temp paths + - [x] Full `pytest -q` passes without `AF_UNIX path too long` + - [x] Test still verifies socket mode is exactly `0o600` + +--- + +#### ⬜️ FU-P14-T5-1: Add macOS CI execution for broker socket-path regression coverage +- **Description:** Extend GitHub Actions CI with a macOS test path (similar to dedicated workflow lanes such as DocC) so the AF_UNIX path-length-sensitive broker socket test is exercised on macOS runners during PR validation. +- **Priority:** P1 +- **Dependencies:** P14-T5 +- **Parallelizable:** yes +- **Outputs/Artifacts:** + - Updated `.github/workflows/ci.yml` (or a dedicated workflow) to run broker transport tests on `macos-latest` + - CI documentation note describing why macOS coverage is required for AF_UNIX path-length behavior +- **Acceptance Criteria:** + - [ ] GitHub Actions runs the broker socket permission/path regression test on a macOS runner for pull requests + - [ ] The macOS job status is visible in PR checks and gates merges on failure + - [ ] Existing Linux test matrix behavior remains unchanged + +--- + #### ✅ P14-T1: Bound per-session ID restore maps in broker transport — Completed (2026-02-20, PASS) - **Description:** Prevent unbounded memory growth in long-lived broker sessions by pruning/removing alias/restore entries once responses are routed (and define safe behavior when local ID space wraps). - **Priority:** P1 diff --git a/tests/unit/test_broker_transport.py b/tests/unit/test_broker_transport.py index 4246b16..2213969 100644 --- a/tests/unit/test_broker_transport.py +++ b/tests/unit/test_broker_transport.py @@ -21,9 +21,11 @@ import asyncio import json import os +import shutil import socket import stat import struct +import tempfile import time from typing import Any from unittest.mock import AsyncMock, MagicMock, patch @@ -1106,13 +1108,17 @@ class TestSocketPermissions: """Socket file must be created with 0600 permissions (FU-P13-T12).""" @pytest.mark.asyncio - async def test_socket_created_with_0600_permissions(self, tmp_path: Any) -> None: + async def test_socket_created_with_0600_permissions(self) -> None: """After start(), the socket file has owner-only read/write permissions.""" - server = _make_server(tmp_path) - await server.start() + short_dir = tempfile.mkdtemp(dir="/tmp", prefix="mcpb") + server = _make_server(short_dir) try: - socket_path = tmp_path / "broker.sock" - mode = stat.S_IMODE(socket_path.stat().st_mode) - assert mode == 0o600, f"Expected 0600, got {oct(mode)}" + await server.start() + try: + socket_path = os.path.join(short_dir, "broker.sock") + mode = stat.S_IMODE(os.stat(socket_path).st_mode) + assert mode == 0o600, f"Expected 0600, got {oct(mode)}" + finally: + await server.stop() finally: - await server.stop() + shutil.rmtree(short_dir, ignore_errors=True)