Fix the permission gate, which failed open on every single tool call - #32
Merged
Conversation
Found by running the app rather than the suite. A Thread's timeline held 106
identical lines:
PreToolUse:Bash failed (exit 1) — Tervin hook: Tervin did not answer within 5s.
`HookHandler::decide` was sync while `PermissionArbiter::decide` is async, and
`ArbiterHandler` bridged the two with `Handle::block_on`. That is called from
`serve_one`, which runs inside `tokio::spawn`, and `block_on` panics when called
from within an async context. The task died, the socket closed with no reply,
the client waited out its full 5s timeout and exited 1. Exit 1 is non-blocking,
so the gate failed open every time, silently, while claiming in the UI to be
gating the session. That is the exact failure mode this project exists to
avoid.
The trait is now async, so nothing bridges. The `runtime: Handle` field is gone
rather than moved to a blocking pool, because the bridge was the bug.
Why the suite missed it: `ArbiterHandler` is the only handler Tervin ever
constructs and it was the only one with no test. Every gate test, including the
live one against the real CLI, used a trivial handler that returned a decision
directly and never called `block_on`. Same shape as the BlocksPanel failure the
testing guide describes: the code that ships was the code never exercised.
So two tests now drive the real handler through the real socket and the real
client. Both assert on exit codes rather than on the decision, because exit 1
is the specific signature of "no answer" and a decision assertion would pass
for a timeout. Verified by reintroducing `block_on` and confirming the new test
fails with the identical message from the screenshot.
Also four things that made the app hard to read, all found the same way:
- A run of identical consecutive timeline events collapses to one row with a
count. The information in the hundredth repeat is the number, not the text.
Only consecutive ones merge, so nothing is reordered and an interleaved event
breaks the run, which is asserted.
- A Block's command was `pre-wrap` with `break-word` inside a flex child that
can shrink to nothing, so in a narrow pane it wrapped one character per line.
Now one truncated line with the full text in the tooltip.
- The Plan tab interpolated a Thread's title into a sentence. Titles come from
the first prompt, so it read as gibberish. The Thread is already named in the
header.
- Titles were 80 characters cut mid-word. Now 48, cut at a word boundary, with
whitespace collapsed so a pasted prompt cannot put newlines in a title.
rust 680 to 682, vitest 319 to 321.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
QuintinBotes
enabled auto-merge (squash)
August 2, 2026 23:04
The gate panel listed one line per failed hook run. A broken hook fires once per tool call, so an hour of work produced 59 byte-identical lines and a panel nobody could read. This is the same reasoning the working hooks already had: that code collapses them into "N of your hooks ran" because "four hooks ran fine" is reassurance rather than information. Failures deserve the same treatment for the same reason. Grouped by name, exit code and message, so two genuinely different failures never merge, and in first-seen order so the earliest stays at the top. Separate from the timeline grouping in the previous commit: this is a different panel, which is why fixing one left the other a wall. Co-Authored-By: Claude Opus 5 <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.
Found by running the app, not the suite. A Thread's timeline held 106 identical lines:
The bug
HookHandler::decidewas sync;PermissionArbiter::decideis async.ArbiterHandlerbridged them withHandle::block_on, whichserve_onecalls from insidetokio::spawn. Blocking a runtime worker from within the runtime panics: the task died, the socket closed with no reply, the client waited out its full 5s and exited 1.Exit 1 is non-blocking. So the gate failed open on every tool call, silently, while the UI stated it was gating the session. That is precisely the class of dishonesty this project exists to avoid, so it is the most serious bug found so far.
The trait is async now, so nothing bridges. The
runtime: Handlefield is deleted rather than moved to a blocking pool, because the bridge was the bug.Why the suite missed it
ArbiterHandleris the only handler Tervin ever constructs, and it was the only one with no test. Every gate test, including the live one against the real CLI, used a trivial handler that returned a decision directly and never calledblock_on. Same shape as theBlocksPanelfailuredocs/TESTING.mdopens with: the code that ships was the code never exercised.Two tests now drive the real handler through the real socket and the real client, asserting on exit codes rather than decisions, because exit 1 is the signature of "no answer" and a decision assertion passes for a timeout. One covers deny and defer; one covers 6 concurrent calls, since
serve_onespawns per connection specifically so a slow decision cannot delay the next, and nothing asserted that.Verified the guard fires: reintroducing
block_onfails the new test with the identical message from the screenshot.Four things that made the app unreadable
All found the same way, by using it:
×N. The information in the hundredth repeat is the count. Only consecutive identical events merge, so nothing is reordered and an interleaved event breaks the run, which is asserted.pre-wrapplusbreak-wordon a flex child that can shrink to nothing. Now one truncated line, full text in the tooltip and the expanded body.682 rust, 321 vitest, clippy clean, fmt clean, tsc clean.
🤖 Generated with Claude Code