fix(opencode): keep the session's recorded model on model-less prompts; brake the compaction spiral - #11
Merged
Conversation
Plugin parent-wakeup prompts are delivered over HTTP with a body that structurally cannot carry a model, so prompt resolution fell through to the plugin-injected agent default and then persisted it via setAgentModel: a session whose user selected a 1M-context model was silently switched to the 262K agent default, permanently overwriting the selection after the first wakeup. Both the inference route and the compaction gate then used the smaller window (harness-opencode#36). currentModel can never return empty - its provider.defaultModel() tail always resolves - so a naive reorder would have made the agent default dead code and changed fresh-session behavior. Instead, split it into recordedModel (persisted SessionTable.model, then the newest user message carrying a model; undefined when the session has no recorded selection) and currentModel (recordedModel plus the provider-default fallback, unchanged for its command-path callers). The precedence is now: explicit input.model, then the session's recorded selection, then the agent default, then the provider default - which keeps the agent default authoritative for fresh sessions and fixes model-less prompts on sessions with a recorded selection. Applied in createUserMessage and shellImpl (same omission at both sites). setAgentModel now runs only for explicit input.model selections; a model resolved from the agent default or read back from the session is not a user choice and is no longer written to the session record. The task tool's background-result injection now pins the parent session's recorded model on the wakeup-style prompt it sends; it previously passed agent and variant but no model. The agent-variant test encoded the old precedence (a model-less prompt reverting to the agent default after an explicit selection); it is reordered to keep full variant coverage under the fixed contract and to pin the new sticky-selection behavior. Adds wakeup-precedence, non-persistence, explicit-persistence and background-injection tests.
A failed compaction kept its errored summary row (including partial streamed output) inside the next serialization scope, so every retry serialized strictly more than the last; the awaitingCompactionProgress cycle guard was armed only when compaction.fallback_model was configured; and a completed compaction that reduced nothing handed the loop straight back into compaction. Together: sessions whose compaction request exceeds every configured budget fail forever while growing, and normal requests gate-reject - a bricked session (harness-opencode#31). - filterCompacted now excludes summary rows that are not completed summaries (errored, unfinished, crash remnants, blank) from model scope. This MARKS, never deletes: the durable rows stay for the UI and diagnosis, and no user/assistant history is touched (anomalyco#18), but a failed attempt can no longer enlarge the next request or the next compaction attempt. - The cycle guard is armed after every completed compaction, with or without a fallback model: a rebuilt request that still comes back "compact" terminates the run with the existing progress verdict instead of re-entering compaction. - processCompaction applies the CompactionExecutor verdict semantics on the live path. The post-compaction admission check (previously fallback-only) now runs for every live-path compaction, and an overflow-driven single-shot compaction whose projected continuation does not shrink the projection of the exact pre-compaction state fails terminally with a diagnosable no-reduction error. CompactionExecutor itself is NOT wired in: it executes the bounded chunked rolling-summary model (issue anomalyco#18 hierarchical summarization, out of scope here), so wiring it would replace the execution model rather than brake it; the guard reuses its verdict semantics directly on the live path instead. The no-reduction check is skipped when a prior turn is replayed verbatim: that media-overflow flow strips attachment mass the projections cannot see, so its marginal arithmetic would misjudge a successful strip; the loop-level guard covers re-entry there. Terminal errors stay SessionV1.ContextOverflowError, so the public error surface and the processor's collapsed "Input exceeds context window of this model" message are unchanged for existing callers, while the distinct messages keep a bricked session diagnosable. Updates the filterCompacted unit and boundary tests that pinned the old errored-summary retention, and adds spiral-brake tests: a failed attempt does not enlarge the next serialized scope, a non-reducing overflow compaction fails terminally, the live loop terminates instead of re-compacting, and seeded user history survives byte-identical.
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
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.
Issue for this PR
Issues are disabled on this repo, so these are tracked in the harness repo:
Shodocan/harness-opencode#36 (wakeup drops the session model)
Shodocan/harness-opencode#31 (compaction death spiral) — this is PR 1 of 2
Type of change
What does this PR do?
Two bugs that feed each other.
1. A model-less prompt loses the session's model.
createUserMessageresolvedinput.model ?? ag.model ?? currentModel(sessionID). The workflow plugin posts parent-wakeupprompts with no
model(its client body type cannot carry one), and it injects a model forevery policy agent, so
ag.modelwas always truthy and the session's own recorded selectionwas never reached.
setAgentModelthen persisted that wrong value, so one wakeup permanentlyoverwrote the user's choice. A 1M-context session got switched to a 262K model, which also
moved the compaction gate to 262K and compacted at ~210-230K.
I could not just reorder the
??chain:currentModelends inprovider.defaultModel()withorDie, so it never returns empty andag.modelwould have become dead code, rerouting freshsessions to the provider default. So
currentModelis split —recordedModelreturns only anactual recorded selection (
SessionTable.model, else the newest user message carrying a model)or
undefined— and resolution becomesinput.model ?? recordedModel ?? ag.model ?? provider.defaultModel().setAgentModelis now gated on an explicitinput.model.injectBackgroundResultpasses the parent's recorded model, since that path can.2. A failed compaction made the next one bigger. Compaction scope is everything since the
last completed summary, and a summary row carrying an
erroris neverisCompletedSummary,so it stayed in scope and got re-serialized. Failed attempts pruned nothing while appending
markers, so each retry serialized strictly more. Momentum session
ses_f858b80aeffeniNr1o0hCcFuGfreached ~968K against a 947,520 budget and then bricked.The errored row was only dropped when a
fallback_modelwas configured, and the one cycleguard was armed only in that same case. Nothing checked that a compaction actually shrank
anything —
CompactionExecutorhasno-reductionandpost-compaction-over-budgetverdictsbut is never wired into
src.filterCompactednow excludes non-completed summary rows from model scope (the durable rowsstay),
awaitingCompactionProgressis armed after every completed compaction, theresume-admission check runs for every live-path compaction rather than only the fallback, and a
no-reductionverdict compares symmetrictoModelMessagesEffectprojections before/after.I did not wire
CompactionExecutoritself — that swaps single-shot compaction for its chunkedrolling-summary model, which is anomalyco#18's work, not surgical. Its verdict semantics are reused
directly instead.
History is never deleted (anomalyco#18's guarantee); the brake marks. New terminal errors reuse
SessionV1.ContextOverflowError, the typeprocessor.tsalready collapses to, with distinctmessages, so string-matching callers keep working and a bricked session becomes diagnosable.
Deliberately out of scope: bounding/chunking the compaction input, changing
serialize()'sreasoning mass, an
input + max_tokens <= windowclamp, thereserved: 12000flooring, and anyoverflow.tsarithmetic.overflow.tsis untouched here.How did you verify your code works?
Tests written first and confirmed failing for the intended reason — e.g.
modelIDcame backagent-modelwheresession-modelwas expected, and a non-reducing compaction returnedcontinuewherestopwas expected.New coverage: persisted-vs-agent-default precedence; no spurious persistence on fallback;
explicit
input.modelstill persists (no regression);injectBackgroundResultpreserves theparent model; a failed attempt does not enlarge the next serialized scope; a non-reducing
compaction terminates instead of looping; history intact after both.
Three existing tests encoded the old behaviour and were updated to the new contract rather than
deleted, keeping their history-preservation assertions. One of them gained a stronger assertion
(no assistant row leaks into model scope) alongside the changed count.
prompt.test.ts'sagent-variant test asserted that a model-less prompt reverts to the agent default after an
explicit selection — that reversion is the bug — so it was reordered with fresh-session
behaviour still asserted first.
bun test test/session/ test/tool/: baseline 1087 pass / 8 fail (5 environmental + 3 scaffoldedRED), final 1093 pass / 5 fail. The 5 are environmental and reproduce on clean HEAD via
git stash:tool.writepermissions expects mode 420 and gets 436 (umask), threeworkflow-observer tests, and
provider-transport-wire.test.tsmissing the ephemeral fixture/tmp/workflows-v4.3.0/test/fixtures/provider-transport.ts. None are in a file this PR touches.bun typecheck30/30 (the pre-existing@opencode-ai/pluginTS2307 was a missing workspacelink locally, resolved by
bun install; not a code change).oxlint0 errors, warning countsunchanged.
Screenshots / recordings
Not a UI change.
Checklist