sapling

Summary Closes #1417. Both (Watchman's BSER codec) and take a length from the encoded data and use it to index into a buffer without validating it against the remaining input, so malformed/truncated payloads panic instead of returning a deserialization error. serde_bser casts a signed wire length straight to . A negative length (e.g. ) becomes , making the slice reader overflow on (debug: attempt to add with overflow; release: inverted-range slice panic) and making the stream reader call (capacity overflow). The fix rejects deserialize_charself.bytes[0]&self.bytes[..width]widthcargo testwatchman/rust/serde_bsercargo testeden/scm/lib/mincode` (1 pre-existing roundtrip + 3 new tests pass). New tests cover: negative bytestring length (slice + stream readers), oversized string length, empty char, truncated multi-byte char.

Closes #1409. hard-codes as the "Best reviewed with" link in the stack footer. Teams on GitHub Enterprise (#512), running a self-hosted ReviewStack fork (#304), or using another stack-aware review tool get a wrong or dead link in every PR. The ReviewStack source itself anticipates alternate hosts (: the URL "can change over time (particularly if someone wants to run their own fork of ReviewStack)"). This PR adds two configs: producing: Stack created with Sapling. Best reviewed with MyReview. Design notes Unset ⇒ byte-for-byte today's output. The three existing tests pass unchanged, which doubles as the regression proof. The URL template uses placeholders (, , , ) rather than the full Sapling templater: it keeps a pure, doctest-able function and matches how the URL is already written. Happy to switch to the templater (as does) if you'd prefer. The sentence shape is deliberately not** configurable. only tolerates the intro line because it starts with the literal ; free-form text would also risk corrupting ReviewStack's bullet parsing (-prefixed lines, bare , ). Keeping the fixed sentence means even old binaries parse customized bodies correctly. An invalid URL template aborts at submit time with a message naming the config. still suppresses the whole line; the new configs are no-ops in that case. Config resolution happens at the call boundary (: ), so the library function and the test mocks take non-Optional values; the defaults are exported as / . Follow-up The footer-parser hardening that was originally a second commit here (tolerating free-text introduction lines after the modern footer marker) is now its own PR per review feedback: #1419. The two are independent. Test plan New doctests in (custom URL, custom name, both, invalid-template abort, round-trip parse of a customized footer) — module doctests all pass. New + : the mock server computes the expected footer from the same configs the test sets, so it only matches if the templates were applied. now takes required keyword-only / args; all mock callers construct them from the exported defaults via . Existing pass unchanged. All four tests ran green against an OSS -equivalent build () in a manylinux_2_34 container on the initial submission; module doctests re-verified after each review revision. 🤖 Generated with Claude Code https://claude.ai/code/session_01UaQdFpGUAEQSBSqHwtQCZ1

Split out of #1410 per review feedback (discussion) — that PR makes the footer's review link configurable; this one hardens the parser that reads the footer back. The two are independent: this change applies directly to . Motivation The stack footer is part of the PR description, so anything that edits the description on github.com can perturb it: an author adding a test plan from the web UI, a reviewer fixing a typo, a bot that rewrites PR bodies. Today, any non-bullet line left between the footer marker and the bullet list makes return , and the only symptom is that silently stops linking the stack — no error is shown. ReviewStack's own parser already implements this more tolerant grammar: treats non-bulleted lines after the marker as introduction prose, and its design comment anticipates that text changing over time ("particularly if someone wants to run their own fork of ReviewStack") — which #1410 enables. Behavior After the modern marker, free-text lines before the first bullet are skipped. Text after the bullet list still terminates the list (unchanged). Legacy marker-less bodies keep the conservative behavior — free text still ends the search — so detection cannot regress for pre-marker PRs. Tests Doctests cover the tolerated introduction, after-list termination, and the legacy conservative path. 🤖 Generated with Claude Code