Skip to content

fix: context-limit safety net for spawn+resume mode (#346) - #349

Open
ranxianglei wants to merge 2 commits into
masterfrom
2026-08-28_spawn-resume-context-limit
Open

fix: context-limit safety net for spawn+resume mode (#346)#349
ranxianglei wants to merge 2 commits into
masterfrom
2026-08-28_spawn-resume-context-limit

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Root cause (issue #346)

In headless per-message spawn+resume mode the model context limit was never known when the messages-transform pipeline ran:

  1. The system hook is the only writer of state.modelContextLimit, and it runs after messages.transform within a request — so on the (only) request a spawned process handles, the limit is still undefined during all threshold math.
  2. The system hook never persisted the limit it learned (saveSessionState only runs inside the messages-transform pipeline) — the next spawned process started from undefined again. Learned and lost, every message, forever.
  3. The init-time catalog seed (hydrateModelLimitsFromClient, fire-and-forget) races server readiness in spawned processes; nothing retried.

With state.modelContextLimit === undefined, every percentage threshold (maxContextLimit/minContextLimit = "80%", emergencyThresholdPercent, gc.majorGcThresholdPercent = "100%") resolved to undefined, and every consumer treated that as "do nothing" — nudges, batch-cleanup GC, and in-flight tool-output truncation were all silently disabled. Sessions grew to the length-rejection wall (229,479 / 229,535 tokens on a 262144 window + ~17K system + 16K max_tokens) → immediate rejection → exit-0 empty run → infinite retry loop.

A second, independent defect: even with a known limit, in-flight truncation started at 100% of the window — already past the serving wall once system prompt + tool schemas + max_tokens are added.

Changes

  1. Persist the limit — the system hook now saves modelContextLimit + model identity on change, so a freshly spawned process resumes with the limit known. (State file shape is additive; the fields already existed.)
  2. Lazy catalog hydration — on a catalog miss during messages.transform, the catalog is hydrated once per process from client.config.providers() (the server is guaranteed up during a request) before threshold math.
  3. compress.contextLimitFallback (new, default 128000, 0 disables) — when the limit is genuinely unknown, a configurable fallback window drives nudge thresholds, the emergency override, batch cleanup GC, and in-flight truncation. The real model limit always takes precedence. Documented in CONFIGURATION.md/zh + schema + validation.
  4. Overhead-aware in-flight truncation — threshold is now min(gc.majorGcThresholdPercent × limit, limit − systemPromptTokens − 16384) (OUTPUT_RESERVE_TOKENS). With the production numbers (limit 262144, system ~17K, 229479 tokens) truncation now fires at 228760 instead of never. A window that cannot fit the overhead logs an ERROR and bails.
  5. Hard guard — after the full transform pipeline, if post-transform tokens exceed limit − systemPromptTokens − 16384, ACP logs a loud ERROR ("ACP hard guard: …") with the budget breakdown. The exit-0 empty response itself is opencode-core behavior and cannot be changed from a plugin (upstream report candidate).

Also fixed along the way: internal-agent (title/summary/compaction) system prompts no longer overwrite the session limit with their own small model's limit.

Testing

  • 1049/1049 pass (baseline 1029; +20 new tests across tests/model-switch-limits.test.ts, tests/truncate-tools.test.ts, new tests/context-limit-fallback.test.ts).
  • Pre-fix failure check: with the source changes reverted, all 11 behavioral tests fail (lazy hydration, persistence, internal-agent guard, hydrateAndResolve ×3, hard guard, production wall repro, overhead bail, fallback truncation) — re-applying the fixes turns them green.
  • Production repro test: limit 262144, systemPromptTokens 17000, currentTokens 229479 (the exact production token count) → truncation fires; pre-fix it was a no-op.
  • All 10 pre-existing 切换模型导致上下水水平计算错误 #312 model-switch tests pass unchanged (their configs carry no fallback, preserving legacy invalidation semantics).
  • npm run typecheck + npm run build pass.

Rollback / escape hatches

  • "compress": { "contextLimitFallback": 0 } restores the exact legacy behavior.
  • Larger max_tokens deployments: lower gc.majorGcThresholdPercent (the min() keeps the stricter bound).

Devlog: devlog/2026-08-28_spawn-resume-context-limit/ (REQ + DESIGN + WORKLOG).

In headless per-message spawn+resume mode the model context limit was
never known when the messages-transform pipeline ran: the system hook
(its only writer) runs after messages.transform within a request and
never persisted its value, and the init-time catalog seed races server
readiness. Every percentage threshold therefore resolved to undefined
and the entire safety net (nudges, GC, in-flight truncation) was
silently disabled, letting sessions grow to the length-rejection wall
(~229k tokens on a 262144 window) in an infinite empty-response retry
loop.

- Persist modelContextLimit + identity from the system hook on change
- Lazy one-shot catalog hydration during a request on catalog miss
- New compress.contextLimitFallback (default 128000, 0 disables) drives
  thresholds/GC/truncation when the model limit is unknown
- In-flight truncation becomes overhead-aware:
  min(gc threshold, limit - systemPromptTokens - 16384 output reserve)
- ERROR log ("ACP hard guard") when post-transform tokens exceed the
  model budget
- Internal-agent (title/summary/compaction) system prompts no longer
  overwrite the session limit

Tests: 1049 pass (20 new); all 11 behavioral tests verified to fail
with the source changes reverted.
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

📦 Built Plugin Artifact

Branch: 2026-08-28_spawn-resume-context-limit (24626ae)

Option A — Install from npm PR tag (recommended)

opencode plugin opencode-acp@pr-349 --global

Each push to this PR publishes a new version under the pr-349 npm tag.

Option B — Install from GitHub

opencode plugin "github:ranxianglei/opencode-acp#2026-08-28_spawn-resume-context-limit" --global

Option C — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf opencode-acp-pr349.tgz
cp -r package/dist ~/.cache/opencode/packages/opencode-acp@latest/node_modules/opencode-acp/dist
  1. Restart opencode to pick up changes.

This comment is automatically updated on each push.

Source:
- Correct the truncation-threshold comment (dual-regime rationale:
  provider-usage counts already include the system prompt — margin;
  fallback counts exclude it — exact bound)
- Throttle the 'window too small' ERROR to once per session
- hydrateAndResolve: in-flight promise instead of a boolean so
  concurrent callers share one hydration
- System hook: only write the model identity pair when present (a limit
  without identity no longer clobbers the #312 staleness pair)
- Extend contextLimitFallback docs (switch-invalidation case, per-model
  precedence) in config.ts, dcp.schema.json, CONFIGURATION.md/zh

Tests:
- Restore 7 hollowed truncation tests (tiny windows now bail before the
  protection/skip branches run) — branches execute again
- Add the §5.7 multi-turn growth-cycle test (fallback-only config,
  preserveRecentMessages 20; asserts shouldInjectThisTurn, baseline, and
  anchor sets per turn; turn-anchor assertion is the pre-fix discriminator)
- Poll for the persisted state file instead of a fixed sleep; restore
  XDG env vars in finally
- Stub hydrateAndResolve mirrors the real once-per-process hydration
- Remove the phantom 'strategies' config field from the new test factory
- Faster MEDIUM_OUTPUT fixture (2201 tokens, verified) for the new
  truncation tests; correct stale threshold comments

Verification bugs found and fixed in this round: state.sessionId is
string | null (throttle key), missing lib/messages/inject barrel import.
@ranxianglei

Copy link
Copy Markdown
Owner Author

[bot] Dual-agent review complete (independent source + test reviewers, per AGENTS.md §5.3/§5.6). No blockers from either. Follow-up commit 24626ae addresses the findings; suite is now 1050/1050 (was 1049, +1 §5.7 test), typecheck + build clean.

Source (reviewer 1) — all minors, all fixed:

  • Threshold comment corrected: the systemPromptTokens term is intentional — provider-usage counts already include the system prompt (conservative margin there), while fallback counts exclude it (exact bound there); dropping the term would re-break the production case. Comment now says so.
  • "Window too small" ERROR throttled to once per session (module-level Set).
  • hydrateAndResolve now stores the in-flight promise instead of a boolean — concurrent callers share one hydration instead of one skipping it.
  • System hook only writes the model identity pair when present — a limit without identity can no longer clobber the 切换模型导致上下水水平计算错误 #312 staleness pair.
  • contextLimitFallback docs extended (applies after switch-invalidation too; per-model modelMaxLimits/modelMinLimits take precedence) in config.ts, dcp.schema.json, CONFIGURATION.md/zh.
  • Deferred (pre-existing, out of scope): non-atomic state-file write in saveSessionState.

Tests (reviewer 2) — 2 majors, all fixed:

  • 7 pre-existing truncation tests were silently hollowed out by the earlier makeState(1000) → tiny-window bail (protection/skip branches never executed). Restored to realistic 200K windows; the branches now run.
  • §5.7 gap closed: new multi-turn growth-cycle test (growth cycle: fallback drives the nudge across turns…) — fallback-only config, preserveRecentMessages: 20, three turns, asserting shouldInjectThisTurn + lastPerMessageNudgeTokens + anchor sets after every turn. The turn-2 anchor assertion is the pre-fix discriminator (pre-fix: overMinLimit never true → no anchors ever).
  • Minors: poll for the persisted state file instead of a 100ms sleep; XDG env vars restored in finally; stub hydrateAndResolve mirrors once-per-process hydration; phantom strategies field removed from the new factory; faster verified fixture (MEDIUM_OUTPUT, 2201 tokens) for the two new truncation tests; stale threshold comments corrected.
  • Deferred (pre-existing): config-factory gaps in older test files (tests are excluded from typecheck; runtime unaffected).

Counting basis for the pre-fix failure check (PR body said "11"): 16 tests assert the new behavior (19 if the fallback test file's import failure is counted); the "11" = the 11 named behavioral tests — the remaining 5 (hard-guard-silent, 2 fixed compat tests, etc.) pass by design pre-fix as controls.

Two verification bugs were caught and fixed in this round: state.sessionId is string | null (throttle key) and a missing lib/messages/inject barrel import in the new test file.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · vllm-qwen/qwen3.8-27b

[bot] Independent verification of final PR state (head 24626ae) — all green.

Re-ran the full verification suite on the branch after the follow-up review commit, and did a sanity pass over the diff:

Local verification (this session, on 2026-08-28_spawn-resume-context-limit @ 24626ae):

  • npm test: 1050/1050 pass, 0 fail (matches the post-review count in the review summary; the PR body's 1049 was pre-§5.7-test)
  • npm run typecheck: clean
  • npm run build: clean (dist/index.js 418 KB)

CI (GitHub Actions on 24626ae): pr-validation, test (22), test (24), build, e2e, build-artifact — all success.

PR state: open, mergeable, mergeable_state: clean, not draft, 19 files / 2 commits, base master. package.json version untouched (no bump on a fix branch, per §5.1.1.1). Devlog devlog/2026-08-28_spawn-resume-context-limit/ present with REQ + DESIGN + WORKLOG (DESIGN justified — data-flow changes across hooks/state/gc/inject).

Sanity review of the diff — no issues found:

  • hydrateAndResolve (lib/state/state.ts:112-128) caches the in-flight promise — safe because hydrateFromClient (lib/state/model-limits.ts:47) has an internal try/catch and never rejects (returns 0 on failure), so there's no cached-rejection hazard for subsequent callers.
  • Internal-agent guard ordering is correct: the INTERNAL_AGENT_SIGNATURES early-return (lib/hooks.ts:107) fires before the limit-attribution block (lib/hooks.ts:120), so title/summary/compaction agents on small models can no longer clobber the session limit.
  • System hook writes modelProviderID/modelID only when present (lib/hooks.ts:134-138) — a limit without identity can't clobber the 切换模型导致上下水水平计算错误 #312 staleness pair; saveSessionState fires only on actual change.
  • resolveEffectiveContextLimit (lib/state/utils.ts:419-439): real model limit always takes precedence; fallback (compress.contextLimitFallback, default 128000) only when unknown; undefined when both unavailable — contextLimitFallback: 0 restores exact legacy behavior, as documented.
  • Overhead-aware truncation threshold min(gc.majorGcThresholdPercent × limit, limit − systemPromptTokens − OUTPUT_RESERVE_TOKENS) with the "window too small" ERROR throttled to once per session (module-level Set, lib/messages/truncate-tools.ts).
  • Validation: contextLimitFallback must be a non-negative number (lib/config-validation.ts); schema + CONFIGURATION.md/zh-CN updated.

Status: dual-agent review (per §5.3/§5.6) was completed in the prior round with all findings addressed in 24626ae; this round independently confirms the final state. The PR is ready — merge is a human-only operation (§5.1.1.2). Please merge when ready: #349

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · vllm-qwen/qwen3.8-27b

[bot] Re-checked the thread — this forward re-sent the dual-agent review summary (floor 3); there's no new request beyond what's already addressed, and the PR is unchanged since my last verification:

  • Head still 24626ae (2 commits), state open, mergeable_state: clean, not draft.
  • Dual-agent review (§5.3/§5.6) complete, all findings fixed in 24626ae.
  • My independent re-verification (floor 6): 1050/1050 tests, typecheck + build clean, all CI checks green.

Nothing further needed from me. The PR is ready — merge is a human-only operation (§5.1.1.2). When you're ready: #349

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