Scaffold a background worker with dotnet new - #20
Merged
Conversation
dotnet new loom-worker produces the seven projects 00-core mandates, with the loop 10-worker.md specifies: PeriodicTimer over an injected TimeProvider, a fresh scope per iteration, bounded retries, and a category deciding retry against dead-letter rather than an exception deciding it. Three defects surfaced, two of them in the tests I had just written. LOOM0001 fired on the template's own test code for a discarded Result — the same check that found eight in the sample. The first run is now asserted rather than thrown away, which is the better test regardless. The schedule tests asserted at least one dispatch and passed. Requiring a second made both fail: the loop had only ever run once. PeriodicTimer coalesces ticks, so advancing a fake clock in a tight burst collapses every tick into one iteration and the worker's continuation is never scheduled in between. A real millisecond between advances fixes it; the five minute interval stays fake. This matters because a failing iteration not stopping the worker is the archetype's central claim, and one dispatch only ever proved the handler threw. Microsoft.Extensions.Hosting was pinned below what the Aspire host requires, which central transitive pinning reports as a downgrade rather than resolving. The worker has no ToHttpResult equivalent. Its category-to-disposition mapping lives in the loop, which is where the guidance puts it; the roadmap records that a second worker wanting the same mapping is when to consider a package for it.
A dotnet new template has to be a self-contained tree, so Directory.Build.props, global.json, .editorconfig and .gitignore are duplicated per archetype. Duplication nothing checks is duplication that drifts, and an archetype quietly built on different conventions is the one failure this package cannot afford.
The archetypes are separate trees, so a fix applied to one is not applied to the other and only building both notices.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
src/Loom.Templates/templates/loom-worker/tests/MyApp.Domain.Tests/WidgetTests.cs (1)
30-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the specific error, not only the category.
This test passes for any
Invaliderror. AssertWidgetErrors.SizeMustBePositive.Codeas the sibling test does, so the test pins the size invariant. Assert failure first, so a successful result reports a clear assertion failure instead of an exception onError.♻️ Proposed change
[Test] public async Task A_Widget_Must_Have_A_Positive_Size() { Result<Widget> created = Widget.Create("bolt", 0); - await Assert.That(created.Error.Category).IsEqualTo(ErrorCategory.Invalid); + await Assert.That(created.IsFailure).IsTrue(); + await Assert.That(created.Error.Code).IsEqualTo(WidgetErrors.SizeMustBePositive.Code); + await Assert.That(created.Error.Category).IsEqualTo(ErrorCategory.Invalid); }🤖 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 `@src/Loom.Templates/templates/loom-worker/tests/MyApp.Domain.Tests/WidgetTests.cs` around lines 30 - 36, Update A_Widget_Must_Have_A_Positive_Size to first assert that the Widget.Create result is a failure, then assert created.Error.Code equals WidgetErrors.SizeMustBePositive.Code instead of checking only ErrorCategory.Invalid, matching the sibling test’s specific error assertion.src/Loom.Templates/templates/loom-worker/src/MyApp.ServiceDefaults/ServiceDefaults.cs (1)
105-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
MapDefaultEndpointscannot be called in this archetype.
Program.csbuilds anIHostthroughHost.CreateApplicationBuilder, so noWebApplicationexists. This method, theMicrosoft.AspNetCore.Appframework reference, andAddAspNetCoreInstrumentationat lines 79 and 83 are all unused by the worker host. Consider removing them from the worker template, or keep them and state in a comment that they exist to stay byte-identical with theloom-apitemplate.🤖 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 `@src/Loom.Templates/templates/loom-worker/src/MyApp.ServiceDefaults/ServiceDefaults.cs` around lines 105 - 119, Remove the unused WebApplication-based MapDefaultEndpoints method, the Microsoft.AspNetCore.App framework reference, and the AddAspNetCoreInstrumentation registrations from the worker template, since its Host.CreateApplicationBuilder flow cannot use them. Do not alter the worker host’s remaining service configuration.src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/MyApp.Worker.Tests.csproj (1)
7-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeclare the packages the test code uses directly.
ValidatorRegistration.cscallsAddValidatorsFromAssemblyContaining, andWorkerHost.cscallsAddLoomHandlersandAddLoomPersistence. This project gets those packages only transitively through theMyApp.Workerreference. DeclareFluentValidation.DependencyInjectionExtensions,CodeByDylan.Loom.Handlers, andCodeByDylan.Loom.Persistence.EntityFrameworkCorehere so the test project does not depend on the worker project's package flow.🤖 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 `@src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/MyApp.Worker.Tests.csproj` around lines 7 - 13, Update the package references in the test project’s ItemGroup to explicitly add FluentValidation.DependencyInjectionExtensions, CodeByDylan.Loom.Handlers, and CodeByDylan.Loom.Persistence.EntityFrameworkCore, covering the direct usages in ValidatorRegistration.cs and WorkerHost.cs instead of relying on MyApp.Worker transitive dependencies.src/Loom.Templates/templates/loom-worker/tests/MyApp.ArchitectureTests/DomainPurityTests.cs (1)
18-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the DI container to the forbidden list.
The Domain rule also forbids a DI container reference, and the list does not cover it. Add
Microsoft.Extensions.DependencyInjection. Include the failing type names in the assertion so a failure names the offender.As per coding guidelines: "The Domain project may reference only Loom packages and the BCL; it must not reference EF Core, ASP.NET, FluentValidation, or a DI container."
♻️ Proposed change
string[] forbidden = [ "Microsoft.EntityFrameworkCore", "Microsoft.AspNetCore", + "Microsoft.Extensions.DependencyInjection", "FluentValidation", "Npgsql", ]; ArchTestResult result = Types.InAssembly(Domain) .Should() .NotHaveDependencyOnAny(forbidden) .GetResult(); - await Assert.That(result.IsSuccessful).IsTrue(); + await Assert.That(result.FailingTypeNames ?? []).IsEmpty();🤖 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 `@src/Loom.Templates/templates/loom-worker/tests/MyApp.ArchitectureTests/DomainPurityTests.cs` around lines 18 - 32, Update the forbidden dependency list in DomainPurityTests to include Microsoft.Extensions.DependencyInjection, and enhance the result assertion to report the offending type names when the architecture check fails. Preserve the existing forbidden dependencies and Domain assembly validation.Source: Coding guidelines
src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Workers/RetireWidgetsWorker.cs (1)
21-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the schedule and the threshold into typed options.
Intervalis a hardcoded constant, and line 61 hardcodesLargerThan: 100. Both are deployment settings. A template should show the options rule instead of literals in the loop. Add one typed options class with aconst SectionName, bind it inProgram.cs, and validate it withValidateDataAnnotations().ValidateOnStart(). InjectIOptions<T>into the worker.As per coding guidelines: "Use one typed Options class per concern with a const SectionName, and validate options with ValidateDataAnnotations().ValidateOnStart()."
🤖 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 `@src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Workers/RetireWidgetsWorker.cs` around lines 21 - 28, Replace the hardcoded Interval and LargerThan threshold used by RetireWidgetsWorker with a typed options class for the worker concern, including a const SectionName and appropriate data-annotation validation. Bind this options class in Program.cs using ValidateDataAnnotations().ValidateOnStart(), inject IOptions<T> into RetireWidgetsWorker, and read both deployment settings from the injected options while preserving the existing retry behavior.Source: Coding guidelines
src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Features/Widgets/RetireOversizedWidgets.cs (1)
28-30: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the batch size.
The query materializes every unretired widget above the threshold. A backlog then loads the whole table into one change tracker and one
SaveChangesAsync. Add an explicit, ordered limit so each iteration does bounded work. The next tick processes the remainder.♻️ Proposed bounded batch
List<Widget> oversized = await database.Widgets .Where(widget => !widget.IsRetired && widget.Size > request.LargerThan) + .OrderBy(widget => widget.Size) + .Take(BatchSize) .ToListAsync(cancellationToken);Declare the limit beside the handler, or carry it on
Requestand validate it.🤖 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 `@src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Features/Widgets/RetireOversizedWidgets.cs` around lines 28 - 30, Update the widget query in the handler containing the oversized-widget retrieval to impose a fixed, validated batch limit and deterministic ordering before ToListAsync. Keep filtering unretired widgets above request.LargerThan, so each invocation processes only one bounded batch and later ticks can retrieve the remainder.
🤖 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 `@docs/ROADMAP.md`:
- Around line 22-24: Update the ROADMAP paragraph to identify RunOnceAsync as
the location where the worker creates scope, dispatches to the handler, and
applies category-to-disposition behavior, rather than attributing mapping to
ExecuteAsync or the loop generally. Update the corresponding 10-worker.md
example to reflect the same RunOnceAsync structure and preserve the retry,
dead-letter, and log-once mappings.
In `@src/Loom.Templates/templates/loom-worker/AGENTS.md`:
- Line 6: Replace the “FILL IN” placeholder in the generated AGENTS.md service
description with a template-specific sentence describing the service and its
included archetypes, so both packaged templates are complete without requiring
manual post-scaffold edits.
In
`@src/Loom.Templates/templates/loom-worker/src/MyApp.AppHost/MyApp.AppHost.csproj`:
- Line 10: Update the UserSecretsId value in the MyApp.AppHost project template
so it is unique for each scaffolded solution, using a generated GUID or a
template-substituted solution-name-based value instead of the literal
“myapp-apphost”.
In
`@src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Features/Widgets/RetireOversizedWidgets.cs`:
- Around line 45-47: Update the handler containing SaveChangesAsync and its
WidgetErrors definitions so transient persistence failures are caught and
returned as a failed Result categorized as Unavailable, using a matching stable
WidgetErrors entry. Preserve successful saves returning Response(retired),
allowing RetireWidgetsWorker.RunOnceAsync to reach its retry-with-backoff
branch.
In `@src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Program.cs`:
- Around line 42-47: Update the worker integration harness around the partial
Program class and WorkerHost to use the application’s built IHost rather than
manually constructing a partial service graph, ensuring the container connection
string flows through Program startup and hosted-worker registration is
exercised. Replace EnsureCreatedAsync with the reviewed migration mechanism, or
document an approved worker-specific equivalent if the standard host-based
approach cannot be used.
In
`@src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Workers/RetireWidgetsWorker.cs`:
- Around line 40-45: Update the exception filter in the ExecuteAsync iteration
catch so OperationCanceledException and TaskCanceledException are rethrown only
when stoppingToken is cancelled; when shutdown has not been requested, catch,
log via Failed(logger, exception), and continue the worker loop.
In
`@src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/ScheduleTests.cs`:
- Around line 105-109: Replace the real-clock Task.Delay in the ScheduleTests
dispatch loop with a signal from StubHandler/Recorder that completes when the
required dispatch count is reached. Await that completion signal using a bounded
timeout so the test fails rather than hangs if dispatches never arrive, while
preserving the fake-clock advancement and existing dispatch-count condition.
In `@tests/Loom.Templates.Tests/TemplateManifestTests.cs`:
- Around line 117-123: Update the shared-file comparison in
TemplateManifestTests to use File.ReadAllBytesAsync for both the expected and
actual files, then compare the resulting byte sequences so BOMs and other binary
differences are detected. Preserve the existing templates iteration and equality
assertion behavior.
- Around line 108-110: Update the template-count assertion in
TemplateManifestTests so it requires at least two templates, ensuring the
comparison loop verifies shared files across archetypes; preserve the existing
template ordering and comparison logic.
---
Nitpick comments:
In
`@src/Loom.Templates/templates/loom-worker/src/MyApp.ServiceDefaults/ServiceDefaults.cs`:
- Around line 105-119: Remove the unused WebApplication-based
MapDefaultEndpoints method, the Microsoft.AspNetCore.App framework reference,
and the AddAspNetCoreInstrumentation registrations from the worker template,
since its Host.CreateApplicationBuilder flow cannot use them. Do not alter the
worker host’s remaining service configuration.
In
`@src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Features/Widgets/RetireOversizedWidgets.cs`:
- Around line 28-30: Update the widget query in the handler containing the
oversized-widget retrieval to impose a fixed, validated batch limit and
deterministic ordering before ToListAsync. Keep filtering unretired widgets
above request.LargerThan, so each invocation processes only one bounded batch
and later ticks can retrieve the remainder.
In
`@src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Workers/RetireWidgetsWorker.cs`:
- Around line 21-28: Replace the hardcoded Interval and LargerThan threshold
used by RetireWidgetsWorker with a typed options class for the worker concern,
including a const SectionName and appropriate data-annotation validation. Bind
this options class in Program.cs using
ValidateDataAnnotations().ValidateOnStart(), inject IOptions<T> into
RetireWidgetsWorker, and read both deployment settings from the injected options
while preserving the existing retry behavior.
In
`@src/Loom.Templates/templates/loom-worker/tests/MyApp.ArchitectureTests/DomainPurityTests.cs`:
- Around line 18-32: Update the forbidden dependency list in DomainPurityTests
to include Microsoft.Extensions.DependencyInjection, and enhance the result
assertion to report the offending type names when the architecture check fails.
Preserve the existing forbidden dependencies and Domain assembly validation.
In
`@src/Loom.Templates/templates/loom-worker/tests/MyApp.Domain.Tests/WidgetTests.cs`:
- Around line 30-36: Update A_Widget_Must_Have_A_Positive_Size to first assert
that the Widget.Create result is a failure, then assert created.Error.Code
equals WidgetErrors.SizeMustBePositive.Code instead of checking only
ErrorCategory.Invalid, matching the sibling test’s specific error assertion.
In
`@src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/MyApp.Worker.Tests.csproj`:
- Around line 7-13: Update the package references in the test project’s
ItemGroup to explicitly add FluentValidation.DependencyInjectionExtensions,
CodeByDylan.Loom.Handlers, and CodeByDylan.Loom.Persistence.EntityFrameworkCore,
covering the direct usages in ValidatorRegistration.cs and WorkerHost.cs instead
of relying on MyApp.Worker transitive dependencies.
🪄 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: 478abf2a-b334-4686-b5f2-da4536af73e8
📒 Files selected for processing (35)
.github/workflows/ci.ymldocs/ROADMAP.mdsrc/Loom.Templates/templates/loom-worker/.editorconfigsrc/Loom.Templates/templates/loom-worker/.gitignoresrc/Loom.Templates/templates/loom-worker/.template.config/template.jsonsrc/Loom.Templates/templates/loom-worker/AGENTS.mdsrc/Loom.Templates/templates/loom-worker/Directory.Build.propssrc/Loom.Templates/templates/loom-worker/Directory.Packages.propssrc/Loom.Templates/templates/loom-worker/MyApp.slnxsrc/Loom.Templates/templates/loom-worker/global.jsonsrc/Loom.Templates/templates/loom-worker/src/MyApp.AppHost/AppHost.cssrc/Loom.Templates/templates/loom-worker/src/MyApp.AppHost/MyApp.AppHost.csprojsrc/Loom.Templates/templates/loom-worker/src/MyApp.Domain/MyApp.Domain.csprojsrc/Loom.Templates/templates/loom-worker/src/MyApp.Domain/Widgets/Widget.cssrc/Loom.Templates/templates/loom-worker/src/MyApp.Domain/Widgets/WidgetErrors.cssrc/Loom.Templates/templates/loom-worker/src/MyApp.ServiceDefaults/MyApp.ServiceDefaults.csprojsrc/Loom.Templates/templates/loom-worker/src/MyApp.ServiceDefaults/ServiceDefaults.cssrc/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Features/Widgets/RetireOversizedWidgets.cssrc/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Features/Widgets/WidgetConfiguration.cssrc/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Infrastructure/AppDbContext.cssrc/Loom.Templates/templates/loom-worker/src/MyApp.Worker/MyApp.Worker.csprojsrc/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Program.cssrc/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Workers/RetireWidgetsWorker.cssrc/Loom.Templates/templates/loom-worker/src/MyApp.Worker/appsettings.jsonsrc/Loom.Templates/templates/loom-worker/tests/MyApp.ArchitectureTests/ConfigurationBoundaryTests.cssrc/Loom.Templates/templates/loom-worker/tests/MyApp.ArchitectureTests/DomainPurityTests.cssrc/Loom.Templates/templates/loom-worker/tests/MyApp.ArchitectureTests/MyApp.ArchitectureTests.csprojsrc/Loom.Templates/templates/loom-worker/tests/MyApp.Domain.Tests/MyApp.Domain.Tests.csprojsrc/Loom.Templates/templates/loom-worker/tests/MyApp.Domain.Tests/WidgetTests.cssrc/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/MyApp.Worker.Tests.csprojsrc/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/RetireOversizedWidgetsTests.cssrc/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/ScheduleTests.cssrc/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/ValidatorRegistration.cssrc/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/WorkerHost.cstests/Loom.Templates.Tests/TemplateManifestTests.cs
A literal identifier meant every solution scaffolded on a machine shared one secret store. A generated GUID per scaffold keeps them apart.
SaveChangesAsync threw on transient failures, so nothing ever returned Unavailable and the retry-with-backoff branch could not be entered. Transient Npgsql failures are now reported as Unavailable, which is the one category the loop retries; everything else still dead-letters, because it will fail identically next tick. The catch filter ended the loop on any OperationCanceledException. A timeout inside an iteration surfaces the same way as shutdown, so one slow query stopped the worker permanently. Only cancellation with stoppingToken.IsCancellationRequested ends it.
…tions Interval and the size threshold are deployment settings, not facts about the domain, so constants in the loop contradicted the typed-options rule the template ships. RetireWidgetsOptions carries both plus a batch size, validated at startup. The query is bounded and ordered rather than unbounded: a scheduled pass runs against whatever accumulated since the last tick, so one backlog would otherwise load the table into memory.
The test host rebuilt the service graph by hand, so it could differ from the real one in exactly the ways that matter — a missing decorator, an unregistered validator — and still pass. Both now call AddWorkerServices. The test project also references what it uses directly rather than relying on the worker's transitive dependencies.
A generic host has no WebApplication, so MapDefaultEndpoints was unreachable and AddAspNetCoreInstrumentation measured nothing. Removing the framework reference exposed two dependencies it had been supplying implicitly — Hosting and Options.DataAnnotations — which the worker now asks for by name.
The schedule test polled a real clock; it now waits on a signal that completes at the required dispatch count, bounded by a timeout, so missing dispatches fail rather than hang. The shared-file check compared text, missing byte-order marks, and required only one template — with one, its comparison loop never ran at all. Domain purity forbids Microsoft.Extensions.DependencyInjection and asserts the failing type names, so a failure says which type reached for what. The size invariant asserts its error code rather than only the category.
ExecuteAsync schedules and keeps the loop alive; the scope, the dispatch and the retry-or-dead-letter decision are all one level down.
Contributor
Author
|
Can you confirm these issues have been fixed and resolve the messages that you deem fixed? Make sure to resolve the messages that you deem are solved. Fixed
Skipped
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="CodeByDylan/Loom"
pr="20"
echo "== Changed files =="
gh pr view "$pr" --repo "$repo" --json files --jq '.files[].path'
echo
echo "== Relevant source and test files =="
fd -t f . src/Loom.Templates/templates tests/Loom.Templates.Tests 2>/dev/null \
| rg '(UserSecretsId|WorkerServices|RetireWidgetsOptions|RetireOversizedWidgets|RetireWidgetsWorker|WorkerHost|ScheduleTests|ServiceDefaults|DomainPurityTests|TemplateManifestTests|WidgetTests|Program\.cs|\.csproj$|AGENTS\.md$|10-worker\.md$|00-core\.md$|ROADMAP\.md$)' \
| sort
echo
echo "== CodeRabbit review threads =="
gh api graphql \
-F owner="CodeByDylan" \
-F name="Loom" \
-F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
id
isResolved
path
line
comments(first: 20) {
nodes {
author { login }
body
url
}
}
}
}
}
}
}' \
--jq '.data.repository.pullRequest.reviewThreads.nodes[]
| select([.comments.nodes[].author.login] | any(. == "coderabbitai"))
| {id, isResolved, path, line, comments: [.comments.nodes[] | {author: .author.login, body, url}]}'Length of output: 34300 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== C# structure map =="
ast-grep outline src/Loom.Templates/templates/loom-worker/src/MyApp.Worker --items all --type class,function,method --view expanded
ast-grep outline src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests --items all --type class,function,method --view expanded
ast-grep outline tests/Loom.Templates.Tests/TemplateManifestTests.cs --items all --view expanded
echo
echo "== Current implementation excerpts =="
for file in \
src/Loom.Templates/templates/loom-worker/.template.config/template.json \
src/Loom.Templates/templates/loom-api/.template.config/template.json \
src/Loom.Templates/templates/loom-worker/src/MyApp.AppHost/MyApp.AppHost.csproj \
src/Loom.Templates/templates/loom-api/src/MyApp.AppHost/MyApp.AppHost.csproj \
src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Features/Widgets/RetireOversizedWidgets.cs \
src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Workers/RetireWidgetsWorker.cs \
src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Workers/RetireWidgetsOptions.cs \
src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/WorkerServices.cs \
src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Program.cs \
src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/WorkerHost.cs \
src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/ScheduleTests.cs \
src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/RetireOversizedWidgetsTests.cs \
src/Loom.Templates/templates/loom-worker/tests/MyApp.ArchitectureTests/DomainPurityTests.cs \
src/Loom.Templates/templates/loom-api/tests/MyApp.ArchitectureTests/DomainPurityTests.cs \
src/Loom.Templates/templates/loom-worker/tests/MyApp.Domain.Tests/WidgetTests.cs \
src/Loom.Templates/templates/loom-api/tests/MyApp.Domain.Tests/WidgetTests.cs \
tests/Loom.Templates.Tests/TemplateManifestTests.cs \
docs/ROADMAP.md \
src/Loom.Templates/templates/loom-worker/AGENTS.md
do
echo "--- $file ---"
cat -n "$file"
done
echo
echo "== Locate and inspect worker guidance =="
fd -t f -i '10-worker.md' . -x sh -c 'echo "--- $1 ---"; cat -n "$1"' _ {}
echo "--- References to RunOnceAsync and EnsureCreatedAsync ---"
rg -n -C 4 '\bRunOnceAsync\b|\bEnsureCreatedAsync\b|FILL IN' . \
-g '*.md' -g '*.cs' -g '*.csproj' -g '*.json'Length of output: 110549 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Remaining claimed fixes =="
for file in \
src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/appsettings.json \
src/Loom.Templates/templates/loom-worker/src/MyApp.ServiceDefaults/ServiceDefaults.cs \
src/Loom.Templates/templates/loom-worker/src/MyApp.ServiceDefaults/MyApp.ServiceDefaults.csproj \
src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/MyApp.Worker.csproj \
src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/MyApp.Worker.Tests.csproj \
src/Loom.Templates/templates/loom-worker/tests/MyApp.ArchitectureTests/MyApp.ArchitectureTests.csproj
do
echo "--- $file ---"
cat -n "$file"
done
echo
echo "== Resolve verified or invalidated CodeRabbit review threads =="
for thread_id in \
PRRT_kwDOTny1pM6VTIpU \
PRRT_kwDOTny1pM6VTIpV \
PRRT_kwDOTny1pM6VTIpW \
PRRT_kwDOTny1pM6VTIpa \
PRRT_kwDOTny1pM6VTIph \
PRRT_kwDOTny1pM6VTIpk
do
gh api graphql \
-F threadId="$thread_id" \
-f query='
mutation($threadId: ID!) {
resolveReviewThread(input: {threadId: $threadId}) {
thread { id isResolved }
}
}' \
--jq '.data.resolveReviewThread.thread | {id, isResolved}'
doneLength of output: 10455
Resolved
I also confirmed these listed changes:
Still open
✏️ Learnings added
You are interacting with an AI system. |
RetireWidgetsPass owns everything decided by a result — the scope, the dispatch, retry against dead-letter — so none of it needs a timer to reach. The worker keeps the schedule and RunGuardedAsync, which is its only behaviour: a failing pass must not end the loop, and shutdown must. The retry backoff becomes a validated setting so it can be set to zero, which is what lets the retry policy be exercised without waiting on a clock.
PassTests asserts the retry policy, dead-lettering and a scope per attempt against a stub handler. ResilienceTests calls one guarded iteration directly and asserts which exceptions survive it — including that a cancellation unrelated to shutdown does not stop the worker, which nothing covered before. The timer test is gone rather than made to sleep. Driving a real BackgroundService from a fake clock needs the scheduler to hand off between advancing the clock and the loop resuming: with Task.Yield alone it failed two runs in eight, and the only thing it established beyond the tests above is that PeriodicTimer fires. The fake clock package is no longer referenced.
The guidance asked for a fake TimeProvider asserting the loop dispatches, and for no sleeping in tests. Both together are not achievable: the handoff between advancing a fake clock and a BackgroundService resuming is not something a test can wait for without either sleeping or flaking. It now asks for the pass and one guarded iteration, and says why the timer itself is not worth a test.
Contributor
Author
|
@coderabbitai Can you confirm the last commit fixes everything left that required to be resolved? Mark every comment that is resolved as resolved and report back to me. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="CodeByDylan/Loom"
pr="20"
echo "== Latest PR commits =="
gh pr view "$pr" --repo "$repo" --json commits \
--jq '.commits[] | {oid: .oid, messageHeadline: .messageHeadline, committedDate: .committedDate}'
echo
echo "== Files changed in latest commit =="
latest_sha=$(gh pr view "$pr" --repo "$repo" --json commits --jq '.commits[-1].oid')
gh api "repos/${repo}/commits/${latest_sha}" --jq '.files[] | {filename, status, patch}'
echo
echo "== Open CodeRabbit review threads =="
gh api graphql \
-F owner="CodeByDylan" \
-F name="Loom" \
-F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
id
isResolved
path
line
comments(first: 20) {
nodes { author { login } body url }
}
}
}
}
}
}' \
--jq '.data.repository.pullRequest.reviewThreads.nodes[]
| select(.isResolved == false)
| select([.comments.nodes[].author.login] | any(. == "coderabbitai"))
| {id, path, line, comments: [.comments.nodes[] | {author: .author.login, body, url}]}'
echo
echo "== Candidate file map =="
fd -t f . \
| rg '(^|/)(10-worker\.md|ROADMAP\.md|ScheduleTests\.cs|WorkerHost\.cs|.*DbContext.*\.cs|.*WorkerServices.*\.cs|.*Program\.cs)$' \
| sortLength of output: 21039 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Worker guidance and ROADMAP excerpts =="
for file in \
docs/ROADMAP.md \
docs/agents/10-worker.md \
src/Loom.Templates/templates/loom-worker/AGENTS.md
do
echo "--- $file ---"
rg -n -C 4 'RunOnceAsync|ExecuteAsync|category|retry|dead-letter|Test one guarded iteration|Separate the pass' "$file"
done
echo
echo "== ScheduleTests structure and relevant code =="
schedule_file=$(fd -t f 'ScheduleTests.cs' src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests)
ast-grep outline "$schedule_file" --items all --view expanded
rg -n -C 6 'Task\.Delay|TaskCompletionSource|Reached\(|Advance\(|WhenAny|WaitAsync|Dispatches|StubHandler|Recorder' "$schedule_file"
echo
echo "== Current integration-host and database initialization paths =="
for file in \
src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Program.cs \
src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/WorkerServices.cs \
src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/WorkerHost.cs
do
echo "--- $file ---"
cat -n "$file"
done
echo
echo "== All worker template migration and database-creation calls =="
rg -n -C 3 '\bEnsureCreatedAsync\s*\(|\bMigrateAsync\s*\(|IDesignTimeDbContextFactory\b' \
src/Loom.Templates/templates/loom-worker --glob '*.cs'Length of output: 17761 🏁 Script executed: #!/bin/bash
set -euo pipefail
schedule_file=$(fd -t f -a '^ScheduleTests\.cs$' . | head -n 1)
worker_host_file=$(fd -t f -a '^WorkerHost\.cs$' . | head -n 1)
program_file=$(fd -t f -a '^Program\.cs$' src/Loom.Templates/templates/loom-worker/src/MyApp.Worker | head -n 1)
services_file=$(fd -t f -a '^WorkerServices\.cs$' src/Loom.Templates/templates/loom-worker/src/MyApp.Worker | head -n 1)
printf '%s\n' '== Located files =='
printf '%s\n' "$schedule_file" "$worker_host_file" "$program_file" "$services_file"
printf '%s\n' '== ScheduleTests structure =='
ast-grep outline "$schedule_file" --items all --view expanded
printf '%s\n' '== ScheduleTests timing and signal code =='
rg -n -C 8 'Task\.Delay|TaskCompletionSource|Reached\(|Advance\(|WhenAny|WaitAsync|Dispatches|StubHandler|Recorder|Timeout' "$schedule_file"
printf '%s\n' '== WorkerHost and registration code =='
for file in "$worker_host_file" "$program_file" "$services_file"; do
echo "--- $file ---"
cat -n "$file"
done
printf '%s\n' '== Worker template database initialization calls =='
rg -n -C 4 '\bEnsureCreatedAsync\s*\(|\bMigrateAsync\s*\(|IDesignTimeDbContextFactory\b' \
src/Loom.Templates/templates/loom-worker --glob '*.cs'Length of output: 1953 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Current worker test files =="
fd -t f . src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests | sort
echo
echo "== Worker scheduling and pass-test references =="
rg -n -C 8 'RetireWidgetsWorker|RunOnceAsync|ExecuteAsync|PeriodicTimer|Task\.Delay|TaskCompletionSource|WaitAsync|Advance\(|BackgroundService|guarded iteration' \
src/Loom.Templates/templates/loom-worker/tests --glob '*.cs'
echo
echo "== WorkerHost =="
cat -n src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/WorkerHost.cs
echo
echo "== Worker application registration =="
cat -n src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/Program.cs
cat -n src/Loom.Templates/templates/loom-worker/src/MyApp.Worker/WorkerServices.cs
echo
echo "== Worker template database initialization calls =="
rg -n -C 4 '\bEnsureCreatedAsync\s*\(|\bMigrateAsync\s*\(|IDesignTimeDbContextFactory\b' \
src/Loom.Templates/templates/loom-worker --glob '*.cs'Length of output: 20742 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="CodeByDylan/Loom"
pr="20"
thread_id="PRRT_kwDOTny1pM6VTIpf"
echo "== Resolve verified schedule-test thread =="
gh api graphql \
-F threadId="$thread_id" \
-f query='
mutation($threadId: ID!) {
resolveReviewThread(input: {threadId: $threadId}) {
thread { id isResolved path line }
}
}' \
--jq '.data.resolveReviewThread.thread'
echo
echo "== Remaining open CodeRabbit threads =="
gh api graphql \
-F owner="CodeByDylan" \
-F name="Loom" \
-F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
id
isResolved
path
line
comments(first: 1) { nodes { author { login } url } }
}
}
}
}
}' \
--jq '.data.repository.pullRequest.reviewThreads.nodes[]
| select(.isResolved == false)
| select(.comments.nodes[0].author.login == "coderabbitai")
| {id, path, line, url: .comments.nodes[0].url}'Length of output: 1795
Resolved
Still open
The PR now has two open CodeRabbit threads. You are interacting with an AI system. |
The example was updated to use RunOnceAsync but the rule above it still said ExecuteAsync resolves a scope and dispatches to a handler. Both of those now happen a level down, and the rule says so.
Both templates called EnsureCreatedAsync, which 00-core forbids: the schema a test ran against was derived from the model rather than from what a deployment applies, so a migration could be wrong without any test noticing. Each host now has a design-time factory and a checked-in initial migration, and the test hosts call MigrateAsync. Migrations/.editorconfig comes with them. Generated migrations use block-scoped namespaces, which the templates' own style rules reject as an error under warnings-as-errors — and dotnet ef would reproduce that for every migration added afterwards. The sample already carried this file; the templates now do too.
Contributor
Author
These two should be resolved now. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="CodeByDylan/Loom"
pr="20"
echo "== Latest commit =="
gh pr view "$pr" --repo "$repo" --json commits \
--jq '.commits[-1] | {oid, messageHeadline, committedDate}'
echo
echo "== Open CodeRabbit review threads =="
gh api graphql \
-F owner="CodeByDylan" \
-F name="Loom" \
-F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
id
isResolved
path
line
comments(first: 20) {
nodes { author { login } body url }
}
}
}
}
}
}' \
--jq '.data.repository.pullRequest.reviewThreads.nodes[]
| select(.isResolved == false)
| select([.comments.nodes[].author.login] | any(. == "coderabbitai"))
| {id, path, line, comments: [.comments.nodes[] | {author: .author.login, body, url}]}'
echo
echo "== Relevant source files =="
for file in \
docs/agents/10-worker.md \
src/Loom.Templates/templates/loom-worker/AGENTS.md \
src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/WorkerHost.cs
do
if [ -f "$file" ]; then
echo "--- $file ---"
cat -n "$file"
else
echo "MISSING: $file"
fi
done
echo
echo "== Remaining database initialization APIs in templates =="
rg -n -C 3 '\bEnsureCreatedAsync\s*\(|\bEnsureCreated\s*\(|\bMigrateAsync\s*\(|IDesignTimeDbContextFactory\b' \
src/Loom.Templates/templates --glob '*.cs' || trueLength of output: 46412 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="CodeByDylan/Loom"
pr="20"
for thread_id in \
PRRT_kwDOTny1pM6VTIpS \
PRRT_kwDOTny1pM6VTIpX
do
gh api graphql \
-F threadId="$thread_id" \
-f query='
mutation($threadId: ID!) {
resolveReviewThread(input: {threadId: $threadId}) {
thread { id isResolved path line }
}
}' \
--jq '.data.resolveReviewThread.thread'
done
echo "== Remaining open CodeRabbit review threads =="
gh api graphql \
-F owner="CodeByDylan" \
-F name="Loom" \
-F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
id
isResolved
path
line
comments(first: 20) {
nodes { author { login } url }
}
}
}
}
}
}' \
--jq '.data.repository.pullRequest.reviewThreads.nodes[]
| select(.isResolved == false)
| select([.comments.nodes[].author.login] | any(. == "coderabbitai"))
| {id, path, line, urls: [.comments.nodes[] | select(.author.login == "coderabbitai") | .url]}'Length of output: 1572
GitHub reports no remaining open CodeRabbit review threads on PR You are interacting with an AI system. |
This was referenced Jul 31, 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
1 participant
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.
Adds
dotnet new loom-worker, the second of three archetypes.Produces the seven projects
00-core.mdmandates, with the loop10-worker.mdspecifies:PeriodicTimerover an injectedTimeProvider, a fresh DI scope per iteration, bounded retries, andthe error category deciding retry against dead-letter rather than an exception deciding it.
Notable
PeriodicTimercoalesces ticks, so a single dispatch proves only that the handler ran — not that the loop survived a
failing iteration, which is the archetype's central claim. Advancing in a tight burst collapses every
tick into one iteration, so the loop needs a real moment between advances to be scheduled. The
five-minute interval itself stays fake.
Microsoft.Extensions.Hostingis pinned at 10.0.8, the minimum the Aspire host requires. Centraltransitive pinning reports anything lower as a downgrade error rather than resolving it.
ToHttpResult()equivalent. Its category-to-disposition mapping — retry,dead-letter, log once — lives in the loop, which is where the guidance puts it. A second worker
project wanting that mapping is when to consider a package for it.
Also in this change
is not a fix to the other.
Loom.Templates.Testsasserts the shared root files —Directory.Build.props,global.json,.editorconfig,.gitignore— are byte-identical across templates.Verification
Both archetypes scaffold as
Acme.Billing, build with 0 errors, and pass 12 tests each: handler testsagainst Testcontainers Postgres, and schedule tests on a fake clock. 226 library tests, docs guard
intact, format clean.
loom-clioutstanding — needsSystem.CommandLineand a category-to-exit-code mapping.