fix(verify): let the timeout of a discovered test command be configured (AGT-4678) - #819
Merged
Merged
Conversation
…ed (AGT-4678) A test command the verifier discovers itself gets the generic 300 s. cgf-portal's apps/pipelines suite takes 272 s calm and 295 s with two verifications at once, so the cap sits inside the suite's natural duration and both verdicts after the AGT-4676 slot limit were lost to it: one at 87%, one with pytest reporting 294.91 s. Add autonomous.verify.commandTimeoutMs (60 s to 900 s, unset keeps 300 s) and apply it to discovered commands only; a command a repository declares in its own manifest keeps the timeout it declared. The bound is generic slack, not a product requirement, and what it exists for (stopping a hung test) still holds at 600 s.
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.
TL;DR
A test command the verifier discovers itself (
discover.ts) gets a fixed 300 s timeout. cgf-portal'sapps/pipelinessuite takes 272 s with the machine calm and 295 s with two verifications at once, so the cap is inside the suite's natural duration and the verdict is lost whenever the machine carries any load; the attempt then falls back to the LLM tester (about 16 minutes).Classification of the bound (before relaxing it)
discover.ts(DEFAULT_TIMEOUT_MS), not a figure derived from this repository. A repository that declares its own manifest already sets it per command (manifest.ts, up to 900 s).1 failed, 5946 passed, 812 skipped in 272.08s, calm), 294.91 s (skipped, 1 warning in 294.91s, two verifications at once), and 87% at the cap for another run. The cap is below the duration, so it cannot pass.Evidence (daemon log, 22:20 restart, after the AGT-4676 slot limit)
Two verifications finished and both ended
timeout after 300000ms: one at[ 87%], one with pytest reportingin 294.91s. Before the slot limit: 7 of 7 at 3%.Change
autonomous.verify.commandTimeoutMs(optional, 60 s to 900 s; unset keeps 300 s).withDiscoveredTimeoutapplies it inloadTrustedVerifyPlanto discovered commands only. A command a repository declared in its own manifest keeps the timeout it declared.Tests
withDiscoveredTimeoutdoes not mutate its input.loadConfigcarries the field through and leaves it unset by default; values below 60 s, above 900 s, 0 and negative are rejected. (A schema fieldloadConfigdoes not carry never reaches the verifier.)vitestdeterministic tester, config, verify and pipeline verify suites: 286 of 287 pass. The one failure (runs npm package scripts against head-installed node_modules at base) fails the same way on a checkout without this change (it runs real npm under machine load).Risk
A verification that really hangs now holds its slot for up to 600 s instead of 300 s; with two slots and the 20 minute slot wait bound that is acceptable.
After deploy
Set
autonomous.verify.commandTimeoutMs: 600000in the machine'sconfig.yaml; the share of verifications that end in the timeout should fall.