fix(subagent): surface primary error when fallback retry masks it + journal failure reason (#59) - #69
Merged
Merged
Conversation
…ournal failure reason #59 dogfood finding: a failed fallback retry returned ONLY the fallback's error, masking why the primary (e.g. an explicit model string) failed at all. Investigation showed explicit vs inherited model resolution are the same code path (both strings -> getModel(provider, id)); the observed asymmetry was transient provider failure + this masking, so the fix is diagnosability: - tools/subagent.ts: when the retry also fails, compose the surfaced error with both attempts (primary '<model>': <err>; fallback: <err>). - engine/retry-fallback.ts: same composed-error contract for the lifecycle/bg spawn wrapper (withModelFallbackRetry). - engine/spawnSubagent.ts finishRun: journal the failure reason on the run:ended event (archived failing runs had empty resultSummary and no error field — post-hoc diagnosis from the journal was impossible). Tests: composed-error on both retry paths; run:ended carries error.
…guard in retry wrapper Review findings on PR #69: the journaled run:ended.error rode on a TS spread quirk (excess-property bypass) — declare it on the RunEndedEvent interface so the journal schema is the contract; and mirror the direct path's identical-error dedup guard in withModelFallbackRetry so both composition sites share one contract.
This was referenced Aug 29, 2026
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
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.
Problem (#59)
Dispatching a subagent with an explicit
modelstring appeared to fail where the inherited session model succeeded — the surfaced error named the fallback model only. Two real defects hid behind that symptom:run:endedjournal event carried noerrorfield — the archived failing runs (fl-msbwt1jj,fl-msbwwqpg) have emptyresultSummaryand zero diagnostic content, making post-hoc diagnosis impossible.Investigation (closes the "explicit vs inherited divergence" theory)
resolveAgentModelbuilds the inherit path's model as${parentModel.provider}/${parentModel.id}from the live session model — so both paths resolve via the identicalgetModel(provider, id)call.ModelRuntime.create()in this environment:getModel("Ollama", "glm-5.2:cloud")resolves cleanly (localmodels.jsonprovider key, exact case). No runtime error.Changes
tools/subagent.ts— after a failed fallback retry, compose the surfaced error with both attempts:primary '<model>' failed: <err>; fallback '<model>' failed: <err>.engine/retry-fallback.ts— same composed-error contract inwithModelFallbackRetry(lifecycle/bg spawn sites).engine/spawnSubagent.ts—finishRunjournals the failure reason onrun:ended(conditionally, so successful runs stay unchanged).Tests
run:endedcarries a meaningfulerroron failed runs.pnpm typecheckclean.Closes #59