feat(BA-7322): derive the start-service launch budget from agent config - #13685
Draft
jopemachine wants to merge 27 commits into
Draft
feat(BA-7322): derive the start-service launch budget from agent config#13685jopemachine wants to merge 27 commits into
jopemachine wants to merge 27 commits into
Conversation
…reply
The agent gave the kernel runner 10 seconds to answer a start-service
request, but the runner is allowed 30 seconds to wait for the app's port
to open. Any app that took longer than 10 seconds to bind was reported
to the manager as {"status": "failed", "error": "timeout"} and surfaced
as an HTTP 500, even though the app went on to start moments later and a
retry would succeed.
Raise the agent's reply timeout past the runner's launch timeout so the
inner budget always expires first and the caller sees the runner's real
verdict.
The runner's 30 second launch timeout is deliberately left alone: it
terminates the service process when it expires, so shortening it would
kill apps that currently survive to be picked up by a retry.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The agent and the kernel runner each carried their own hardcoded service-launch timeout. They live in packages that cannot import each other, since the runner ships inside the container, so nothing stopped a future edit to one of them from re-inverting the pair. Add `service-launch-timeout-sec` to the `[kernel-lifecycles]` section of the agent config and send it in the start-service payload. The runner uses it as the port-wait budget and the agent waits that long plus a fixed margin for the reply, so both sides now come from one number and operators can tune it per deployment. The default reproduces the previous behaviour exactly: a 30 second launch budget with the agent giving up at 35. A payload without the key keeps the runner's own default, so containers created before the upgrade are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
size:L
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
9 tasks
The agent's reply budget and the kernel runner's launch budget are two literals in packages that cannot import each other, since the runner ships inside the container. Nothing but a test can keep them ordered, and getting them backwards is the defect this branch fixes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pair it guarded no longer exists: both budgets now derive from one configured value, and test_service_launch_budget.py asserts the ordering across configured budgets instead of between two literals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comparing the two literals missed the case where the constant stays right but the call site hardcodes its own number. Drive feed_start_service instead, with the reply arriving only after a delay, and assert on the budget it actually installs. Verified against three ways of reintroducing the defect: lowering the constant, hardcoding the call site, and raising the runner's own budget. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The delayed-reply task and the timeout-branch case were decoration: the budget the call installs is what catches the defect. Still fails on all three ways of reintroducing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
'Budget' was invented here; these two files say launch_timeout, exec_timeout, timeout_seconds. 'Installed' was invented too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
'Budget' was invented for these changes; the surrounding code says launch_timeout, exec_timeout, timeout_seconds. Renames the test file, the case dataclass, the config description and the news fragment to match, and regenerates sample.toml. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The old comment gave the cross-package reason the two sides carry separate numbers, which reads as if it justified the introspection. It does not: there is simply no constant to import, only a parameter default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two files with near-identical names covering the same subject was an artifact of the stack, not a decision: the ordering test has to stand alone on the fix branch, which is the one being backported. Here it belongs with the rest, and reading the runner's timeout by introspection goes away -- this branch gives it a name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The follow-up adds the configured-timeout cases to the same subject, so a name scoped to 'ordering' would have forced a second, near-identically named file next to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both are fixed at import time, so driving feed_start_service against a mocked socket pair and a pre-filled queue proved nothing the comparison does not -- the reply was already queued, so nothing ever waited. The machinery only covered a call site that ignores its own constant, which is not a way this breaks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reading it back out of the function signature was a workaround for it not having one. Naming it changes no behaviour -- it stays the default of _start_service -- and the test then just compares two imported numbers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Neither name says which end of the handshake it is, so the assertion read as two unrelated numbers. Aliased, it states the invariant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Base automatically changed from
fix/BA-7258-start-service-reply-timeout
to
main
August 11, 2026 07:06
…able-service-launch-budget # Conflicts: # src/ai/backend/agent/kernel.py # src/ai/backend/kernel/base.py # tests/unit/agent/test_service_launch_timeout.py
ruff wanted the config lookup on one line, and mypy reads the field defaults declared inside Annotated[..., Field(default=...)] as required constructor arguments -- model_validate goes through the alias the production code reads anyway. Co-Authored-By: Claude Opus 5 (1M context) <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
1 participant
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.
📚 Stacked PRs
Part of the start-service timeout work. Merge in order (bottom-up):
fix(BA-7258): wait longer than the kernel runner for a start-service replyfeat(BA-7322): derive the start-service launch timeout from agent config ← you are here#13684 is merged, and backported as #13696 (26.8) and #13697 (26.4). This PR stays on
main.Resolves #13683 (BA-7322)
Summary
service-launch-timeout-secto the[kernel-lifecycles]section of the agent config and send it in the start-service payload: the runner uses it as the port-wait timeout, and the agent waits that long plus a fixed margin for the reply. The ordering then holds by construction rather than by convention, and operators can tune it.Why the knob is worth having
#13684 moves the false-failure cutoff from 10s to the runner's own 30s, which covered every launch measured under a saturated pinned core (7/7 succeeded, 22.9s–26.8s). But that measurement peaked ~3s short of the 30s ceiling, and pushing contention further produced repeatable failures at ~31.7s. A site whose images bind that slowly currently has no way to buy headroom without a code change; this PR gives it one.
Test plan
tests/unit/agent/test_service_launch_timeout.py— 7 passed: across configured timeouts the agent always outlasts the runner, the runner takes the timeout from the payload, and it falls back when the key is absent.configs/agent/sample.toml; the diff is the new key only.'kernel-lifecycles': {... 'service-launch-timeout-sec': 30.0}fromagent.toml.0.2, launchingjupyterfails at 0.35s carrying the runner's ownextra_data: 'opening the service port timed out: jupyter'rather than the agent's generic'timeout'; back at30.0, the same app on the same session returns 200 in 0.67s.Known gap
Lowering the configured value below 25s re-inverts the pair for containers created before the upgrade: their runner predates the payload key and stays at its own 30s default, while the agent would wait only
configured + 5. Raising the value is safe; lowering it only takes effect for containers created afterwards.🤖 Generated with Claude Code