fix(copilot): address review comments — wait_for_stash fast path, error marker, compat test

- Add fast path in wait_for_stash: check event.is_set() before clearing
  to avoid unnecessary 0.5s timeout when PostToolUse hook completes
  before the streaming loop calls wait_for_stash
- Tighten "failed" error marker to "failed to" in _is_tool_error_or_denial
  to avoid matching benign outputs like "3 tests failed"
- Add max_buffer_size to SDK compat test fields_we_use
This commit is contained in:
Zamil Majdy
2026-02-20 11:18:49 +07:00
parent 3491365b45
commit 3a38b5e9bd
4 changed files with 11 additions and 11 deletions

View File

@@ -569,15 +569,10 @@ async def test_wait_for_stash_already_stashed():
# Stash before waiting — simulates hook completing before message arrives
_stash("Read", "file contents")
# Event is now set; wait_for_stash clears it and re-waits.
# Since the stash already happened, the event won't fire again,
# so this should timeout. But the stash DATA is available.
# Event is now set; wait_for_stash detects the fast path and returns
# immediately without timing out.
result = await wait_for_stash(timeout=0.05)
# Returns False because the event was cleared before waiting and
# no NEW signal arrived. This is expected: the flush will find the
# data in the stash directly — wait_for_stash is only needed when
# the stash hasn't happened yet.
assert result is False
assert result is True
# But the stash itself is populated
assert _pto.get({}).get("Read") == ["file contents"]

View File

@@ -104,6 +104,7 @@ def test_agent_options_accepts_all_our_fields():
"model",
"env",
"resume",
"max_buffer_size",
]
sig = inspect.signature(ClaudeAgentOptions)
for field in fields_we_use:

View File

@@ -459,7 +459,7 @@ def _is_tool_error_or_denial(content: str | None) -> bool:
"maximum", # subtask-limit denial
"denied",
"blocked",
"failed", # internal tool execution failures
"failed to", # internal tool execution failures
'"iserror": true', # MCP protocol error flag
)
)

View File

@@ -169,11 +169,15 @@ async def wait_for_stash(timeout: float = 0.5) -> bool:
event = _stash_event.get(None)
if event is None:
return False
# Clear before waiting so we detect new signals only.
event.clear()
# Fast path: hook already completed before we got here.
if event.is_set():
event.clear()
return True
# Slow path: wait for the hook to signal.
try:
async with asyncio.timeout(timeout):
await event.wait()
event.clear()
return True
except TimeoutError:
return False