Add specialist lifecycle registry test - #122
Conversation
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAdded an end-to-end test validating the complete lifecycle of background Task execution: launching a child session in background mode with tool registration, querying task output, stopping the running process, resuming execution, and asserting persisted session events and usage accounting. ChangesSpecialist Task Lifecycle
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Blockers
- None found.
Non-Blocking
- None.
Looks Good
- The new
TestRegisteredSpecialistToolsLifecycleexercises the registeredTask,TaskOutput, andTaskStoptools through the real registry path rather than only testing executor helpers directly. - It covers the background launch path end-to-end: child session creation, CLI args (
--init-session-id, stream-json, enabled tools, depth/tag/calling IDs/cwd), background output replay,TaskStopkill behavior, and killed status/exit-code reporting. - It also covers resume accounting through the registered
Tasktool, including resume args, specialist start/stop events, and parent usage rollup from stream-json usage events. - Local validation passed: focused specialist lifecycle/accounting tests,
go test -count=1 ./internal/specialist ./internal/background ./internal/sessions ./internal/tools,go test -count=1 -p 1 ./...,go vet ./...,go build ./cmd/zero,go run ./cmd/zero-release build,go run ./cmd/zero-release smoke, andgit diff --check. - The PR is based on current
origin/main(merge-base == origin/main), so no separate latest-main merge was needed. - GitHub CI is green on
d770099; CodeRabbit also approved.
Verdict: Approved — this is test-only coverage for the specialist lifecycle registry path, and it passes local and CI validation.
Summary:
specialist.RegisterToolsandtools.Registry.Task(worker, run_in_background=true),TaskOutput,TaskStop, andTask(resume=...)sharing the same task/session id.Self-review:
textevents while the task is running, thenTaskStoptransitions the task to killed before the resume path is exercised.Tests:
GOCACHE=/tmp/zero-go-cache go test ./internal/specialist -run TestRegisteredSpecialistToolsLifecycle -count=1GOCACHE=/tmp/zero-go-cache go test ./internal/specialistGOCACHE=/tmp/zero-go-cache go test ./...git diff --cached --checkSummary by CodeRabbit