fix(rulebook): raise the hook's regex length bound from 400 to 2000 (v0.88.0, ENG-1170) - #267
Conversation
GitHub Actions
Active
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6121c7bbfd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # wide enough for the second. It was 400 before 0.75.0: a hook older than that | ||
| # drops any rule carrying a longer pattern, which is why the server floors such | ||
| # a rule at `min_hook_version` 0.75.0. | ||
| _RX_MAX = 2000 |
There was a problem hiding this comment.
Raising only the hook limit leaves the shipped authoring flow enforcing the old value: plugins/memhub/skills/start-rulebook/scripts/starter_rulebook.py:51,96-103,694-696 still truncates repo-derived alternations around 360 characters and reports every seeded pattern over 400 as a load failure. Consequently, repositories with enough generated/ignored/secret paths cannot benefit from the newly supported 401–2000-character patterns—the generator either omits paths or rejects an otherwise loadable rule. The create-rule guidance at plugins/memhub/skills/create-rule/SKILL.md:460-463 also still tells authors that such patterns are silently dropped.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 8d6d768: starter_rulebook.py RX_MAX is 2000 and _fit cites it instead of a literal; create-rule SKILL.md LOAD line says 2000.
…v0.75.0) `_RX_MAX` dropped the WHOLE rule, silently, for any pattern over 400 characters. Length is not the backtracking guard — `_RX_NESTED` is: a 60-character pattern can stall and a 1,500-character alternation of literal paths cannot. 400 refused the second, which is a real rule shape (`path_rx` / `cmd_rx` over a list of paths), and bought nothing against the first. The bound stays a bound (2000) so one server rule cannot cost unbounded compile time or storage. A hook older than this release still drops a rule carrying a longer pattern; MemHub-Backend floors such a rule at `min_hook_version` 0.75.0 in the paired change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
6121c7b to
b2e6f26
Compare
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2e6f26890
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # wide enough for the second. It was 400 before 0.75.0: a hook older than that | ||
| # drops any rule carrying a longer pattern, which is why the server floors such | ||
| # a rule at `min_hook_version` 0.75.0. | ||
| _RX_MAX = 2000 |
There was a problem hiding this comment.
Raising this bound newly lets deeply nested patterns reach re.compile, but rx_ok catches only re.error; for example, "(" * 500 + "a" + ")" * 500 is 1,001 characters and raises RecursionError on supported Python versions. The live loader happens to swallow this through to_hook_rule's outer exception handler, but rulebook_verify.py::_load_failure calls H.rx_ok directly, so verifying such a candidate now terminates with a traceback instead of reporting LOAD FAIL. Catch RecursionError (or otherwise bound nesting) before accepting the expanded length range.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed on 3.9 and 3.12: the 1,001-char nested pattern raises RecursionError, not re.error. rx_ok now catches both; covered in rulebook_hook_test.py (rule drops, no traceback) and rulebook_verify_test.py (reports LOAD FAIL, no traceback). 8d6d768.
…n (v0.76.0) (#270) #260 (v0.69.0) made the harness lane default-on for every install. On costs the person a classifier call per flagged turn and a headless authoring run per moment on THEIR OWN model quota, and files proposals into a shared team rulebook. Felix's call, 2026-09-21: an install should not start that without being asked. Opt-in again. Three gates, flipped together because they are one switch and must never disagree: `extract_enabled` in harness_extract, `harness_extract_on` in rulebook_hook (the error-arc pairing), and the shell `case` in claude-hooks.json that runs before either. The shell gate proceeds only on an explicit on value (1/on/true/yes, any case), so unset, blank and unrecognised all exit before python starts. The empty string moved back with the default, and unrecognised values moved with it: under default-on a typo ran the lane, under opt-in a typo must not start the spend. Tests assert both sides, including the spellings #260 added. Docstrings, the module headers, the rulebook_hook comment and the README moved in the same change. The README section was still titled "(flagged off)" and still ended "With the variable unset, the default, none of this runs" — both false since #260, true again now. `_OFF` is removed; nothing reads it. Anyone who was relying on default-on since v0.69.0 must now set MEMHUB_HARNESS_EXTRACT=1. check-plugin.sh 69 of 72, the same three pre-existing failures as origin/main (codex_history, readers_cli, readers_validation). Nine version files to 0.76.0 (0.74.1 and 0.75.0 are taken by open #268 and #267). Refs ENG-1107, reverts the default from #260. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
# Conflicts: # plugins/memhub-staging/.claude-plugin/plugin.json # plugins/memhub-staging/.codex-plugin/plugin.json # plugins/memhub-staging/.mcp.json # plugins/memhub/.claude-plugin/plugin.json # plugins/memhub/.codex-plugin/plugin.json # plugins/memhub/.cursor-plugin/plugin.json # plugins/memhub/.mcp.json # plugins/memhub/mcp.json # plugins/memhub/plugin.json
…rn instead of tracing back
Merged main: 0.75.0 was long taken, staging manifests are gone from the
public repo, so the bump is 0.88.0 across the six production manifests and
the hook comment names that as the version the server floors a long
pattern at.
Codex's two findings on the earlier head, both real:
- `rx_ok` caught only `re.error`. A pattern nested past the parser's
recursion limit ("(" * 500 + "a" + ")" * 500, 1001 chars) raises
RecursionError, which the wider bound now lets reach `re.compile`.
`rulebook_verify` calls `rx_ok` directly, so a verify of such a candidate
tracebacked instead of reporting LOAD FAIL. Caught alongside `re.error`.
- The starter generator still truncated repo-derived alternations at the old
400 and reported anything over it as a load failure; the create-rule skill
still said over-400 is dropped. Both now say 2000.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
felix-xtrace
changed the title
🧠 Session context1 session behind this pull request. Team rules that fired while building this
Effort Sessions
|
felix-xtrace
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d6d768c71
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| try: | ||
| re.compile(pat) | ||
| except re.error: | ||
| except (re.error, RecursionError): |
There was a problem hiding this comment.
Patterns newly admitted by the 2,000-character bound can make re.compile raise OverflowError, which this handler still lets escape. For example, "a{" + "9" * 500 + "}" is 503 characters, bypasses _RX_NESTED, and raises “the repetition number is too large”; it was rejected by the former length check before compilation. Although the live loader's outer handler drops the rule, rulebook_verify.py::_load_failure calls rx_ok directly, so the required authoring verification command terminates with a traceback rather than reporting LOAD FAIL.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real, and older than this PR: a{99999999999} is 14 characters and raises OverflowError too, so the length check never covered it. rx_ok now treats any exception from re.compile as "does not load" — except Exception, since the answer is the same whatever the parser objected to. Regression cases in both test files (rule drops; verify reports LOAD FAIL, no traceback). 751a98f.
…rule Codex, on 8d6d768: `a{` + 500 nines + `}` is 503 characters, passes the nested-quantifier lint, and raises OverflowError from `re.compile`, which `rx_ok` let escape — so `rulebook_verify` tracebacked instead of saying LOAD FAIL. The class is older than the 2000 bound (`a{99999999999}` is 14 characters and does the same), so the guard is now `except Exception`: a pattern off the wire either compiles or the rule drops, whatever the parser objected to. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 751a98f26c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # wide enough for the second. It was 400 before 0.88.0: a hook older than that | ||
| # drops any rule carrying a longer pattern, which is why the server floors such | ||
| # a rule at `min_hook_version` 0.88.0. | ||
| _RX_MAX = 2000 |
There was a problem hiding this comment.
With the new bound, a rule such as command_rx = "a?" * 134 + "a" * 134 + "$" is 403 characters and passes both rx_ok and _RX_NESTED, although matching it against a command of 134 as causes exponential backtracking in evaluate's synchronous re.search. It was rejected by the old 400-character check; now it can make each PreToolUse check run until the 5-second hook timeout (and skip enforcement), so the newly allowed range needs a safe matching limit or lint that rejects this shape.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real shape, but not one the length bound ever guarded: "a?" * n + "a" * n + "$" at n=22 is 67 characters and already takes 0.33s against 22 a-characters; n=26 (79 chars) clears the 5-second hook timeout. The 400 bound refused the 403-character example while admitting the 79-character one — which is the PR premise: length is not the backtracking guard, _RX_NESTED is a denylist and the hook comment names a bounded matcher (worker + wall clock, §5.1 Phase 2) as the real fix. Not changed here; the bounded matcher is its own PR.
This branch was successfully deployed
1 participant
What
_RX_MAXinrulebook_hook.pygoes 400 → 2000. Version bumped to 0.88.0 across the six production manifests (the branch was rebased-by-merge onto main after 0.75.0–0.87.0 shipped and the staging manifests left the public repo in #274).Why
rx_okdrops the whole rule, silently, when any pattern is over_RX_MAX. Length is not the backtracking guard —_RX_NESTEDis. A 60-character pattern can stall; a 1,500-character alternation of literal paths cannot. 400 refused the second (a realpath_rx/cmd_rxshape) and bought nothing against the first. The bound stays a bound so one server rule cannot cost unbounded compile time.Nothing in Claude Code imposes 400; it was our own constant.
Also in this PR (Codex findings on the earlier head, both real)
rx_okcaught onlyre.error. A pattern nested past the parser's recursion limit ("(" * 500 + "a" + ")" * 500, 1,001 chars) raisesRecursionError, which the wider bound now lets reachre.compile.rulebook_verifycallsrx_okdirectly, so verifying such a candidate tracebacked instead of reportingLOAD FAIL. Now caught alongsidere.error.starter_rulebook.py) still truncated repo-derived alternations at the old 400 and reported anything over it as a load failure; the create-rule skill doc still said over-400 is dropped. Both say 2000 now.Rollout
A hook older than 0.88.0 still drops a rule carrying a pattern over 400. MemHub-Backend #1351 (merged to staging 2026-09-28) raised the server's
MAX_REGEXto 2000 and floors such a rule atmin_hook_version0.75.0 — a number this PR no longer ships under. Until the floor is moved to 0.88.0 (MemHub-Backend#1401), a 0.75.0–0.87.0 hook passes the floor and silently drops the rule. Merge this, release 0.88.0, then merge the backend floor change.Tests
tests/rulebook_hook_test.py: a ~1.3k-character alternation loads; a pattern at_RX_MAXloads and one past it drops the rule; a deeply nested pattern under_RX_MAXdrops the rule without a traceback.tests/rulebook_verify_test.py: over-length LOAD failure probes at 2050; deeply nested pattern reportsLOAD FAIL, no traceback.MEMHUB_RULEBOOK_FETCH=0 uv run --python 3.12 --with 'mcp<2' python tests/run_all.py→ all 73 suites passed.Not done: tag, marketplace pin. Linear: ENG-1170.
🤖 Generated with Claude Code