Skip to content

build(sandbox): pin state-based ACP silence watchdog + E2E version diagnostic fix - #2898

Merged
lukemarsden merged 4 commits into
mainfrom
fix/acp-silence-watchdog-state-based
Jul 30, 2026
Merged

build(sandbox): pin state-based ACP silence watchdog + E2E version diagnostic fix#2898
lukemarsden merged 4 commits into
mainfrom
fix/acp-silence-watchdog-state-based

Conversation

@lukemarsden

Copy link
Copy Markdown
Collaborator

Pins ZED_COMMIT=e36d6f47ee. Paired Zed PR: (linked below; opened this first per the ordering rule in CLAUDE.md.)

Two follow-ups to the wedge work in #2896, both prompted by the E2E claude-round failure seen during that merge.

1. Judge agent silence by thread state, not a tool-duration guess

The watchdog shipped in #2896 disarmed permanently on the agent's first event. That covers the production wedge (prompt accepted, zero events ever) but misses the other shape — streamed some output, then stalled before completing. That shape is real: the E2E claude round produced a correct answer and then never sent message_completed.

The obvious fix, a second longer idle timeout, needs a constant for "the longest plausible tool call". That number is unknowable and wrong the first time someone runs a 40-minute build.

So the watchdog now asks the thread whether anything is actually outstanding: has_outstanding_work() is true while any tool call is Pending / InProgress / WaitingForConfirmation. Busy threads are exempt for exactly as long as the work genuinely takes — no duration constant anywhere — and silence is only judged when nothing is running. Both shapes then fall out of one rule:

generating + no events for budget + nothing outstanding ⇒ wedged

Because the budget no longer has to cover tool calls, only model think-time between consecutive events, a single modest 120s default is defensible.

2. Fix the E2E agent-version diagnostic

run_e2e.sh reported the agent version with npm view @anthropic-ai/claude-agent-acp — a package that 404s. Zed installs @agentclientprotocol/claude-agent-acp, which today resolves to 0.63.0 — the same version that was wedged in the production container.

The wrong scope silently returned unknown, disabling the one signal that distinguishes "we regressed" from "the agent package changed under us". Now queries the correct scope, warns loudly if it cannot resolve, and states explicitly that the install is unpinned.

Validation

  • cargo check --package zed --features external_websocket_sync — clean
  • cargo test -p external_websocket_sync — 56 passed (new test asserts outstanding tool calls are never judged silent)
  • Drift sweep — 28/28
  • E2E on this exact tree — running; this PR will not be merged until it is green

Still open

The npm install remains unpinned, so the claude round is not reproducible across time. Tracked in design/2026-07-29-acp-agent-silent-prompt-wedge.md.

🤖 Generated with Claude Code

Bumps ZED_COMMIT to e36d6f47ee, which replaces the first-event-only silence
watchdog with a state-based one, and repairs the E2E agent-version diagnostic.

The original watchdog disarmed permanently on the agent's first event, so it
caught the production wedge (prompt accepted, zero events) but missed the shape
seen in the E2E claude round (correct answer streamed, then no message_completed).
Rather than hard-code a second timeout sized to the longest plausible tool call,
busy-ness is now read from thread state: a Pending/InProgress/WaitingForConfirmation
tool call is exempt for as long as the work genuinely takes, and silence is only
judged when nothing is outstanding.

Also updates the incident write-up for both fixes.
@cursor

cursor Bot commented Jul 30, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@lukemarsden

Copy link
Copy Markdown
Collaborator Author

Paired Zed PR: helixml/zed#74

Merge order: Zed #74 first, then this one.

An unpinned install makes the claude round non-reproducible, but pinning is the
wrong fix. Production auto-installs the latest @agentclientprotocol/claude-agent-acp
in every desktop container, and tracking latest is essential rather than incidental
— the Anthropic API and Claude Code move fast, and keeping up with them is the point
of this integration. Pinning CI would make CI test something we do not ship, and
would convert a real signal (agent-package regression) into silence.

The remedy is attribution plus resilience: log the resolved agent version so a
failure can be attributed, and harden Zed against a misbehaving agent rather than
assuming a well-behaved one. Notes the practical consequence for flake triage.
Re-pins to 473a1c7cc2 (helixml/zed main, merge of PR #74) rather than the
intermediate branch commit the PR was opened with. The merge commit is
content-identical to the tree the E2E validated (git diff f5c77cf39a..473a1c7cc2
is empty), so the validation transfers exactly.

Carries, on top of the silence watchdog already pinned by #2896:
- interrupt cancels are now targeted, so a cancel racing the creation loop can
  no longer kill a newer turn (root cause of intermittent Phase 17 failures, and
  of a user pressing stop-then-retype landing in a stuck interaction)
- the silence watchdog judges by thread state rather than a tool-duration guess
- three E2E observability repairs: broken grep -c counters, a missing
  turn-lifecycle/cancel-ordering dump, and a round filter that discarded real
  Helix interaction ids while reporting them as 'wrong agent'
@lukemarsden
lukemarsden merged commit b68429e into main Jul 30, 2026
5 checks passed
@lukemarsden
lukemarsden deleted the fix/acp-silence-watchdog-state-based branch July 30, 2026 11:08
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