Skip to content

fix(subagents): keep a settled run findable by the id it was given - #1198

Open
brynne8 wants to merge 1 commit into
agegr:mainfrom
brynne8:fix/subagent-settled-run-findable
Open

brynne8 wants to merge 1 commit into
agegr:mainfrom
brynne8:fix/subagent-settled-run-findable

Conversation

@brynne8

@brynne8 brynne8 commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

What breaks today

A parent dispatches a background Agent run, the child finishes, and the parent's next get_subagent_result({ sessionId }) — for the very id the Agent call just returned — answers "Subagent not found". The parent has no way to check that claim, so it reasonably concludes the child's session was deleted, and either re-does the work or reports a loss that never happened.

Why it happens

Three lookups in get() all come up empty at once, right after a run settles:

  1. Settling deletes the run from the live registry (getSubagentRuns().delete(...) at the six settle points).
  2. The child's AgentSessionWrapper is released once the child goes idle, so there is no live wrapper to read from.
  3. dependencies.resolveSessionPath(sessionId) can still miss a session that settled moments ago.

With all three gone, nothing answered for an id this process had handed out itself.

The fix

  • A settled run moves to getSettledSubagentRuns() — a bounded map (64 entries, insertion order) — instead of vanishing.
  • get() consults it only after both the live wrapper and the session-file read have failed.

What the fallback deliberately cannot do:

  • Shadow a real file read. A fresh read of the session file always wins; the record only fills the hole the lookup leaves.
  • Invent an unknown id. An id that was never run is still reported as unknown (get returns null).
  • Serve a superseded result. resume re-registers the run live, and a re-settled resume overwrites the record.

Test evidence

The new test is behavioural: it resumes/settles a run against the real controller, then takes away both the wrapper (getSession returns nothing, resolveSessionPath returns null) and asserts get() still returns the completed result with the resumed run's text; it also asserts an id nothing ever ran is still null. No source-text assertions.

  • Fails against current main (test kept, fix stashed):
    • bun test lib/subagent-runtime.test.mjs → 14 pass / 1 fail (AssertionError: actual: undefined, expected: "completed")
    • node --experimental-strip-types --test lib/subagent-runtime.test.mjs → same single failure
  • Passes with the fix, both runners: 15 pass / 0 fail
  • Full suite npm test: 2627 tests, 2626 pass, 0 fail, 1 skipped
  • bun x tsc --noEmit → exit 0; bun run lint (eslint) → exit 0

`get_subagent_result` reported "Subagent not found" for a background run that
had finished moments earlier. Settling deletes the live record, the child's
wrapper is released once the child goes idle, and the session file can still
be unresolvable right after it settled: with all three gone, nothing answered
for an id this process had handed out itself, and the parent had no way to
check the claim — it reasonably reads it as the child's session having been
deleted.

- A settled run moves to `getSettledSubagentRuns()` (64 entries, insertion
  order) instead of vanishing. `get()` consults it only after a live wrapper
  and a read of the session file have both failed, so a fresh read of the file
  always wins and an id that was never run is still reported as unknown.
- `resume` re-registers the run live, and a resume that settles again
  overwrites the record, so the fallback cannot serve a superseded result.
- The new test settles a run, then takes away both the wrapper and the path
  lookup and still expects the completed result. It fails without the fix
  (node --test and bun test alike) and passes with it.
@brynne8
brynne8 force-pushed the fix/subagent-settled-run-findable branch from 659a361 to 351e729 Compare October 12, 2026 03:11

This branch has not been deployed

No deployments
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