tui: stop the ctx dialog writing a context the model cannot load - #6
Merged
Conversation
The loop's only positive termination path was unreachable. loop.ts defined COMPLETE_SIGNAL and checked output for it, but runIteration sent the user's prompt verbatim with no wrapper and no system-prompt injection — the model was never told the safe word existed. A repo-wide search found the literal only in loop.ts, in two tests that hand-feed it via a mock LLM, and in design docs; the .skein agent personas emit different tokens entirely (TASK_COMPLETE, ...). Consequence: no loop could ever reach status "completed". Every run ended stalled/max_reached/cancelled/error — all failure-shaped — even when the work finished early. - new loop/completion.ts (import-free, same cycle-avoidance as similarity.ts): contractPart() discloses the token per iteration; matchesCompletion() reads it back tolerating case, whitespace and code fences, but only in the trailing window, and never when the prompt itself carries the token (an echo is not a signal); promptDisablesCompletion() lets create() warn about that up front. - completionToken is now configurable per loop, defaulting to the existing constant, so a loop can align with a persona that signals TASK_COMPLETE. - the user's prompt stays the first content part and is never rewritten. - correct the CLI's inaccurate "omit for back-to-back ralph style" interval help — DefaultIntervalSeconds applies at wait time regardless. Also adds the openspec changes covering this and the follow-on work (session-cancellation-integrity, retire-auto-reply, loop-spec-queue), and gives fix-loop-reliability the specs/ delta it was missing. Co-Authored-By: Claude <noreply@anthropic.com>
Four changes, ordered cheapest-and-most-certain first, from an investigation
into whether an "agent swarm" would actually help.
Key measurement (2026-07-26, same instant):
z4 gpu_util 85% inference={busy:false, in_flight:0, slots_total:1}
rocky gpu_util 99% inference={busy:true, in_flight:1, slots_total:1}
proxmox gpu_util 3% inference={busy:false, in_flight:0, slots_total:1}
skein's get_providers_status exposes gpu_util_pct and not the inference block,
so z4 — 48GB free, model resident, zero requests queued — reads as 85% busy and
gets skipped. rocky's 99% happens to agree with reality, which is what makes it
hard to notice. GPU utilisation measures whether silicon is doing something; it
cannot tell "serving a request" from "holding weights". opencode-skein already
reads the exact signal in placement.ts; skein does not.
- provider-capacity-truth: publish queue depth as a provenance-tagged fact
(exact vs inferred). No new infrastructure. Ship first.
- fleet-instance-presence: instances announce themselves and per-session state
over the mDNS layer that already exists. Includes wedge detection — the
18h hang on 2026-07-25 was invisible because every provider signal was green
(z4 was fine; the client was dead), and nothing watches clients.
- provider-slot-leases: TTL leases + re-verify immediately before dispatch.
Deliberately advisory — losing the race costs a queued request, not
correctness, so consensus would be the wrong trade. Corrects an earlier
assumption: the shared SQLite covers same-host contention exactly but is a
local file, so it does nothing cross-host.
- agent-coordination-bus: MQTT, gated behind the above, with a decision gate
that can stop it. Broker exists (hlab-mosquitto 192.168.1.131:1883) but is
plain-text — 8883/9001 closed — so identifying metadata needs TLS first.
Carries typed facts only; explicitly not a claim registry (no atomic CAS)
and not agent chatter.
Co-Authored-By: Claude <noreply@anthropic.com>
…sation
Measured on the live fleet, one instant:
z4 util= 85% inference={busy:false, in_flight:0, slots_total:1}
rocky util= 99% inference={busy:true, in_flight:1, slots_total:1}
proxmox util= 3% inference={busy:false, in_flight:0, slots_total:1}
z4 was completely free — 48GB, model resident, empty queue — while reading 85%
utilised, because an AMD host pinned at ttl 0 never unloads and idle weights
read high. Any scheduler using utilisation as a busy proxy skips it. rocky's 99%
happens to agree with reality, which is what makes the proxy treacherous: it is
right often enough to look sound. Utilisation says whether silicon is doing
something; only the server knows its queue.
Adds local/capacity.ts producing a normalised snapshot with a `signal` field
distinguishing measured queue depth ("exact") from a utilisation guess
("inferred"), so consumers can weight accordingly. Queue depth always wins where
both exist, in both directions.
Also handles two cases that bite:
- unreachable != idle. A failed probe yields no inFlight/freeSlots/busy at all
rather than zeros, so a scheduler cannot read "not busy" and dispatch into a
hole.
- exact telemetry without slots_total (host with no model resident, observed on
m3 and m5) — busy stays authoritative, host reports free pending a swap-in.
Verified live against all five hosts. placement.ts is untouched: same inputs,
same choices; this makes what it already knows observable.
Co-Authored-By: Claude <noreply@anthropic.com>
… state Session ses_0691e2d30ffe1mwU1XPH5gr2mQ sat wedged for 18h48m on 2026-07-25 with no way to stop it. The provider (z4) reported in_flight 0 — it had finished and moved on — while the client held an ESTABLISHED socket reading a stream that would never deliver another byte. The log shows 30 cancels inside 30ms, then silence. Root cause was ordering. `cancel` used SynchronizedRef.modify, which commits the new state BEFORE running the returned effect. So the first cancel wrote Idle and then parked forever inside Fiber.interrupt on an unresponsive fiber: idleIfCurrent() never ran, onIdle never fired, and the session stayed visibly busy — while every later cancel matched `case "Idle"` and returned Effect.void. One real cancel and 29 no-ops. The first Esc disarmed the escape hatch and no subsequent keypress could recover. - new Cancelling state: `busy` stays true while a cancel is in flight, so the spinner is honest and a second cancel has something to escalate. - cancel uses modifyEffect, committing Idle only once the run is released. - interruption is bounded by a grace period (injectable for tests) and racing an escalate Deferred, so a second cancel skips the remaining wait. - release happens regardless of outcome; an abandoned fiber is logged at WARN rather than holding the session hostage. Two things worth knowing for future edits here: Interruption is signalled on a detached fiber and awaited via a Deferred, not by racing Fiber.interrupt directly. A race must interrupt its loser and wait for it, and the loser would be an await on a fiber that by definition will not die — which reproduces the original hang inside the fix. ensureRunning treats Cancelling like Idle deliberately: pressing Esc and immediately submitting a new prompt must not wait for the outgoing fiber to drain. finishCancel is id-scoped so the in-flight cancellation cannot clobber the replacement run. This is covered by the pre-existing "cancel does not deadlock when replacement work starts before interrupted run exits" test. Adds regression cover including the 30-cancel burst. Runner backs every session, so the full session/loop/effect suites were run: 30/30 runner, 20/20 loop. The 5 failures in session/llm.test.ts are pre-existing — verified identical with this change stashed. Co-Authored-By: Claude <noreply@anthropic.com>
…snapshots Live incident, 2026-07-26: every opencode-skein process on this machine hung with zero forward progress and no surfaced error. Root cause: summarize() ran on every agent step (not once per turn), each call republishing the full turn diff — patch text included — as a new, permanent event row, since the session store is event-sourced and never overwrites. One session had 12,881 message.updated rows / 1.6 GB from this alone, out of a 17 GB shared SQLite file that every concurrent opencode process on the machine contends for. - busy_timeout=5000 on the shared connection so a lock conflict waits and surfaces an error instead of stalling silently. - Per-session reentrancy guard so overlapping summarize() calls can't stack. - Throttle the expensive diff+patch-text persist to once per 5s per session, even single-threaded; the cheap counter reset now only runs alongside an actual full run, so a throttled call no longer flickers the display to zero. - A force:true call at turn completion guarantees the final persisted diff is never left stale by a throttled intermediate step. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… default-model action /models search for a local provider (e.g. "z4", "proxmox") silently dropped the GB size the moment a filter was typed — dialog-select.tsx's flattened row footer showed option.category instead of option.footer, clobbering the size with the provider name rather than showing both. - Add a `provenance` field so a flattened, filtered row shows size and group together (`flattenedFooter()`), instead of one replacing the other. - Show each local provider's total VRAM/unified memory (fetched once per dialog lifetime via /api/hardware, same pattern as the sidebar and dialog-model-ctx) folded into that provenance text. - Add a "Set as default" action alongside Favorite/Set context size, writing the selection to the workspace config so it's used as the fallback model without a restart. - Close a latent footgun in mergeDiscoveredModel: an existing entry with sizeBytes explicitly undefined would silently clobber a freshly discovered size via object-spread precedence. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Concurrent opencode-skein agents (manual TUI instances, /loop sessions) in one checkout switch branches out from under each other — confirmed live: two opencode-skein processes running against the same brick-now checkout during the 2026-07-26 incident, with no separation between them. The external skein orchestrator already avoids this by giving every change its own worktree at ../opencode-worktrees/<slug> (confirmed via this repo's own .skein/coder-context.md handoffs). This has been named as a gap twice in the backlog and punted both times: loop-spec-queue's non-goals call it "a separate change", provider-slot-leases' non-goals call it "a separate problem". This is that change. Phase 1: packages/opencode/src/git/worktree.ts — ensure/merge/cleanup over plain git worktree add/merge --no-ff/remove, following the existing shell-out pattern in snapshot/index.ts. Never pushes, matching loop-spec-queue's authority boundary. Takes repoRoot explicitly so it's testable standalone against a scratch git repo, independent of loop-spec- queue's (currently unimplemented) queue driver. Wiring into /loop's start/stop path is Phase 2, deliberately deferred — see openspec/changes/agent-worktree-isolation/tasks.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…f hardcoded 750ms
The context dialog offered hand-authored presets up to 1M with nothing
checking them against the model's achievable ceiling, and computed its
recommendation from a prompt budget rather than the hard n_ctx. This is
how `--ctx-size 98304` reached a 32k-trained model.
- fetch /api/fit/{model} in the dialog and annotate every preset above
max_fit_ctx (rows stay visible; apply() refuses them)
- cap computeRecommendedCtx at max_fit_ctx; the ceiling outranks
MIN_WORKFLOW_CTX so a 32k model is never told to grow to 64k
- take the current hard ctx from configured_ctx, not limit.context
(= max_safe_ctx, a prompt budget that already reserves output)
- guard local.model.setCtxSize server-side so a bad value cannot get
through when the TUI is bypassed
- sync the stale tui copy of the generated llama-skein types, which was
missing max_fit_ctx and under_configured
Co-Authored-By: Claude <noreply@anthropic.com>
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
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.
Three bugs in the context dialog, verified against the live z4 backend rather than by code reading alone:
1. Presets above the achievable ceiling were silently selectable
PRESETSinpackages/tui/src/local/model-fit.tsincludes98304("96k"), unchecked againstmax_fit_ctx. Log forensics on z4 caught this dialog writing exactly that onto a 32768-trained model.The dialog now polls
getModelFiton its existing 15s timer, annotates over-ceiling rows⚠ above ceiling (32k) — backend cannot load this, andapply()refuses them with a toast without closing.DialogSelectOption.disabledwas deliberately not used — in this repo'sDialogSelectit filters the row out entirely, hiding the reason. (Side finding: that makes thedisabled: trueinfo rows indialog-tuning.tsxinvisible today — pre-existing, left alone.)2.
computeRecommendedCtxwas uncappedClamped only to
[65536, 262144], with no knowledge of the ceiling. With the real numbers above it computed(1416 + 15574) × 26050 / 1416= 312,555 → clamped to 262144 — an 8× over-recommendation on a 32k model.Now takes an optional
maxFitCtx, applied after the existing floor. That ordering is load-bearing: floor-last would leave a 32k model recommended at 64k and make the fix a no-op.3. A units error made every recommendation systematically wrong
current()readprovider.models[id].limit.context, whichprovider.ts:1529fills frommax_safe_ctx— a prompt budget with output reserve and margin already subtracted — whilekvEstMbis the KV for the real hardn_ctx.The skew is measurable: 26050/32768 = 0.795, a systematic ~20% under-recommendation. This is the mechanism behind the operator report that auto ctx "always sets a very small size", and it is a distinct bug from the over-write in the log.
Now uses
configured_ctxfrom/api/fit, falling back tolimit.contextMax(same unit) rather thanlimit.context.Server-side guard
local.tssetModelCtxSizenow probesgetModelFitand returnsfalsewhen the requested size exceedsmax_fit_ctx, so bypassing the TUI does not bypass the check. Unknown ceiling (0) passes through. Kept the endpoint'sSchema.Booleanshape rather than widening it into the JS SDK and desktop app; corrected the now-ambiguous TUI toast instead.Stale hand-copied client
packages/tui/src/local/llama-skein/gen/types.gen.tsis a hand-copied duplicate whose types had nomax_fit_ctxat all, so the fix was not expressible against it. Restored byte-identity with thepackages/opencodecopy.build:llama-skein-clientwas deliberately not run — it reads acontracts/llama-skein.openapi.jsonwith uncommitted local changes.Follow-up worth filing: nothing regenerates the TUI copy, so it will drift again on the next spec change.
Verification
bun run typecheck→ 23 successful, 23 totalbunx prettier --checkon all authored files → all passbunx oxlinton touched files → 3 warnings, all pre-existing linespackages/opencode: 41 pass / 0 failpackages/tui: 189 pass / 1 skip / 9 fail — baseline with the work stashed is 181 pass / the identical 9 failures. This adds 8 passes and zero failures.11 new tests, including a
Bun.servefake llama-skein asserting noPATCHreaches the backend. The guard test was validated by temporarily deleting the guard line and confirming it failed.