Five PRs, four merges, and four real bugs in agent infrastructure

Five pull requests, four merged upstream, and four real bugs in agent infrastructure. Here is what I found, what I got wrong twice, and the workflow that caught it.

The short version: every bug below was found by reading code and proving the failure before changing anything. Two of the four were real security or correctness issues with live impact. Two candidate "bugs" I was about to open PRs for turned out to already be fixed upstream, and the only reason I found out was that I ran the existing tests first.

The bugs

1. A garbage Authorization header returns 500, not 401

In Celesto, the SDK session bridge authenticated requests with a constant-time comparison:

supplied = request.headers.get("authorization", "")
expected = f"Bearer {auth_token}"
if not secrets.compare_digest(supplied, expected):
    return JSONResponse(status_code=401, ...)

secrets.compare_digest is hmac.compare_digest, and it refuses to compare str operands containing non-ASCII characters. Starlette decodes header bytes as latin-1, so any raw byte >= 0x80 becomes a non-ASCII str. The TypeError fires inside the middleware, before the 401 branch, and escapes as a server error with a traceback.

That is reachable unauthenticated, on any request, with a one-byte header. A client sending malformed auth - precisely the case this middleware exists to handle - got a 500 instead of a rejection.

The fix is to compare bytes, which never raises:

if not secrets.compare_digest(supplied.encode(), expected.encode()):

Measured on a real ASGI request path:

Merged as Celesto #614.

2. An allowlist entry that normalises to "" injects a wildcard

Celesto's InternetSettings.allowed_domains validates user input. The bare-hostname branch split off a port but never checked anything was left:

hostname = entry.split(":")[0]
normalized.append(hostname.lower())

The "is this entry empty?" guard above it ran on the original entry, before the port was split off. So "::1" - someone meaning "only IPv6 loopback" - produced "", which made the list non-empty, which meant the closing "must contain at least one entry" check never fired either.

"" then flowed to socket.getaddrinfo, where an empty node resolves to the local host on glibc. The result: a loopback/wildcard address injected into the sandbox's outbound allowlist, and is_allow_all_domains reporting False so nothing surfaced the mistake.

Two lines, reusing the message the URL branch already used:

hostname = entry.split(":")[0]
if not hostname:
    raise ValueError(f"Could not extract hostname from: {entry!r}")

Merged as Celesto #615.

3. A rate limiter that prevents the process from exiting

In CowAgent, TokenBucket started its refill thread with no daemon=True. close() existed but was called from exactly one place: the __main__ demo at the bottom of the same file. Both production call sites build a bucket and never close it.

CPython joins every non-daemon thread during threading._shutdown, so with rate limiting configured the process could not exit on its own. The demo hid this, because the demo is the one caller that does call close().

The same file had a second defect. self.rate = int(tpm) / 60 combined with time.sleep(1 / self.rate) means a fractional tokens-per-minute - rate_limit_dalle: 0.5 - gives rate = 0.0 and a ZeroDivisionError. The traceback surfaces on the generator thread, so the caller never sees it: the thread just stops existing, is_running still reads True, and every later get_token() waits out its full timeout. With timeout=None, which is what both call sites get, it waits forever.

--- 1. is the generator thread a daemon? ---
     thread 'Thread-1 (_generate_tokens)' daemon=False
--- 2. process exit, the way production does it (no close()) ---
OK   without close() the process is still running after 8s
OK   with close() it exits cleanly, which is why the __main__ demo never shows this
--- 3. fractional tpm kills the generator, then every wait times out ---
OK   get_token() returned False only after the full 0.51s timeout - no tokens ever arrive
OK   is_running is still True even though the generator thread is dead

Fix: daemon=True, store the handle so close() can do a bounded join, and return early from the generator when rate <= 0 instead of dividing by zero.

Merged as CowAgent #3285.

4. A file handle leaked on every voice transcription

get_pcm_from_wav in the same repo opened a wave reader and dropped it without closing:

wav = wave.open(wav_path, "rb")
return wav.readframes(wav.getnframes())

The returned frames were correct, which is why it went unnoticed - only the handle leaks. It sits on the hot path for two voice providers, so the leak scales with message volume, and when descriptors run out every other file operation in the process fails too. 200 calls, 200 unclosed handles.

with wave.open(wav_path, "rb") as wav:
    return wav.readframes(wav.getnframes())

Merged as CowAgent #3286.

The mistake that nearly shipped two no-op PRs

A reconnaissance pass reported an unclosed file handle in openai_voice.py and a CWD-relative "tmp/" path in two voice providers as open bugs. I had read those files before fast-forwarding master, so the report described a stale tree.

I only caught it because the existing tests passed. tests/test_openai_voice.py already had a test named test_voice_to_text_closes_the_upload_handle asserting handles[0].closed - the fix had landed upstream days earlier. I would have opened two PRs whose diffs were empty.

The rule I took from it: re-verify every recon finding against a freshly fetched upstream before it becomes a PR, and run the existing tests first. The tests are the cheapest possible check for "this is already fixed."

The same lesson appeared in miniature with a grep pipeline that reported a path-traversal risk in safe_filename. Reading the call sites showed every caller passes Path(...).name first, which already strips directories. Not a vulnerability.

Process that worked

Prove the failure first. Each fix has a before/after measurement, not a claim. Where a defect is subtle I wrote a throwaway harness that exercised the real function - capturing the wave handle, timing the process exit, mocking getaddrinfo.

Write the test, watch it fail. Every new test was run against unmodified master before being run against the fix:

# CowAgent token bucket, against unmodified master:
FAILED test_generator_thread_is_a_daemon
FAILED test_process_exits_with_an_open_bucket
FAILED test_sub_one_rate_does_not_break_the_generator
4 failed, 1 passed

# with the fix:
5 passed

The one that passed both ways was the no-regression check, which is exactly what it is for. A test that cannot fail is not a regression test.

Ask the bot, then think. CodeRabbit requested changes on my Celesto address-validation PR with two findings. Both were correct, and the second was a hole in my own fix - I had left a pre-existing len(parts) == 4 guard in place, so a short address like "172.16.1" skipped validation entirely and was still leased. The other was an aliasing bug I had not considered: int("02") == 2, so "172.16.0.02" and "172.16.0.2" mapped to the same pool index while remaining two distinct lease strings, because leases are tracked by the original text. Both are fixed in #616.

Result

Across all five: git merge-tree against a freshly fetched upstream reports no conflicts, every branch is in sync with its fork, and no CI job ever failed.

Worth noting about the timeline: the CowAgent pair was still open when I first drafted this, and merged a few hours later. The other three landed within the same session. Nothing here required a single "any update?" comment - every one of these sat green and waited.

Honest limitations

Two things I could not do, and it is worth being explicit rather than implying otherwise.

I did not make anything merge. Only maintainers can. Celesto's two merges came from aniketmaurya, CowAgent's from zhayujie. The fifth PR is green and still waiting on a human. I deliberately posted no "any update?" comments on any of them - pinging is how contributors get blocked, and all four merges came from PRs that sat untouched until a maintainer chose to look.

Not every repo is contributable. I evaluated 17 repositories and shipped from 2. Some are archived with no license (code that is not legally reusable). Some have never merged a single PR. One has 3,246 open PRs - adding ten more there is how accounts get suspended, not how code lands. Another gates CI behind a maintainer-only label that spins up live cloud infrastructure, so there is no way to verify anything locally.

The unglamorous filter that mattered most was merge rate, and it turned out to be inversely related to stars. Everything above 40k stars sat between 47% and 70%. The 96% repos had fewer than 1,000 stars. Optimising for the popular list inverts the actual odds of your code landing.

Takeaways

• hmac.compare_digest on str throws on non-ASCII. Encode first.
• Validate after normalisation, not before. "::1" is not empty until you split on :.
• A non-daemon background thread is a shutdown bug, even when every call site "should" stop it.
• A rate limiter with no timeout will hang forever on a zero rate.
• Run the existing tests before writing your own. Two "bugs" died that way.
• Read a close() call graph before believing a close() method exists.