Skip to content

fix: make the check timeout configurable so real build gates can pass - #74

Open
bryfa wants to merge 2 commits into
NateBJones-Projects:mainfrom
bryfa:fix/configurable-check-timeout
Open

fix: make the check timeout configurable so real build gates can pass#74
bryfa wants to merge 2 commits into
NateBJones-Projects:mainfrom
bryfa:fix/configurable-check-timeout

Conversation

@bryfa

@bryfa bryfa commented Jul 23, 2026

Copy link
Copy Markdown

The defect

CHECK_TIMEOUT_S = 60 is a module constant referenced directly inside Verifier._run_check, with no parameter, no config key, and no manifest field. A task's timeout_s governs only the worker subprocess — it has never applied to the check.

So any check that legitimately runs longer than 60 seconds is killed, regardless of what the task declares. The repo-feature kit is the clearest case: checks/check_repo_feature.py sets timeout=1800 on its own subprocess.run for the build command, so the kit believes it has 30 minutes while the parent gives it 60 seconds.

Reproduced on 71763e6:

task:   timeout_s=2400, check="sleep 65 && echo 'build+test done' && exit 0"
result: elapsed=60.0s  timed_out=True  ok=False  rc=-15 (SIGTERM)
output: [ringer.py] check timed out after 60s

This matters more than a slow build. A killed check produces verdict TIMEOUT, which is written to runs.jsonl and aggregated into the per-(model, task_type) scoreboard. On that scoreboard a slow-but-correct build is indistinguishable from a worker that produced nothing — so the routing signal Ringer exists to produce is quietly wrong for any lane whose check compiles or runs a test suite.

The fix

Resolution order, highest precedence first:

  1. the task's check_timeout_s (new, optional manifest field)
  2. check_timeout_s in config.toml (new, optional install-wide default)
  3. DEFAULT_CHECK_TIMEOUT_S = 60 (unchanged)

Every resolved value is clamped to MAX_CHECK_TIMEOUT_S = 3600 and validated at parse time, so the gate is still guaranteed to terminate — an unbounded check would be an unbounded run.

The repo-feature kit now declares "check_timeout_s": 1800, since its check shells out to a real build/test command. Its README explains when a kit must set this.

Backward compatibility

  • A manifest that does not opt in resolves to exactly 60 — identical behaviour to before.
  • CHECK_TIMEOUT_S is retained as an alias for anything referencing it.
  • Verifier() still constructs with no arguments.
  • Retry behaviour is untouched: TIMEOUT remains a retryable verdict and max_attempts stays 2.

Tests

New tests/test_check_timeout.py, 20 tests covering resolution precedence, parse-time validation, config loading and rejection, actual subprocess enforcement, accurate timeout reporting (the message names the resolved budget, not the constant), and the unchanged retry contract. One test is an unscaled proof that a check surviving 65 real seconds now passes — opt-in via RINGER_SLOW_TESTS=1 so it does not cost a minute on every run.

Suite goes 179 → 199. The three failures present on 71763e6 (test_design_reference ×2, test_scoreboard_page ×1) are unchanged — they are HTML/design-token assertions unrelated to execution; one asserts a hardcoded "Generated July 6, 2026" date string.

Run with:

cd tests && python3.13 -m unittest discover -s . -t .
cd tests && RINGER_SLOW_TESTS=1 python3.13 -m unittest test_check_timeout.SlowRealTimeoutTests -v

CHECK_TIMEOUT_S was a module constant with no override path, so every task's
check command was killed at 60s regardless of what the task declared. A task
with "timeout_s": 2400 whose check runs `dotnet build && dotnet test` was
SIGTERM'd at exactly 60.0s and reported TIMEOUT — a false negative on the
mechanism that decides PASS, and indistinguishable on the scoreboard from a
worker that produced nothing.

Reproduced on 71763e6: task timeout_s=2400, check `sleep 65 && echo ok`
-> elapsed=60.0s timed_out=True rc=-15.

Resolution order, highest first:
  1. the task's "check_timeout_s" (new, optional)
  2. config.toml "check_timeout_s" (new, optional, install-wide default)
  3. DEFAULT_CHECK_TIMEOUT_S = 60 (unchanged)
Every resolved value is clamped to MAX_CHECK_TIMEOUT_S = 3600 and validated at
parse time, so the gate is still guaranteed to terminate.

Backwards compatible: a manifest that does not opt in resolves to 60, exactly
as before. CHECK_TIMEOUT_S is retained as an alias. Verifier() still constructs
with no arguments. Retry behaviour is untouched — TIMEOUT remains a retryable
verdict and max_attempts stays 2.

The repo-feature kit now declares check_timeout_s: 1800, since its check shells
out to a real build/test command; its README explains when a kit must do this.

tests/test_check_timeout.py: 19 tests covering resolution precedence, parse-time
validation, config loading, actual subprocess enforcement, accurate timeout
reporting, and the unchanged retry contract. Plus an opt-in unscaled proof
(RINGER_SLOW_TESTS=1) that a check surviving 65 real seconds now passes.

Suite: 179 -> 199 tests. The 3 pre-existing failures on 71763e6
(test_design_reference x2, test_scoreboard_page x1) are unchanged.
Uncommitted locally since 2026-07-23.

engines/opencode-sandboxed.sh: RINGER_EXTRA_WRITABLE (colon-separated absolute
dirs) widens the writable set. The .NET SDK writes NuGet caches, ~/.dotnet and
MSBuild node state outside the repo; when those writes are denied MSBuild does
not error, it HANGS with no output — undiagnosable from inside a worker. Entries
are passed as -D params like every other path and never interpolated into the
profile text, so the rule-injection guarantee still holds.

docs/MODEL-NOTES.md: codex run record for hospedo phase-5c. The single FAIL was
the check's bug, not the model's — the gate grepped \b85\b while C# writes 85m,
and no word boundary exists between a digit and m, so a correct implementation
could never pass. Recorded so the scoreboard is not read as evidence against
codex on code-feature.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant