Skip to content

fix(launch): stop the classifier recursing through cue's own shim - #133

Merged
NagyVikt merged 1 commit into
mainfrom
fix/classifier-shim-recursion
Aug 7, 2026
Merged

fix(launch): stop the classifier recursing through cue's own shim#133
NagyVikt merged 1 commit into
mainfrom
fix/classifier-shim-recursion

Conversation

@NagyVikt

@NagyVikt NagyVikt commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

The bug

The skill/profile classifiers spawned the bare name claude, which on any machine
with cue's shims on PATH (the default — rcSnippet pins them with
fish_add_path -p) resolved to exec cue launch claude "$@". Back inside
launch.ts, the CUE_SMART_SUBSET fold turned that child's own argv into the
next classification prompt, which spawned another classifier, which folded ITS
argv, and so on.

Measured live before the fix:

Prompt template repetitions in one classifier's argv 10
Command line length 67 KB
Concurrent classifier processes 7
Resident memory for those 7 ~2.9 GB

Scope correction (from self-review): MAX_LAUNCH_DEPTH = 3 (launch.ts:1579)
already bounds process nesting through the CUE_LAUNCHING counter, so this was
never unbounded recursion. The 10 repetitions are of the prompt template inside a
single argv, and the 7 concurrent processes came from several sessions launching at
once rather than one 7-deep chain. This is a waste bug, not a runaway. The
memory figures above are unchanged.

Why it wasn't caught

CUE_BYPASS=1 was already set on the spawn, and the docstring claimed it made the
shim "transparent". It never did — the only reader was launch-loader.ts:160,
where it suppresses a spinner.

The fix

Two independent guards; either alone breaks the loop.

  • launch.ts parse() — refuses the CUE_SMART_SUBSET argv fold under
    CUE_BYPASS, making the flag mean what the classifier already assumed. An
    explicit --subset still wins, so deliberate overrides are unaffected.
  • classifierBinOrder() — resolves the real binary first, keeping the bare
    name only as a fallback. A classification no longer boots cue launch at all,
    which also removes a full process boot from every launch's critical path.

Known gap, deliberately not fixed here

CUE_BYPASS is documented in docs/launch.md:207 and docs/shell-install.md:77-82
as "exec the real binary directly; no resolve, no materialize, no profile"
and nothing implements that. This PR adds a second, narrower reader rather than
implementing the contract, so CUE_BYPASS=1 claude --version still runs the full
pipeline. Widening it here would smuggle a behavioral change into a memory fix, so
it is noted in-code and left as a follow-up.

Verification

  • bun run typecheck — clean.
  • bun run lint — 7 warnings, all in files this PR does not touch.
  • Full suite: 33 failing on this branch, 33 on origin/main, sets byte-identical.
    All 33 pre-existing. (liedetector-tag-density.sh > accepts every step of the 5-point raster appeared in one branch run and vanished on re-run — flaky on both
    sides, absent from the compared pair.)
  • New tests: 4 for classifierBinOrder (real-binary-first, shim-skipping, fallback,
    no duplicates), 2 for the parse guard (bypass suppresses fold, explicit --subset
    unaffected).
  • Behavior preserved on a real machine — CUE_SMART_SUBSET=1, no CUE_BYPASS:
    claude -p "fix the checkout bug" → subset "-p fix the checkout bug".
  • Recursion cut — same env plus CUE_BYPASS=1 and a classifier argv → subset null.
  • classifierBinOrder()[0]/home/deadpool/.local/bin/claude while
    which -a claude still lists the shim first.

🤖 Generated with Claude Code

The skill/profile classifiers spawn the bare name `claude`. cue's shims sit
first on PATH by design (`fish_add_path -p`), so that spawn re-entered
`cue launch`, where the `CUE_SMART_SUBSET` fold turned the child's own argv
into the next classification prompt — which spawned another classifier, which
folded ITS argv, and so on.

Each level is a full Claude Code process (~400MB RSS) carrying every previous
level's argv. Measured on a live machine before this change: 10 nested levels,
a 67KB command line, ~2.9GB resident across 7 concurrent classifier processes
for a single launch.

`CUE_BYPASS=1` was already set on the spawn and documented as making the shim
"transparent", but it is only read by launch-loader (spinner suppression) — it
never gated the subset pipeline.

Two guards, either of which alone breaks the loop:

- launch.ts `parse()` refuses the CUE_SMART_SUBSET argv fold under CUE_BYPASS,
  making the flag mean what the classifier already assumed. An explicit
  `--subset` still wins, so deliberate overrides are unaffected.
- `classifierBinOrder()` resolves the real binary FIRST and keeps the bare name
  only as a fallback, so a classification no longer boots `cue launch` at all.

Normal launches still classify: with CUE_SMART_SUBSET=1 and no CUE_BYPASS,
`claude -p "fix the checkout bug"` still folds to a subset prompt.
@NagyVikt
NagyVikt merged commit e3f4a7c into main Aug 7, 2026
6 checks passed

@NagyVikt NagyVikt left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — /code-review

Decision: APPROVE (0 CRITICAL, 0 HIGH blocking) — after the three findings below were applied in 2501d555.

Author and reviewer are the same person here, so the findings are recorded in full rather than waved through.

CRITICAL

None.

HIGH

None blocking. One structural gap found and deliberately deferred:

CUE_BYPASS does not honour its documented contract. docs/launch.md:207 and docs/shell-install.md:77-82 both promise CUE_BYPASS=1 claude "execs the real binary directly; no resolve, no materialize, no profile". Nothing in src/ implements that — before this PR the only reader was launch-loader.ts:160, which suppresses a spinner. This PR adds a second, narrower reader, so CUE_BYPASS=1 claude --version still runs the full pipeline.

Deferred rather than fixed: implementing the full bypass is a behavioral change that does not belong in a memory fix. Noted in-code in claude-classifier.ts and filed as a follow-up.

MEDIUM

The original PR description overstated the mechanism. It read as 10 process levels. MAX_LAUNCH_DEPTH = 3 (launch.ts:1579) already bounds process nesting via the CUE_LAUNCHING counter — the 10 repetitions were of the prompt template inside a single 67KB argv, and the 7 concurrent processes came from several sessions launching at once. This is a waste bug, not a runaway. Measured memory unchanged. → Corrected in the PR body and in both source comments.

LOW

  • expect(order).not.toHaveLength(0) asserted nothing given the line above it → replaced with an exact toEqual([realBin, "claude"]).
  • docs/launch.md / docs/shell-install.md still describe the unimplemented full-bypass behavior → rolled into the follow-up.

Validation

Check Result
Type check Pass
Lint Pass — 7 warnings, all in untouched files
Tests (targeted) Pass — 31/31
Tests (full suite vs base) Pass — 33 failing both sides, sets byte-identical, zero new failures
Behavior preserved CUE_SMART_SUBSET=1 + no bypass still folds -p to a subset prompt
Recursion cut + CUE_BYPASS=1 and classifier argv → subset: null

Note on the review check

This PR's review GitHub check reports skipping — no automated review ran. This review was invoked manually via /code-review. Worth fixing separately: a gate that reports "skipping" reads as green and is how unreviewed changes have landed before.

NagyVikt added a commit that referenced this pull request Aug 7, 2026
docs/launch.md and docs/shell-install.md have long described
`CUE_BYPASS=1 claude` as "exec the real binary directly; no resolve, no
materialize, no profile". Nothing implemented it. Every reader in src/ did
something narrower: launch-loader dropped its spinner, and parse() (from
#133) refused the CUE_SMART_SUBSET argv fold. `CUE_BYPASS=1 claude --version`
still ran the whole pipeline — resolve, loadout, MCP prune, materialize,
relocated config dir — and on this machine it also fired a smart-subset
classification first.

That gap is what #133 tripped over: claude-classifier set the flag on its
spawn and its docstring claimed the flag made cue's shim transparent. It did
not, so the spawn re-entered `cue launch` and folded its own argv into the
next classification prompt.

launch.run() now short-circuits on the flag: locate the real binary with the
same shim-skipping PATH walk, exec it with the agent's own argv, return its
exit code. Nothing else runs.

Deliberate edges, all covered by tests:
- Exactly "1", matching every other reader. `true`/`on` are not a bypass.
- cue's own flags (--cue-profile, --dry-run, --rematerialize …) are stripped,
  not forwarded — the agent has no idea what they mean.
- The depth guard still runs FIRST and CUE_LAUNCHING is carried to the child,
  so a shim that ever resolved back into cue stays bounded by MAX_LAUNCH_DEPTH
  instead of forking without end.
- parse()'s fold guard stays as defense in depth; under a real bypass its
  result never reaches a classifier anyway.

The three internal callers that set the flag (classifier, discover ×2,
profile-draft-skill) all spawn `claude --print -p <prompt>` and want a raw
agent, so each now gets one instead of a full `cue launch` boot per spawn.

The e2e helper strips an inherited CUE_BYPASS, so a developer who exports it
no longer silently short-circuits every launch test.

Closes #136.

Co-authored-by: NagyVikt <nagy.viktordp@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant