Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Disposition — task 531

**Date:** 2026-11-14
**Outcome:** clean
**Rounds:** 1
**Final open count:** 0

## Summary

Round 1 (general, effort 2) found no issues. `WithNuruTools` on an ASP.NET chain stays the supported usage. The generator starts a Nuru app only from `TimeWarp.Nuru.NuruApp.CreateBuilder`, and the McpHost regression test covers a non-Nuru `CreateBuilder` beside that call. The implementer restatement matches the task.

## Exception log (if accepted-exceptions)

None.

## Escalations

None.
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Review framework — task 531

**Date:** 2026-11-14
**Host task:** kanban/in-progress/531-decide-whether-withnurutools-on-an-aspnet-builder-chain-is-supported-then-fix-the-generator-or-the-doc/
**Diff scope:** Task product delta is commit `19657a81` (`fix(generator): only NuruApp.CreateBuilder starts a Nuru app (531)`) on branch `task/531-decide-whether-withnurutools-on-an-aspnet-builder`: `source/timewarp-nuru-analyzers/generators/interpreter/dsl-interpreter.expressions.cs` and `tests/timewarp-nuru-mcphost-tests/mcphost-07-host-builder-chain.cs`. The branch also contains `origin/feature/overnight-2026-11-14` (through `55696ba5`); that stack is outside this review. `origin/master...HEAD` is not the task delta.
**Plan / brief:** `## Goal (implementer restatement)` in `task.md`. `WithNuruTools` on an ASP.NET chain is supported; the generator was treating every `CreateBuilder` as `NuruApp.CreateBuilder`.
**Effort:** 2 (general only)
**Reviewer roster:** general
**Session IDs:** implement oracle session not reported; review oracle session not reported (Cursor, Grok 4.7)

## Budget (by-diff)

- Lines changed: 318
- Effort: 2
- TCB hits: none
- Roster axes: general
- Turn cap: 120 (--max-turns; cursor uncapped)

## Ground rules

- Reviewers are read-only on product code; they write only under `review/round-N/`
- Severity: bug | suggestion | nit — Status starts as open
- Do not invent issues to fill space; zero issues is a valid outcome
- Address the diff and surrounding call sites; re-verify falsifiable claims by reading the diff and the kitchen `## Results` and `### How to validate` record
- Do not re-run a full build, a full test run, an audit, or the hooks; those belong to the implementer's recorded results, the host audit node, and CI
- Exception: one targeted command may check one suspected bug; the finding names the command and the reason. A full build, full test run, or audit is never that command
- A missing or thin recorded result is a finding, not a reason to run the gates
- Prior rounds are immutable; new work goes in `round-(N+1)/`
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Round 1 — general
**Date:** 2026-11-14
**Scope reviewed:** Commit `19657a81` product files (`dsl-interpreter.expressions.cs`, `mcphost-07-host-builder-chain.cs`), checked against `## Goal (implementer restatement)` and the original task. Surrounding call sites: `DispatchMethodCall`, `HandleNonDslMethod`, `DispatchBuild`, `CreateBuilderLocator`, `BuildLocator.IsConfirmedBuildCall`, `ExtractFromBuildCall`, `WithNuruTools`, and `documentation/user/features/mcp-hosting.md`.

## Summary

`WithNuruTools` is an extension on `IMcpServerBuilder`, and its XML example is the ASP.NET chain the doc already shows, so the pattern is supported and the doc stays. The generator used to start an IR app for every method named `CreateBuilder`. `IsNuruAppCreateBuilder` now requires the bound method's containing type to be `TimeWarp.Nuru.NuruApp`. An unbound call falls back to `CreateBuilderLocator`, which matches only the syntax `NuruApp.CreateBuilder(...)`. Any other `CreateBuilder` falls through to `HandleNonDslMethod`. That ignores a receiver that is not an IR builder, so `builder.Services.AddMcpServer()` is not `NURU_S999`. `DispatchBuild` already returns without recording a Nuru app when the receiver is not an `IrAppBuilder`, and `BuildLocator.IsConfirmedBuildCall` still requires a return type named `NuruApp`, so the host `Build()` does not start extraction.

The regression file puts a stand-in `CreateBuilder` (not `NuruApp`) and the real `AddMcpServer()` + `WithNuruTools` in the same method as a Nuru app. McpHost tests compile with the analyzer (`tests/Directory.Build.props`), and `tests/ci-tests/Directory.Build.props` globs this directory, so the file is in the CI assembly. `AddMcpServer` is the `ModelContextProtocol` 2.4.0 extension the McpHost package already references. `## Results` records `./bin/dev build`, `./bin/dev test`, `ganda repo audit`, and a Web SDK probe of the doc snippet. Those gates were not re-run. The restatement matches the task. No findings.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Round 1 — merged findings
**Date:** 2026-11-14
**Sources:** general

## Counts

| Severity | open | fixed | wontfix |
|----------|------|-------|---------|
| bug | 0 | 0 | 0 |
| suggestion | 0 | 0 | 0 |
| nit | 0 | 0 | 0 |

## Issues

None.

## Duplicates / conflicts

None. One reviewer.
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
# Decide whether WithNuruTools on an ASP.NET builder chain is supported, then fix the generator or the doc

## Description

A `WithNuruTools` call on an ASP.NET builder chain fails the build with `NURU_S999 Unrecognized DSL method: AddMcpServer`, even in a real `Microsoft.NET.Sdk.Web` project referencing TimeWarp.Nuru.McpHost and ModelContextProtocol.AspNetCore. Found by the task 523 consumer check (TimeWarp.Nuru 4.0.0-beta.5 from nuget.org, clean cache; evidence in `kanban/done/523-*/evidence/4.0.0-beta.5/` on branch `task/523-nuru-verification-skill-launch-and-drive-the-real`).

The user doc `documentation/user/.../mcp-hosting.md` (~L50) shows `builder.Services.AddMcpServer()...WithNuruTools(nuruApp)`. The source is the source of truth: `WithNuruTools` is a public extension in `source/timewarp-nuru-mcphost/nuru-mcp-server-builder-extensions.cs` (~line 26). Whether that call is meant to be usable on a non-Nuru builder chain must be decided from that file's intent (it has no Purpose/Design region, see the Purpose/Design task). The doc failing is a docs defect unless the public API says the pattern should work, and the public signature (an extension on the MCP server builder) suggests it should.

Note: the `timewarp-nuru-mcp` server is frozen; `timewarp-nuru-mcphost` (McpHost) is a different package and is not.

## Goal (implementer restatement)

`WithNuruTools` is an extension on `IMcpServerBuilder`, and its own XML `<example>` shows the
ASP.NET chain `builder.Services.AddMcpServer().WithHttpTransport().WithNuruTools(nuruApp)`. So the
pattern is supported, and the build failure is a generator defect, not a doc defect.

Acceptance:

- A method that builds a Nuru app and also calls `WebApplication.CreateBuilder(args)` and
`builder.Services.AddMcpServer()...WithNuruTools(nuruApp)` compiles with no NURU_S999.
- The generator treats a `CreateBuilder` call as the Nuru DSL entry only when it binds to
`TimeWarp.Nuru.NuruApp.CreateBuilder`; other `CreateBuilder` calls (ASP.NET, Host, user types)
are ignored, and their `Build()` is not taken for a Nuru app.
- A regression test runs the doc's chain shape with a non-Nuru `CreateBuilder` receiver and the
real `AddMcpServer()` + `WithNuruTools`, in the same method as a Nuru app.
- Touched files carry `#region Purpose` / `#region Design`.
- Gates green: `./bin/dev build`, `./bin/dev test`, `ganda repo audit`.

## Checklist

- [x] Read `nuru-mcp-server-builder-extensions.cs` and the DSL interpreter to establish intent; record in Notes (ask the human if unclear)
- [x] If supported: fix the generator so it does not interpret non-Nuru builder chains (`AddMcpServer`, `builder.Services...`) as the Nuru DSL, then add a regression test using a chain with a non-Nuru receiver
- [x] If not supported: n/a, the pattern is supported (doc left as is)
- [x] Add `#region Purpose` / `#region Design` to the touched files
- [x] Verify with the `tw-nuru-verify` probe in a real Web SDK project (installed package, clean cache)
- [x] `ganda runfile cache --clear`, then `dotnet run tests/ci-tests/run-ci-tests.cs` green (run through `./bin/dev test`)

## Review

- Round 1: Done

## Notes

- Use `.Map<T>()` in tests, not `DiscoverEndpoints()`.
- New analyzer diagnostics go in `AnalyzerReleases.Unshipped.md`, never Shipped.
- Intent: supported. `WithNuruTools` extends `IMcpServerBuilder`, and its XML `<example>` is the
ASP.NET chain `builder.Services.AddMcpServer().WithHttpTransport().WithNuruTools(...)`, the same
chain `mcp-hosting.md` shows. The doc is correct; the generator was wrong.
- Root cause: `InterpretBuildCall` interprets the whole method body around a Nuru `Build()`.
`DispatchMethodCall` matched `CreateBuilder` by name only, so `WebApplication.CreateBuilder(args)`
became an `IrAppBuilder`; `builder.Services` evaluated to that builder, and `AddMcpServer()` on it
hit `HandleNonDslMethod` and threw NURU_S999. The ASP.NET `builder.Build()` was also recorded as a
built Nuru app. `Host.CreateApplicationBuilder` was never affected (different name).
- Fix: `"CreateBuilder" when IsNuruAppCreateBuilder(invocation)` in
`dsl-interpreter.expressions.cs`. It binds the call and requires `TimeWarp.Nuru.NuruApp`; an
unbound call falls back to the `CreateBuilderLocator` syntax check (`NuruApp.CreateBuilder`).
No new diagnostic.

## Results

Generator fix: `source/timewarp-nuru-analyzers/generators/interpreter/dsl-interpreter.expressions.cs`
(semantic `CreateBuilder` gate, Design region added). Regression test:
`tests/timewarp-nuru-mcphost-tests/mcphost-07-host-builder-chain.cs` (stand-in `WebApplication`
with a static `CreateBuilder`, real `AddMcpServer()` + `WithNuruTools`, same method as the Nuru
app). Without the fix the file fails to compile with
`NURU_S999: Unrecognized DSL method: AddMcpServer` (checked by reverting the fix); with it, 2/2 pass.
`mcp-hosting.md` is unchanged because its snippet is the supported usage.

Probe (`tw-nuru-verify` style): fresh `Microsoft.NET.Sdk.Web` project in `/tmp`, empty
`NUGET_PACKAGES`, `TimeWarp.Nuru.McpHost` 4.0.0-beta.13 packed by `./bin/dev build` (source-mapped
to `artifacts/packages`), `ModelContextProtocol.AspNetCore` 2.4.0, `Program.cs` = the doc's L50
snippet. `dotnet build`: Build succeeded, 0 errors. `dotnet run -- --check`: `probe-ok`.

### Gates

| Gate | Command | Result |
|------|---------|--------|
| Build | `./bin/dev build` | Pass: Build completed successfully, 0 warnings, 0 errors |
| Test | `ganda runfile cache --clear` then `./bin/dev test` | Pass: exit 0; CI multi-mode 2197 total, 2191 passed, 6 skipped, 0 failed; standalone 0 failed |
| Audit | `ganda repo audit` | Pass: "Repository passes all audit checks." (first run passed with 1 advisory, `vscode-window-icon`: `.vscode/settings.json` missing; `ganda repo audit --fix` wrote the git-ignored local file, rerun clean) |

### How to validate

Smoke:

```bash
ganda runfile cache --clear
dotnet run tests/timewarp-nuru-mcphost-tests/mcphost-07-host-builder-chain.cs
```

Expect: both `McpHostHostBuilderChain` tests pass (tools `hbc-ping` and `hbc-greet` registered;
the Nuru app beside the host builder still dispatches). Reverting the one-line `"CreateBuilder"
when IsNuruAppCreateBuilder(...)` guard makes the file fail to build with
`NURU_S999: Unrecognized DSL method: AddMcpServer`.

### Review

Round 1, effort 2, roster general. Counts: bug 0, suggestion 0, nit 0; open 0. Disposition: clean.
No wontfix and no escalation. The implementer restatement matches the task. Paths:
`review/review-framework.md`, `review/round-1/general.md`, `review/round-1/merged.md`,
`review/disposition.md`.

## Session

- Created: 7dce871f-a50c-41b9-b2b5-aefa4c4ad890 (2026-10-23)
- Implement oracle: implemented by implementer-claude (claude, vendor anthropic, model claude-opus-5-5), session not reported, max-turns 500, Done — 2026-11-13T22:36:41Z
- Review oracle: reviewed by reviewer-grok (grok, model Grok 4.7), session not reported, max-turns uncapped (budget ladder 120), effort 2, roster general, round 1, disposition clean — 2026-11-14
- Review oracle: review by reviewer-cursor (cursor, vendor xai, model grok-4.7-high), session not reported, max-turns uncapped (no --max-turns flag; bound 120), artifacts general.md, merged.md, disposition.md — 2026-11-13T22:43:46Z

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Generator emits null checks on user value-type returns from delegate handlers

## Description

A fluent delegate handler that returns a user value type (an enum, or a user struct) generates code that does
not compile in any assembly. Found by task 529's consumer lane (`generator-75-consumer-surface-no-ivt`):

```csharp
.Map("mode {shade:Shade}").WithHandler((Shade shade) => shade).AsQuery().Done()
public enum Shade { Light, Dark }
```

Generated output:

- `if (result is not null)`: CS0037, cannot convert null to the non-nullable `Shade`
- `result?.ToString() ?? ""` in the JSON fallback: CS0023, `?` on a value type

Cause: `HandlerInvokerEmitter.IsKnownValueType` and `GetOutputStrategy`
(`source/timewarp-nuru-analyzers/generators/emitters/handler-invoker-emitter.output.cs`) decide from the type
*name*, so only BCL value types are known. A user value type is treated as a reference type and goes to
`OutputStrategy.Json` with a null guard.

## Checklist

- [ ] Carry the return type's `IsValueType` (and nullable-ness) from the semantic model into the handler model
- [ ] Skip the null guard and `?.` for non-nullable value types; decide whether enums print raw (`ToString()`) rather than JSON
- [ ] Add a regression case (enum and user struct return) to generator-75 or its own generator test
- [ ] `ganda runfile cache --clear`, then `./bin/dev test` green

## Notes

- Workaround in 529's lane: the handler returns `shade.ToString()`.
- Filed from task 529 (2026-11-14).

## Session

- Created: 1411579 (2026-11-13)
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Custom converter ConstraintAlias in a route pattern generates uncompilable code

## Description

`IRouteTypeConverter.ConstraintAlias` is public API ("email" for `{to:email}`), but the generator never reads it:
`dsl-interpreter.behaviors.cs` builds every `CustomConverterDefinition` with `ConstraintAlias: null`. A route
that uses the alias compiles to broken code with no diagnostic. Found by task 529's consumer lane
(`generator-75-consumer-surface-no-ivt`):

```csharp
.AddTypeConverter(new PointConverter()) // TargetType = typeof(Point), ConstraintAlias => "point"
.Map("point {at:point}").WithHandler((Point at) => at.X).AsQuery().Done()
```

Generated output:

- `int result = __handler_8(at);`: CS0103, `at` is never declared on the positional path
- `at = __ar_at!;` / `at = __raw_at!;` in the JSON/option path: CS0029, string to `Point`

`{at:Point}` (the type name) works.

## Checklist

- [ ] Decide: read `ConstraintAlias` at compile time (a literal-returning property body), or report a diagnostic for an unknown constraint instead of emitting broken code
- [ ] Unknown constraint names never produce uncompilable output (diagnostic instead)
- [ ] Regression test (alias form, and an unknown constraint)
- [ ] `ganda runfile cache --clear`, then `./bin/dev test` green

## Notes

- The samples define aliases (`email`, `semver`) but route only by type name, so nothing exercised the alias form.
- Filed from task 529 (2026-11-14).

## Session

- Created: 1411579 (2026-11-14)
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@
// Expression evaluation and dispatch core for the DSL interpreter.
#endregion

#region Design
// Dispatch is by method name, so a name the DSL shares with other APIs must also be checked
// semantically. CreateBuilder is the entry point: only TimeWarp.Nuru.NuruApp.CreateBuilder starts
// an IR app. WebApplication.CreateBuilder and other CreateBuilder calls in the same method body are
// not Nuru, so their chains (builder.Services.AddMcpServer()...) and Build() are ignored instead of
// failing with NURU_S999 (task 531).
#endregion

namespace TimeWarp.Nuru.Generators;

using RoslynSyntaxNode = Microsoft.CodeAnalysis.SyntaxNode;
Expand Down Expand Up @@ -198,7 +206,7 @@ private bool IsAssignmentToField(AssignmentExpressionSyntax assignment, IFieldSy
{
return methodName switch
{
"CreateBuilder" => CreateNewAppBuilder(),
"CreateBuilder" when IsNuruAppCreateBuilder(invocation) => CreateNewAppBuilder(),

"Map" when IsGenericMapCall(invocation) => DispatchMapEndpoint(invocation, receiver),

Expand Down Expand Up @@ -281,6 +289,25 @@ private bool IsAssignmentToField(AssignmentExpressionSyntax assignment, IFieldSy
/// </summary>
private static IrAppBuilder CreateNewAppBuilder() => new();

/// <summary>
/// Checks that a <c>CreateBuilder</c> call is <c>NuruApp.CreateBuilder</c>, not another API's
/// <c>CreateBuilder</c> (for example <c>WebApplication.CreateBuilder</c>) in the same method body.
/// When the call does not bind, falls back to the syntax <c>NuruApp.CreateBuilder(...)</c>.
/// </summary>
private bool IsNuruAppCreateBuilder(InvocationExpressionSyntax invocation)
{
SymbolInfo symbolInfo = SemanticModel.GetSymbolInfo(invocation, CancellationToken);
IMethodSymbol? methodSymbol = symbolInfo.Symbol as IMethodSymbol
?? symbolInfo.CandidateSymbols.OfType<IMethodSymbol>().FirstOrDefault();

if (methodSymbol is null)
return CreateBuilderLocator.IsPotentialMatch(invocation);

INamedTypeSymbol? containingType = methodSymbol.ContainingType;
return containingType is { Name: "NuruApp" }
&& containingType.ContainingNamespace?.ToDisplayString() == "TimeWarp.Nuru";
}

/// <summary>
/// Handles method calls that are not part of the DSL.
/// For builder receivers, throws an error (unknown DSL method).
Expand Down
Loading
Loading