Skip to content

test: cover plan runtime routing races - #83

Closed
jefft72 wants to merge 4 commits into
plan/04-session-testsfrom
plan/05-runtime-tests
Closed

jefft72 wants to merge 4 commits into
plan/04-session-testsfrom
plan/05-runtime-tests

Conversation

@jefft72

@jefft72 jefft72 commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds runtime coverage for prompt admission races around plan continuations, and corrects a plan mode tool expectation that contradicted its own sibling assertion.

Replaces #67.

Reasoning

Prompt admission and session replacement can interleave, and ordering bugs there are easy to introduce and hard to notice.
These tests pin the ordering rather than the implementation, so the coordinator can be refactored without rewriting them.

The plan mode case in the #3592 regression file asserted that leaving plan mode restores only the extension tool, while the assertion thirty lines above it expects the full default set for the same session configuration.
Restoring the full set is the established behaviour, so the expectation was aligned with its sibling rather than the implementation being changed.

Session profile metadata assertions from the original PR are held for the integration stack.

Changes

  • Adds runtime coverage for concurrent prompt admission during a plan rebind
  • Corrects the plan mode tool restoration expectation in the #3592 regression test
  • Removes assertions that depend on session profile metadata

Testing

  • npx vitest --run test/suite/agent-session-runtime.test.ts test/suite/regressions/3592-no-builtin-tools-keeps-extension-tools.test.ts
  • tsgo --noEmit reports zero errors

Exercise replacement reservations, concurrent transitions, and disposal ordering across runtime operations.
Exercise accepted plan routing, profile metadata, and built-in tool suppression across harness activation.
- Concurrent prompt admission during a plan rebind could reorder queued work, and no test pinned the ordering
- Session profile metadata assertions are held for the integration stack because that field does not exist on main
- The new plan mode case asserted that leaving plan mode restores only the extension tool, contradicting the assertion thirty lines above it for the same session configuration
- Restoring the full default set is the established behaviour, so the expectation is aligned with its sibling rather than the implementation being changed

@Issacboy123 Issacboy123 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! These tests cover exclusive runtime transitions, routing races, and tool restoration when built-ins are disabled. They also treat accepted-plan routing as distinct from an ordinary prompt. I found no issues during review.

@Pepps233 Pepps233 closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants