Skip to content

fix(run): make the Instance count stepper work for every non-Claude mode - #454

Merged
Ark0N merged 1 commit into
Ark0N:masterfrom
timkjr:fix/multi-instance-run-count
Sep 19, 2026
Merged

Ark0N merged 1 commit into
Ark0N:masterfrom
timkjr:fix/multi-instance-run-count

Conversation

@timkjr

@timkjr timkjr commented Sep 19, 2026

Copy link
Copy Markdown

Summary

  • The "Instance count" stepper next to the Run button was only ever read by runClaude(). Every other mode's run function — OpenCode, Codex, Gemini, Antigravity, Pi, OMP, Grok, DeepSeek — hardcoded a single quick-start call, so bumping the counter while on any of those modes silently launched exactly one session with no error.
  • Extracted the shared launch-N-and-select-the-first loop into _launchQuickStartInstances(), reused by all eight modes, plus _readTabCount() for the shared clamp/parse. Each mode still builds its own quick-start body (config differs per CLI) via a closure passed into the shared loop.

Test plan

  • npm run typecheck, npm run lint, npm run format:check, npm run check:frontend-syntax
  • npm test (full suite, 7368 passed) — updated test/run-mode-ui.test.ts fixtures for the five modes with existing coverage (Codex, Gemini, Antigravity, Pi, Grok) to include the stepper element
  • Manually verified live across Pi, OMP, and Claude with counts of 2–3

🤖 Generated with Claude Code

https://claude.ai/code/session_012AcpEgRwdEbdTqm4W4wanS

runOpenCode(), runCodex(), runGemini(), runAntigravity(), runPi(), runOmp(),
runGrok(), and runDeepSeek() all ignored the "Instance count" stepper next
to the Run button and hardcoded a single quick-start call — bumping the
counter to 2 or 3 while on any of these modes silently launched exactly one
session, with no error. Only runClaude() ever read it.

Extract the shared launch-N-sessions-and-select-the-first loop into
_launchQuickStartInstances(), reused by all eight modes, and _readTabCount()
for the shared clamp-and-parse. Each mode still builds its own quick-start
body (config differs per CLI), just via a closure passed to the shared
loop instead of a single inline fetch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Ark0N

Ark0N commented Sep 19, 2026

Copy link
Copy Markdown
Owner

This is going in. Thanks for this, and welcome.

You found a real silent-failure bug, which is the best kind of first contribution: the stepper sat there next to the Run button looking like it worked, and on eight of the ten run modes it did nothing at all. No error, no hint, just the wrong number of sessions. The refactor is also the right shape. Each mode keeps its own quick-start body through the buildBody closure, so no payload changed (I diffed all eight against their pre-PR form), and you kept the two things that are easy to lose here: the {success,data} envelope unwrap, and _ensureCreatedSessionVisible() before selectSession().

What I am folding in on top at merge:

A regression test. This is the one thing the PR was missing. Your five test edits are fixture maintenance for the single-instance case, so they pass identically with and without the change, and three of the eight converted modes have no quick-start test at all. The new case drives runGrok() with tabCount at 3 and asserts three POSTs with w1/w2/w3 names and the first session selected. I checked it fails against master's session-ui.js (['w1-grok-case'] vs the expected three) and passes on yours, so it genuinely pins the behaviour rather than just describing it. This matters more than box-ticking here, because the bug happened in the first place by each mode carrying its own copy of the launch call.

The count in the opening banner. Each caller announced Starting Grok session in ..., singular, and then the helper printed Starting 3 Grok session(s) in ... right after it, so a home-screen launch got two headers in a row. Worse, with another session already active _appendSessionLaunchStatus returns early, so the helper's lines were suppressed entirely and a five-instance launch showed one toast promising one session. The callers now read the count first and put it in their own banner, the way runClaude() already did, and the helper's opener is gone.

runClaude() now calls _readTabCount() instead of keeping its own copy of the 1..20 clamp, which is what your JSDoc was already pointing at.

A ?. on the element read, because hoisting the count above each caller's try block means a missing #tabCount would otherwise throw somewhere the launch-error path cannot report it.

Two things I did not change, on purpose. The helper launches sequentially where runClaude()'s local path fans out, but that matches runClaude()'s own remote loop and is the safer default, so I left it. And a partial failure leaving the started sessions unselected behaves exactly like the remote loop already does, so fixing it is a consistency change for both, not for this PR.

One genuinely pre-existing thing you surfaced: the welcome-screen buttons call app.runOpenCode() and friends directly rather than going through app.run(), so they skip the in-flight lock, and now that the other eight modes honour the stepper a double click there produces N duplicate names instead of one. That predates you. I will take it separately.

On #453, which is yours too: it is not being ignored. It is waiting on Discussion #452, because whether Codeman wants in-window split panes at all is a product call I owe you before you spend more time on a 3000-line PR. I will answer there.

@Ark0N
Ark0N merged commit 2d842de into Ark0N:master Sep 19, 2026
2 checks passed
Ark0N pushed a commit that referenced this pull request Sep 19, 2026
…d capture geometry

#454: the behaviour the PR adds had no test, so a regression test drives
runGrok() at tabCount 3 and asserts three quick-start POSTs with sequential
w<n>-<case> names (verified to fail against master's session-ui.js). Each
caller now reads the count BEFORE its opening banner and announces it there,
the way runClaude() already did, so a launch no longer prints two headers and
a launch with another session already active still says how many are starting.
runClaude() calls the shared _readTabCount() instead of its own copy of the
1..20 clamp, and that helper optional-chains the element read, since hoisting
it above each caller's try block would otherwise let a missing #tabCount throw
where the launch-error path cannot report it.

#435: sizeMovedUnderLoad derived from data.source alone. `mux-visible` is not
sufficient: a failed display-message cursor query makes capturePaneBuffer skip
the snapshot repaint and return the raw capture, which the route still labels
mux-visible, so a size that moved during such a load bought a full forced
reload to repair a frame that was never positioned. It now tests
Number.isFinite(data.captureRows) like its two siblings.

Plus the invariants and CLAUDE.md lines promised on #435: a visible capture
reports its geometry and omits it when nothing was positioned, the comparison
runs on mux-visible only, and the replay is capped at one attempt and latches
per session when it cannot converge.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot mentioned this pull request Sep 19, 2026
Ark0N pushed a commit that referenced this pull request Sep 19, 2026
Conflict resolution against the five PRs that landed while this was in review, plus
the items left for merge on the thread.

The real one was `session-ui.js`. #454 refactored all eight non-Claude `run*()`
functions to funnel through one `_launchQuickStartInstances()` helper that does the
POST itself, while this PR replaced that same POST in each of them with
`_quickStartWithCustomModelConfirm()`. Resolved in the helper rather than seven times
over: the helper now goes through the confirm path, and each body builder carries the
`customModel` spread. `runAntigravity` deliberately does NOT, since antigravity's
`customModelInjection` is `unsupported`; parity with this PR's own per-mode choices is
asserted rather than assumed.

That merge creates a question neither feature had alone: the confirm dialog now runs
inside a loop that can launch up to 20 instances. Both questions it can ask (context
window too small, and loading this will unload the model another session is using) are
decisions about the ENDPOINT, and every instance in a batch targets the same one, so
the answer is taken once and carried to the rest. Without that a 20-instance launch
asks the same question 20 times.

Also: `sse-events.ts` is 161 constants (master added two for remote wake, this adds
one, verified by counting rather than by arithmetic), `server.ts` keeps both new SSE
prefixes, the two comments pointing at code that no longer exists are corrected, and
CLAUDE.md's SSE and route counts move to 161 / ~236 / custom-model (6).

`pumpLlamaSwapLogTail`'s unparsed remainder is now capped at 64 KiB. It only shrank at
a `\n\n` frame boundary, so a backend that streams without one would grow it for the
life of a deliberately indefinite connection.

NOT changed, deliberately: the context warning and the swap-conflict warning still
share one `confirmed` flag with the context check first, so confirming "launch anyway"
on a too-small context also skips the "this unloads it for another session" ask. That
is the author's documented choice and the reviewer's own note calls it minor. Both
fixes are worse to make here than to defer: separate flags are new wire surface landed
unreviewed during a release, and reordering the checks adds a network round trip to a
path that currently short-circuits. Raised as a follow-up instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ark0N pushed a commit that referenced this pull request Sep 19, 2026
…hole tree

A full review of the release tree found seven things, and four of them were mine.

**The gate was red, and I put it there.** Splitting `confirmed` into `confirmedContext`
and `confirmedSwap` changed the wire field without moving three assertions that check
it: `custom-model-one-shot-launch.test.ts` and two in `custom-model-run-menu-ui.test.ts`
(the swap modal and the context modal, each of which already receives exactly the right
per-question flag). Moved, with the titles.

**Worse, my own tests for the split never ran.** The four cases in
`session-custom-model.test.ts` that exist specifically to pin it call `mockRunning()`,
which was declared inside a sibling `describe`, so they threw a ReferenceError during
setup. The split would have shipped with no passing server-side coverage while the gate
reported the failure as four broken tests rather than as four tests that were never
written. `mockRunning` is hoisted to the outer describe.

**The submit verifier pressed Enter into shell panes.** `#455`'s SubmitVerifier resolved
its composer glyph as `promptGlyph ?? '❯'`, and only claude and codex declare one, so
the other eight modes fell back to claude's `❯`. That is also starship's default shell
prompt, and pure's, and spaceship's, and p10k lean's. On such a shell the line
`❯ npm run build` sits on screen for as long as the command runs, the verifier reads it
as an unsubmitted prompt, and re-presses Enter into the running program's stdin up to
nine times on its 2s..60s schedule. Mostly a stray newline; not harmless against a y/N
prompt, `read -p`, an installer or a pager, where it takes the default. The module's own
fileoverview already stated the rule this broke. Now `?? ''`, which
`promptStillInComposer()` already treats as inert, so the verifier runs only for a CLI
that actually declares a composer.

**My #451 dedent removal left a count behind**: "Two rules keep it honest" introducing
three numbered rules.

The rest is documentation the split outran. `confirmedContext`/`confirmedSwap` appeared
in no doc at all, while `docs/api-reference.md` (the SemVer-covered contract) still told
an integrator to retry with `confirmed: true` for both questions, which is precisely the
thing the split exists to stop. Documented there, in `docs/custom-model-endpoints.md`
and in CLAUDE.md. The custom-model changeset gained the split and the `CLAUDE_CONFIG_DIR`
multi-user consequence, both user-visible and both previously absent, and #454's gained
the one exception to its own claim: a Custom Endpoints launch ignores the Instance count
stepper and always starts one session.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@timkjr
timkjr deleted the fix/multi-instance-run-count branch September 19, 2026 16:39
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.

2 participants