Skip to content

Feature/clean hooks2#260

Merged
paulirotta merged 7 commits into
mainfrom
feature/clean-hooks2
Jun 16, 2026
Merged

Feature/clean hooks2#260
paulirotta merged 7 commits into
mainfrom
feature/clean-hooks2

Conversation

@paulirotta

Copy link
Copy Markdown
Owner

No description provided.

paulirotta and others added 7 commits June 16, 2026 06:35
…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>
@paulirotta paulirotta merged commit 9fe07fa into main Jun 16, 2026
12 checks passed
@paulirotta paulirotta deleted the feature/clean-hooks2 branch June 16, 2026 10:07
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