review: correct #133's overstated recursion depth, tighten a vacuous assertion - #135
Merged
Merged
Conversation
…ertion
Self-review findings on this PR.
MAX_LAUNCH_DEPTH already bounds the process nesting at 3 via the CUE_LAUNCHING
counter, so the original wording ("10 nested levels") was wrong about the
mechanism: the 10 repetitions were of the prompt template inside a single 67KB
argv, and the 7 concurrent classifier processes came from several sessions
launching at once, not one 7-deep chain. The measured memory is unchanged; the
bug is waste, not runaway recursion. Comments corrected to say so.
Also notes, in the code where it matters, that CUE_BYPASS is documented as a
full bypass ("exec the real binary directly; no resolve, no materialize, no
profile") that nothing implements — this PR's guard is deliberately narrower.
Tracked as a follow-up rather than widened here.
`expect(order).not.toHaveLength(0)` asserted nothing useful; replaced with an
exact `toEqual([realBin, "claude"])`.
This was referenced Aug 7, 2026
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
1 participant
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.
Follow-up to #133. Comment/test-only — no behavior change.
#133 merged while the
/code-reviewpass on it was still running, so these threereview findings never made it onto
main. The functional fix landed correctly; thiscarries the corrections.
What this changes
1. Corrects an overstated claim (MEDIUM). #133's description and both source
comments read as though the classifier recursed 10 process levels deep.
MAX_LAUNCH_DEPTH = 3(launch.ts:1579) already bounds process nesting via theCUE_LAUNCHINGcounter, so it never did. The real shape:once, not one 7-deep chain
It is a waste bug, not a runaway. The measured memory (~2.9GB across those 7) is
unchanged — only the mechanism description was wrong. Comments now say so.
2. Documents a known gap (HIGH, deliberately deferred).
CUE_BYPASSisdocumented in
docs/launch.md:207anddocs/shell-install.md:77-82as "exec thereal binary directly; no resolve, no materialize, no profile" — and nothing
implements it. Before #133 the only reader was
launch-loader.ts:160(spinnersuppression); #133 added a second, narrower one. So
CUE_BYPASS=1 claude --versionstill runs the full pipeline. Implementing the real contract is a behavioral change
that does not belong in a memory fix, so this just records the gap where the next
reader will hit it.
3. Tightens a vacuous assertion (LOW).
expect(order).not.toHaveLength(0)couldnot fail given the line above it → now
toEqual([realBin, "claude"]).Verification
bun run typecheck— cleanbun test src/lib/claude-classifier.test.ts src/commands/launch.parse.test.ts— 40/40 passmain(ca492fb8), auto-merged with no conflicts🤖 Generated with Claude Code