backlog: file #1222 -- a fail-closed contract pinned through a C-stack-dependent trigger - #331
Merged
Merged
Conversation
…-dependent trigger Live: test_recursion_error_is_not_a_value_error reds #302 and #328 while origin/main 8077a03 -- the commit they are cut from -- passes the full suite 11199/0. Two unrelated changes, one shared failing node id. THE CONTRACT IS REAL AND MUST SURVIVE ANY FIX: decode_frame catches RecursionError explicitly because RecursionError is a RuntimeError and NOT a ValueError, so an 'except ValueError' would let a deep-nesting rejection escape the fail-closed path. The defect is the instrument, not the intent. MEASURED THE TRIGGER'S ENVIRONMENT SENSITIVITY, framed as that question and not as an attribution of the CI red -- a local pass or fail cannot attribute a runner failure, and reporting it as though it could is the adjacent-question error: this box (CPython 3.14.6, AMD64, win32): json.loads first raises at depth 16,914, so the test's 100,000 carries 5.9x margin the failing runner: 100,000 does NOT raise, so its threshold exceeds 100,000 same bytes, same trigger, thresholds differing by more than 6x The mechanism is visible in those numbers: sys.getrecursionlimit() is 1000, yet json.loads reaches 16,914. The C accelerator does not use Python's recursion limit -- it consumes the C STACK, which no Python-level knob controls. Interpreter build, thread stack size and runner image all move it; nothing in this repo does. RULED OUT so nobody re-derives it: across 48f8712..8077a03 there are 908 added lines and ZERO occurrences of setrecursionlimit, sys.setrecursion, threading.stack_size, stack_size or RecursionError; the test file is unchanged with blob 24807e7 byte-identical at both ends; and no recursion or stack knob exists in conftest, pyproject or the sandbox modules. Cause UNKNOWN and recorded as unknown rather than guessed -- the runner image is the remaining candidate, it is outside this repo, and it does not cleanly explain main passing. No mechanism is invented to reconcile that. The item names the fix NOT to make: raising the depth buys a green on today's image, re-fires on the next roll, and makes the test MORE environment-coupled. The contract can be pinned directly -- assert the type relationship and drive decode_frame's handler with a raised RecursionError rather than manufacturing one from real recursion. And it names how to prove a fix: the replacement must still FAIL when decode_frame's except RecursionError is narrowed to except ValueError, and must pass BOTH on a box that raises at 16,914 and on one that does not raise at 100,000 -- covering only one of those is what produced this.
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.
Files BACKLOG #1222 -- a fail-closed contract pinned through a C-stack-dependent trigger.
tests/test_sandbox_codec.py::test_recursion_error_is_not_a_value_errorpins a real contract:decode_framecatchesRecursionErrorexplicitly because it is aRuntimeError, so anexcept ValueErrorwould let a deep-nesting rejection escape the fail-closed path -- exactly as the oldbare
KeyErrordid. The contract deserves a test. The trigger does not test it portably.Measured on two environments, and the spread is the finding
json.loadsfirst raises atSame bytes, same trigger, thresholds differing by more than six times. A binary "it passes here" would
have established almost nothing; the spread is what establishes that the trigger is environment-dependent.
The mechanism, which rules out the fix people will reach for first
sys.getrecursionlimit()is 1000, andjson.loadsreaches 16,913. The C accelerator does notconsult Python's recursion limit -- it consumes the C stack, which no Python-level knob controls. So
sys.setrecursionlimit(...)in a conftest is not an available fix, and no repo-side setting couldhave caused or cured this.
Ruled out, so nobody re-derives it
48f8712d..8077a033: 908 added lines, and zero matches forsetrecursionlimit,sys.setrecursion,threading.stack_size,stack_size, orRecursionError.tests/test_sandbox_codec.pyunchanged across that range -- blob24807e73e152byte-identical atboth ends. The failing test is literally the bytes that passed.
conftest.py,pyproject.toml, or the sandbox modules.Cause: UNKNOWN, and recorded as unknown
mainat8077a033passes this test (11,199 passed, 0 failed, full suite) while two unrelated PRsfail it. The runner-image hypothesis does not cleanly explain main passing, and no mechanism is
invented here to reconcile that. An honest unknown is worth more than a plausible story -- and a
plausible story here would be believed.
The named anti-fix
Do not raise the depth until it fails again. That buys a green on today's image, re-fires on the next
roll, and makes the test more environment-coupled rather than less.
The direct alternative: assert the type relationship, and drive
decode_frame's handler with a raisedRecursionErrorrather than manufacturing one from real recursion. Acceptance criterion: thereplacement must still fail when the handler is narrowed to
except ValueError, and must pass onboth a box that raises at 16,913 and one that does not raise at 100,000. Covering only one of those is
what produced this item.
Ledger-only. Verified:
#1222free in the live ledger and the archive; merges clean onto currentmain.