docs: pair the hint_exit examples with a task timeout that outlives their wait - #6
Open
codeAnqiang-ma wants to merge 1 commit into
Open
Conversation
The three `wait_* --timeout N || hint_exit ...` examples in checks.md and authoring-guide.md use N of 50-60s without a `timeout:` override, so the default 30s per-attempt timeout SIGKILLs the whole script long before the fallback arm can run and the documented hint never reaches the student. Give those examples the companion `timeout:` that 060.lock-it-down already uses, and state the constraint next to the two flags that define it. Fixes iximiuz#5 Co-authored-by: Cursor <cursoragent@cursor.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
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.
Fixes #5
The problem
docs/checks.mdlabels this "the canonical shape" for a check with a fallback hint, and promises right after it that "the hint stays visible in the task box until replaced":With default settings that hint can never appear. The snippet has no
timeout:, so the attempt uses the 30 s default (internal/engine/engine.go:42-45,:548-551), and on expiry the runner SIGKILLs the whole process group (internal/engine/runner.go:135-140) whilewait_cwdis still 30 s away from returning. The|| hint_exitarm is never reached, and the attempt is recorded as exit 124 rather thanHintExitCode(42), so the engine takes the ordinary-failure path (:570-575) and just restarts the check. The task still completes normally once the condition holds — only the hint is silently dead.Three examples had a
--timeoutat or above the default with no override:docs/checks.md(wait_cwd --timeout 60,wait_file_mode --timeout 50) anddocs/authoring-guide.md(wait_port --timeout 60).What this changes
Docs only, 15 insertions across 2 files:
timeout:they need. For thewait_file_modeone that means restoringtimeout: 60— the snippet was excerpted frompaths/sample-linux-101/030.files-and-folders/060.lock-it-down/unit.md, which already sets it and explains why in a comment ("the attempt timeout must exceed that or the hint never fires"). The other two gettimeout: 70for their 60 s waits, keeping the same ~10 s headroom that unit uses.--timeoutbullet indocs/checks.mdand thetimeoutbullet indocs/authoring-guide.md— so an author who wants a long bounded wait knows to raise the attempt timeout instead of rediscovering this the hard way.I deliberately kept the examples' original wait durations. Lowering them to
--timeout 15(the value used in every other example in the repo) would fix the snippets just as well with an even smaller diff — happy to switch if you prefer that direction.Testing
Ran on Linux (
golang:1.24,--cap-add=NET_ADMINso the netlink-backed tests execute for real), on this branch:No test is added, since nothing but prose and YAML snippets changed. The behaviour itself is verified in #5 with a throwaway
internal/enginetest that runs the old snippet verbatim under production defaults (no hint within 40 s;attempt 0: exit=124 timedOut=true dur=30.1s) alongside a control that only raisestimeout:and does receive the hint. I left that test out of this PR on purpose — it takes ~60 s of wall clock and asserts a documented invariant rather than product behaviour. As mentioned in the issue, ashellgym validatewarning for--timeout NwithN >=the effective attempt timeout would be the place to catch this mechanically; glad to look at that separately if you want it.Note: on macOS
internal/checkclientdoes not compile (syscall.Stat_t.Modeisuint16on Darwin,uint32on Linux, atcheckclient.go:369), unrelated to this change — hence the Linux container for the test run.Licensing
Per CONTRIBUTING.md: I license this contribution to Ivan Velichko (iximiuz Labs) under any terms, including commercial licensing and relicensing of Shell Gym as a whole.
Produced with AI assistance; I reproduced the behaviour locally and reviewed every line of this change myself.