Repository navigation
fix(autohooks): close ariel connections on the success path too - #198
Merged
Merged
Conversation
Every CLI command branch ends with 'return 0' inside the try block, and a return from a try body skips the else clause - so _close_ariel() never ran on the path every hook takes. The unclosed aiosqlite connection kept a non-daemon worker thread, threading._shutdown joined it forever, and the process printed its output then hung until the 120s watchdog killed it. Observed impact: 'autohooks inject' (the opencode/ariel session-start hook) hangs 120s; the hook's 20s spawnSync timeout then discards the output, so memory injection was silently lost on every session start. Move the close into finally so both paths are covered, and add the missing success-path regression test (the existing test only covered the exception path, which is why this survived). inject: 120s hang -> 2.2s clean exit; full suite 2869 passed, mypy clean.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
typecheck and docs declare setup-python's 'cache: pip' but then install
everything through uv, so /home/runner/.cache/pip is never created and
setup-python's post step fails the job:
##[error]Cache folder path is retrieved for pip but doesn't exist on disk
The real steps passed in every run (mypy success, docs build success); only
the cleanup failed, which left every PR blocked on a red check. Jobs that do
use pip (lint, audit, docs.yml) keep the cache.
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.
Problem
Every CLI command branch in
autohooks/__main__.pyends withreturn 0inside thetryblock, and areturnfrom atrybody skips theelseclause entirely — so_close_ariel()never ran on the success path, which is the path every hook takes.The unclosed aiosqlite connection keeps a non-daemon worker thread alive.
threading._shutdownjoins it forever: the process prints its output, then hangs until the 120 s watchdog kills it.Observed impact
autohooks inject(the opencode/ariel session-start hook) hangs 120 s. The hook'sspawnSynctimeout is 20 s, so the already-printed output is discarded — memory injection was silently lost on every session start. Confirmed in production: onlymirror drift surfacedlines in the log, zeroinjectedlines.Fix
Move the close into
finallyso both paths are covered, and add the missing success-path regression test. The pre-existing test covered only the exception path — which is exactly why this survived.Verification
autohooks inject: 120 s hang → 2.2 s, rc=0autohooks inject --with-recap: 2.2 s, rc=0system.transform: injected 3280B (with recap)now fires on every session startruff check ./ruff format --check .: cleanmypyover 10 dirs: no issues in 245 source filespytest tests/ -q: 2869 passed