Conversation
Contributor
📝 WalkthroughWalkthroughAdds API-hang subtask fixtures and tests manual interruption, child resumption, completion, parent restoration, history transitions, and delegation telemetry. ChangesInterrupted subtask resume
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ParentTask
participant Aimock
participant ChildTask
participant TaskHistory
ParentTask->>Aimock: request API-hang child
Aimock-->>ChildTask: keep provider request pending
ChildTask->>TaskHistory: record interrupted status with parent link
ParentTask->>ChildTask: resume interrupted child
ChildTask->>TaskHistory: record completed child and clear awaitingChildId
ChildTask->>ParentTask: restore parent control
ParentTask->>TaskHistory: record parent completion
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
edelauna
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
edelauna
marked this pull request as ready for review
July 17, 2026 02:01
edelauna
requested review from
JamesRobert20,
hannesrudolph,
navedmerchant and
taltas
as code owners
July 17, 2026 02:01
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/vscode-e2e/src/suite/subtasks.test.ts`:
- Around line 44-57: Update waitForAimockRequestContaining to match only the
intended child request: require the child marker and exclude
SUBTASK_API_HANG_CHILD_MARKER’s parent prompt marker, or use an equivalent
child-only probe. Ensure the polling cannot succeed from older or parent journal
entries before the delayed child request reaches Aimock.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dc9b3a62-8a2d-468c-967b-158aef16c7df
📒 Files selected for processing (3)
apps/vscode-e2e/src/fixtures/subtasks.tsapps/vscode-e2e/src/suite/subtasks.test.tssrc/__tests__/history-resume-delegation.spec.ts
github-actions
Bot
added
the
awaiting-review
…pted subtask parent-link)
github-actions
Bot
removed
the
awaiting-review
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/vscode-e2e/src/fixtures/subtasks.ts (1)
199-231: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse a predicate to exclude the parent marker to prevent unintended matches.
Because
SUBTASK_API_HANG_PARENT_PROMPTembedsSUBTASK_API_HANG_CHILD_MARKERverbatim, checking for the child marker usinguserMessage: apiHangChildMatchrisks intercepting the parent's turns (such as the parent resume request, which also has asequenceIndexof 1). This is the exact same failure mode documented and fixed below for Issue#561.Even if the current tests pass due to fixture ordering or sequence counting artifacts, this approach is fragile and violates the established pattern for handling embedded markers.
♻️ Proposed refactor
Replace
userMessagewith apredicatethat explicitly excludes the parent marker, matching the pattern used in theXPROFILEandINTERRUPTfixtures:mock.addFixture({ match: { - userMessage: apiHangChildMatch, + predicate: (req: ChatCompletionRequest) => + requestContains(req, [SUBTASK_API_HANG_CHILD_MARKER]) && + !requestContains(req, [SUBTASK_API_HANG_PARENT_MARKER]), sequenceIndex: 0, }, // Keep the first child response pending long enough for the e2e test to cancel an in-flight API request. latency: 15_000, response: { toolCalls: [ { name: "attempt_completion", arguments: JSON.stringify({ result: SUBTASK_API_HANG_CHILD_RESULT }), id: "call_api_hang_child_completion_002", }, ], }, }) mock.addFixture({ match: { - userMessage: apiHangChildMatch, + predicate: (req: ChatCompletionRequest) => + requestContains(req, [SUBTASK_API_HANG_CHILD_MARKER]) && + !requestContains(req, [SUBTASK_API_HANG_PARENT_MARKER]), sequenceIndex: 1, }, response: {You can then also safely remove the unused
apiHangChildMatchregex definition on line 42.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/vscode-e2e/src/fixtures/subtasks.ts` around lines 199 - 231, Update both fixtures using apiHangChildMatch in the subtasks fixture setup to use a predicate that matches the child marker while explicitly excluding SUBTASK_API_HANG_PARENT_PROMPT, following the existing XPROFILE and INTERRUPT fixture pattern. Remove the now-unused apiHangChildMatch regex definition.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/vscode-e2e/src/fixtures/subtasks.ts`:
- Around line 199-231: Update both fixtures using apiHangChildMatch in the
subtasks fixture setup to use a predicate that matches the child marker while
explicitly excluding SUBTASK_API_HANG_PARENT_PROMPT, following the existing
XPROFILE and INTERRUPT fixture pattern. Remove the now-unused apiHangChildMatch
regex definition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b10afc3e-3628-48aa-af0b-c6b16c7cdc78
📒 Files selected for processing (3)
apps/vscode-e2e/src/fixtures/subtasks.tsapps/vscode-e2e/src/suite/subtasks.test.tssrc/__tests__/history-resume-delegation.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/tests/history-resume-delegation.spec.ts
- apps/vscode-e2e/src/suite/subtasks.test.ts
github-actions
Bot
added
the
awaiting-review
6 tasks
navedmerchant
approved these changes
Jul 20, 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
2 participants
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.
Related GitHub Issue
Closes: #566
Description
Adds regression coverage for issue #566 (subtask fails to return to parent after a manual pause/resume during an API hang). The production fix already shipped on
mainvia #787 (Story 2.4 of #357):cancelTasknow marks a cancelled delegated child"interrupted"instead of severing the parent link, andAttemptCompletionTool/reopenParentFromDelegationaccept an"interrupted"child when reopening the parent. That fix had only isolated unit tests and a generic (non-hang) e2e interrupt/resume test — nothing exercised the full chain, and nothing specifically reproduced the issue's literal API-hang scenario.This PR is test-only, no production code changes:
src/__tests__/history-resume-delegation.spec.ts— new unit test chainingcancelTask's interrupted-status write → resume →attempt_completion's parent reopen in one test.apps/vscode-e2e/src/fixtures/subtasks.ts/apps/vscode-e2e/src/suite/subtasks.test.ts— new e2e test "API-hung interrupted child resumes and returns to parent" that simulates a hung provider request (15s latency fixture) matching the issue's exact repro, cancels it, resumes it, and asserts the parent regains control.The e2e test's readiness gate polls the aimock server's
/__aimock/journalendpoint to confirm the child's request has actually reached the mock before cancelling, rather than inferring readiness from local state.Test Procedure
pnpm --filter zoo-code test history-resume-delegation(orvitest run history-resume-delegationfromsrc/) — 20 tests pass.pnpm test:ci:mockinapps/vscode-e2eruns the new "API-hung interrupted child resumes and returns to parent" test alongside the existing subtask suite.Pre-Submission Checklist
Documentation Updates
Summary by CodeRabbit
New Features
Bug Fixes
Tests