Repository navigation
Conversation
…choed short Four tests, red against the reader as it is: a file one byte over 64 KiB is still read whole, a named pipe where the file should be blocks the read, and a wrong-typed value or an unrecognised LUMEN_REPEAT_READS is repeated back at any length into the fault row.
…lue back short lumen_settings.json is read on every tool call. It was read whole whatever it was, and a named pipe or device where the file should be could block the call. A file over 64 KiB or one that is not a regular file is now unreadable (repeat reads off, reported as for any unreadable file). A wrong-typed value and an unrecognised LUMEN_REPEAT_READS are echoed as at most 64 characters into the fault row and the stderr line.
Taking the cap off the reader was not caught: a file over the cap was still rejected, only after being read whole. The cap now lives in read_capped, and a test feeds it a reader that never ends and fails if it is read past one byte over the cap. The echo tests assert the exact 64 characters and ellipsis, so changing the limit fails them too.
…angelog what an echo keeps A value of exactly 64 characters is repeated whole; the 65th is cut and an ellipsis added.
… is cut The entry said every echo is the first 64 characters and an ellipsis. A value of at most 64 characters comes back as written, with no ellipsis.
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.
What
lumen_settings.jsonis read on everytools/call.read_fileread it whole, whatever it was, and a long bad value in it was repeated back whole in the fault row and on stderr. This adds two limits. A valid file is read exactly as before.Change
MAX_FILE_BYTES, inread_capped). One byte over the cap is enough to know it is over, so the reader stops there. A larger file isUnreadable("larger than 65536 bytes"): repeat reads off and reported, as for any unreadable file.Unreadable("not a regular file")and is not opened. A named pipe used to block the call until something wrote to it.LUMEN_REPEAT_READSare repeated back as at most 64 characters and an ellipsis (bounded, cut on a character boundary). A short value is repeated whole; a value of exactly 64 characters is too.Precedence (env, then file, then default ON), OFF-on-trouble and the doctor finding are as they were. No schema change, no new dependency, no new fault kind.
Evidence
Order of commits: the tests first (red), then the fix, then two commits that pin what a mutation run and the review found loose.
take(cap))take(u64::MAX))is_filecheck removedboundedreturns its inputboundedcuts on a byte, not a characterThe first run of this table showed two mutations slipping through: removing
takeleft every test green, because an oversized file was still rejected, only after being read whole; and the echo tests were relative to the constant, so changing it passed. The last commit moves the cap intoread_capped, adds a test that feeds it a reader that never ends and fails if it is read past one byte over the cap, and makes the echo tests assert the 64 characters and the ellipsis exactly.read_file): no file about 1 us, a 120-byte file about 25 us. Both are what they were before: the added stat is not visible against the open and the read.…;"yes"andboguscome back whole.Gate
Local gate green on
b47f06c(740 s): fmt, clippy-D warningson the workspace and on the Tauri crate, workspace tests, frontend build and tests with thresholds, e2e (read path, ingest path, decision metering), Tauri crate tests, hook e2e, MSRV 1.94.0 build, install verify. Not run locally: the debug app bundle, because no app code changed. Run withLUMEN_RANKED_TIME_BUDGET_MS=60000, because the lumen-core ranked-outline budget test is wall-clock bound and flakes when the machine is loaded; nothing here touches it.After review,
365cdffadds one test, shortens one doc comment and rewords one CHANGELOG sentence. For it: fmt, clippy-D warningson lumen-core with all targets, the 25 settings tests, the silent-fallback guard andscripts/test-release-notes.sh, all green. The head,a96eafb, rewords that sentence again, CHANGELOG only: a value of at most 64 characters is repeated whole, so "the first 64 characters and an ellipsis" was wrong for it;scripts/test-release-notes.shpasses.The pipe test is Unix-only (
mkfifo); the cap, echo and reader tests run everywhere.Not done
The third review nit on the settings work, that the in-process
lumen-mcptests read the developer's own settings file and environment and could write to the real fault spool, is not fixed here. It needs its own change, a hermetic harness for those tests, and it does not affect 1.8.0 behaviour.