Feature/clean hooks2#260
Merged
Merged
Conversation
…ot paths On Windows, dunce::canonicalize resolves parent-dir components without requiring intermediate directories to exist, so scope/a/b/c/../../../../ could resolve inside the scope. Fix canonicalize_with_fallback to skip the parent shortcut when file_name() is None (path ends in ..) and fall through to normalize_path_lexically. Also suppress Windows-only warnings: unused anyhow::Result in pty_exec.rs, unused listener_kind in bridge.rs, unused bash param and dead exit_code_of in test files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ts on Windows Windows timer resolution is ~15.6ms, making tokio::time::sleep() with 10ms intervals unreliable in CI. Replace all timing-sensitive keepalive tests with tokio::time::advance() under start_paused=true — zero real time, no scheduler jitter. Key pattern: yield_now() before advance() so the spawned task's first poll creates its sleep() BEFORE the clock moves, otherwise the sleep registers after the advancement and fires one interval later. Add tokio test-util feature to ahma_common dev-dependencies. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…mcp_with_scope is_nested_sandbox_environment() returns true on Windows and macOS+Cursor, causing the sandbox to be created in Test mode. SandboxMode::Test disables validate_path scope checking entirely, making path-security tests vacuous — the nested-parent-segments escape test passed the wrong value through. The confusion: is_nested_sandbox_environment() guards OS-level kernel enforcement (seatbelt/Landlock/AppContainer), which is a separate concern from application-level path validation. Use Strict unconditionally here so validate_path enforces scope bounds on all platforms. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… coordinate timeouts, gate pending-capability test
We kept fixing one Windows failure per CI round because the root issues
were systemic, not per-test:
1. CI fails fast — nextest cancels remaining tests at the first failure, so
each 20-min Windows round revealed only ONE failure. Add --no-fail-fast so
a single run surfaces the COMPLETE failure set.
2. Uncoordinated timeout layers — in-test loop deadlines scale x4 on Windows
(SseStream 120s -> 480s) but nextest's per-test hard-kill backstop is a
fixed 360s. So a slow Windows handshake was force-killed mid-wait and
presented as an opaque TIMEOUT[360s] with no diagnostic. Fix:
- roots_handshake_timeout() now uses SandboxReady (60s -> 240s on Windows,
safely under 360s) instead of SseStream, in both duplicated helpers.
- Document the invariant as ahma_common::timeouts::NEXTEST_CI_HARD_KILL_SECS
and add a guard unit test asserting every bounded category fires before
the backstop on the Windows multiplier — so this class can't silently
recur. SseStream is the documented sole exception (request ceilings only).
3. Genuine platform-capability gap tested unconditionally —
red_team_command_write_escape_blocked asserts OS-level write blocking that
on Windows depends on AppContainer spawn isolation (SPEC R6.3.3, still
pending; create_appcontainer_command is a stub). The test is correct but
cannot pass on Windows yet. Gate it #[cfg_attr(windows, ignore)] — Linux
(Landlock) and macOS (Seatbelt) still run and enforce it — and correct the
CLAUDE.md checklist that wrongly claimed it was done on Windows.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ndows pipe-corruption bug) ROOT CAUSE of the Windows handshake hang (test_tool_call_before_roots_handshake): the deferred-sandbox subprocess locked its sandbox but its notifications/sandbox/configured never reached the bridge, so the client waited forever for a notification that was silently dropped. notifications/sandbox/configured was the ONLY stdout message written outside rmcp's serialized transport. emit_sandbox_notification opened a second, unsynchronized OS handle to the same subprocess->bridge pipe and wrote via writeln! (three separate WriteFile syscalls: "\n", json, "\n"). Windows pipes give no cross-handle multi-write atomicity, so those bytes interleaved with concurrent rmcp writes — notably the keepalive ping that fires the instant the sandbox locks — corrupting the line. The bridge's line reader failed to parse it and dropped it. On Unix, per-write() atomicity hid the bug, so it passed there by luck. Fix: - Add emit_sandbox_notification_via_peer: send sandbox lifecycle notifications through the same rmcp peer transport that already carries roots/list, ping, and tool responses (serialized behind the transport mutex). Mirrors the existing send_enhanced_heartbeat CustomNotification pattern. - Route configured (success) and the failed emits in the roots-config flow through the peer. The Linux Landlock fatal-exit path keeps the direct write (process is aborting; no async-transport dependency wanted there). - Harden emit_stdout_notification: build the framed message once and issue a SINGLE write_all instead of writeln!'s three writes, so the remaining raw fallback users (e.g. shutdown 'terminated') are atomic on Windows too. This is the real product bug behind the prior commit's symptom. That commit's timeout coordination converted the opaque 360s nextest hard-kill into a clean 241s diagnostic failure, which is what made this root cause observable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.