fix(cli): make the Python-subprocess timeout configurable via OPENANT_INVOKE_TIMEOUT - #237
Merged
Merged
Conversation
…MEOUT Invoke bounded every Python subprocess (parse/analyze/enhance/verify/report) with a hardcoded 30-minute deadline and no override. A large repo whose analyze/enhance phase legitimately runs longer than 30 minutes was killed mid-phase: the CommandContext deadline fires, the watchdog closes the stdout pipe read-end, and the in-flight io.Copy returns "read |0: file already closed" (rc=2). Because the phase writes its output file only on completion, the kill discards the whole phase's output and downstream phases cascade on the missing file. The failure is volume-correlated — only repos large enough to exceed 30 minutes hit it. Resolve the deadline from OPENANT_INVOKE_TIMEOUT (a Go duration like "2h", or a bare integer of seconds), falling back to the 30m default when unset, empty, or invalid (a warning is printed for a non-empty invalid value). The default is unchanged, so existing behavior is identical unless the operator opts in. The timeout still guards against a genuinely hung parser; it just no longer caps a legitimately long run at a fixed 30 minutes. Completed units are already checkpointed (a re-run resumes), and this lets a single large run finish outright. Tests (internal/python/invoke_timeout_env_test.go): resolveInvokeTimeout parses the duration/seconds forms and falls back to the default on unset/invalid; a wired test proves the env value bounds the ACTUAL deadline of a real hung subprocess (a large default with a tiny env override returns promptly), guarding against the resolver becoming dead code. Existing invoke tests unchanged (go test ./internal/python/ green). Guard the bare-seconds path against int64 overflow: a value beyond ~9.2e9 (≈292 years) would make n*time.Second wrap negative — an already-expired deadline that kills the subprocess immediately (the exact failure this knob prevents). Such values fall back to the default. Covered by TestResolveInvokeTimeout_HugeSecondsDoesNotOverflow.
gadievron
marked this pull request as ready for review
August 14, 2026 19:23
gadievron
requested review from
dgeyshis,
shahar-davidson and
sounil
as code owners
August 14, 2026 19:23
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.
What
Make the CLI's Python-subprocess timeout configurable via
OPENANT_INVOKE_TIMEOUTinstead of a hardcoded 30 minutes.
Why
Invoke(apps/openant-cli/internal/python/invoke.go) bounded every Pythonsubprocess (parse/analyze/enhance/verify/report) with a hardcoded 30-minute
deadline and no override. A large repo whose analyze/enhance phase legitimately
runs longer than 30 minutes was killed mid-phase: the
CommandContextdeadlinefires, the watchdog closes the stdout pipe, and the in-flight
io.Copyreturnsread |0: file already closed. Because the phase writes its output only oncompletion, the kill discards the whole phase's output and downstream phases
cascade on the missing file. The failure is volume-correlated — only repos large
enough to exceed 30 minutes hit it.
How
resolveInvokeTimeout()readsOPENANT_INVOKE_TIMEOUT— a Go duration ("2h","90m") or a bare integer of seconds — and falls back to the 30m default when
unset, empty, or invalid (a warning is printed for a non-empty invalid value).
seconds would wrap
n*time.Secondnegative (an already-expired deadline thatkills the subprocess immediately), so such values fall back to the default.
the timeout still guards against a genuinely hung parser.
Tests
internal/python/invoke_timeout_env_test.go— the resolver parses theduration/seconds forms, falls back on unset/invalid/overflow, and a wired test
proves the env value bounds the actual deadline of a real hung subprocess.
Existing invoke tests unchanged (
go test ./internal/python/green).Compatibility
None. The 30m default is byte-identical when the env var is unset; completed
units are already checkpointed, so a re-run resumes.
Coordination
Open PR #235 (feat/webui) adds a parallel invoke path —
InvokeCtx/InvokeCtxCapturein a new fileinternal/python/invoke_ctx.go— for the webserver, using a caller-supplied cancellable context. There is no file overlap
with this PR (it does not touch
invoke.go), so the two merge cleanly in anyorder (verified: 0 conflict markers). Note the divergence for a follow-up: that
webui path does not honor
OPENANT_INVOKE_TIMEOUT, so if both land, the CLIInvoke()path honors the override and the webui path does not — unifying thetimeout handling across both is a reasonable follow-up once both are merged.