fix(coaching): onboarding 500 — drop opaque IContextAssembler factory that broke Wolverine 6 codegen#129
Conversation
…rine 6 codegen Onboarding returned HTTP 500 on every turn after the Marten 9 / Wolverine 6 upgrade (DEC-071). Root cause: Wolverine 6 made ServiceLocationPolicy.NotAllowed the default, and IContextAssembler was registered via an opaque Scoped lambda factory (`sp => new ContextAssembler(...)` — the Slice 1 PR #77 workaround for constructor selection). Wolverine cannot statically construct a lambda factory, falls back to service location, and now throws InvalidServiceLocationException while compiling the OnboardingTurnHandler (SubmitUserTurn) chain. Fix: register IContextAssembler with a concrete implementation type. The 3-arg legacy constructor is already `internal`, so the container unambiguously selects the public 6-arg onboarding-aware constructor — the lambda workaround is now redundant, and a type registration is statically constructible by Wolverine. Removed the then-unused Microsoft.Extensions.Options using. Testing gap (why CI missed it): no green test compiled the production Wolverine handler graph. The *DiResolutionTests only assert GetRequiredService succeeds (which an opaque lambda satisfies); the onboarding handler unit tests call the static Handle() method directly with mocks (bypassing codegen); and the HTTP/bus integration host swaps IPlanGenerationService for a stub that severs the failing dependency edge. Added WolverineCodegenCompositionTests — a fast, host-free guard that fails if any module service uses an opaque Scoped/Transient lambda factory. It fails on the pre-fix code and passes after. Full suite: 1125/1125. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 8 minutes and 44 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
… 3) (#130) The test suite exercises every LLM surface in Replay mode against the committed eval cache (zero real API calls), so the live coaching path is never proven by CI. Capture an explicit live end-to-end validation pass at the point it becomes meaningful — after Slice 3 (Adaptation Loop) completes the onboard -> plan -> log -> adapt loop — with a repeat at MVP-0 close after Slice 4. Backstops the stub/Replay gap proven necessary by the Wolverine 6 onboarding 500 (#129). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Symptom
Every onboarding turn returned HTTP 500 (
POST /api/v1/onboarding/turns) → the UI showed "That didn't go through. Try again?" and stuck on "Sending…". Found during manual verification of the running app (the E2E suite stubs/onboarding/*, so it can't catch this).Root cause
A regression from the Marten 9 / Wolverine 6 upgrade (DEC-071, #125). Wolverine 6 made
ServiceLocationPolicy.NotAllowedthe default — handler codegen now throws if it can't statically construct a dependency.IContextAssemblerwas registered via an opaque Scoped lambda factory (sp => new ContextAssembler(...), the Slice 1 PR #77 constructor-selection workaround). Wolverine can't statically construct a lambda factory, falls back to service location, and throwsInvalidServiceLocationExceptionwhile compiling theOnboardingTurnHandler(SubmitUserTurn) chain:Fix
Register
IContextAssemblerwith a concrete implementation type (AddScoped<IContextAssembler, ContextAssembler>()). The 3-arg legacy ctor is alreadyinternal, so the container unambiguously selects the public 6-arg onboarding-aware ctor — the lambda workaround is now redundant, and a type registration is statically constructible by Wolverine. (Also removed the then-unusedMicrosoft.Extensions.Optionsusing.) Keeps the strictNotAllowedpolicy — the safety net that surfaced this — rather than relaxing it.The testing gap (closed here)
No green test compiled the production Wolverine handler graph:
*DiResolutionTestsonly assertGetRequiredServicesucceeds — which an opaque lambda factory satisfies. They don't exercise Wolverine codegen.static OnboardingTurnHandler.Handle(...)directly with NSubstitute mocks — bypassing the bus/codegen.RunCoachAppFactory) swapsIPlanGenerationServicefor a stub with noIContextAssemblerdependency — severing the exact edge that triggers the failure.Added
WolverineCodegenCompositionTests— a fast, host-free guard that inspects the productionAddApplicationModulesregistrations and fails if any module service uses an opaque Scoped/Transient lambda factory (with anAlwaysUseServiceLocationFor<T>allow-list escape hatch). Fails on the pre-fix code (found {IContextAssembler}), passes after.Verification
ContextAssemblerDiResolutionTestsstill green (6-arg ctor still selected — no re-introduction of the original bug)Follow-up (not in this PR)
A heavier CI guard could compile the whole handler graph (
dotnet run -- codegensmoke or a host-bootAssertWolverineConfigurationIsValid) to catch all future codegen regressions, not just opaque-factory ones. The composition guard here covers the immediate class.