feat: setup-run substrate 1/3 — conversation bridge, R&S executor, start-or-join #37

Merged
bjoern merged 3 commits from feat/setup-run-substrate into main 2026-07-25 21:44:30 +02:00
Member

First slice of Phase 2's final cut (ADR 0017, 0018, 0020) — the plumbing that lets the Research & Setup agent hold a conversation across circuit lifetimes. No UI in this PR; 2/3 is the wizard's step-3 chat on Fluxor, 3/3 the workspace's "run setup research" entry.

What's in

SetupConversation + registry (UseCases/Agents/Setup) — the bridge between the engine-owned attempt and whatever circuit is looking. The executor posts entries and parks on AskAsync; the UI reads History/PendingQuestion and calls Answer. Per-project instances live in a singleton registry (the engine's own lifetime rule): a reconnecting browser finds the transcript and the open question where it left them. Semantics with teeth:

  • cancelling the attempt abandons the pending question (the register/TCS pair, ReferenceEquals-guarded against a successor's question),
  • BeginAttempt drops a torn-down predecessor's stale question — its asker is gone, an answer would land nowhere,
  • an answer with nothing pending loses harmlessly (double-submit race),
  • Changed fires on engine threads; subscribers marshal themselves (the run-monitor bridge precedent).

ResearchSetupExecutor — the first real IStageExecutor. Resolves the key (fails once, pointing at Settings), the model (stored choice or roster default), and the model's vision capability from the HTTP-cached catalog — the roster deliberately doesn't require vision for R&S (ADR 0015), so a text-only pick must keep view_page images off the wire instead of detonating at the provider. Builds the project-bound blueprint with ask_user wired to the conversation. Signal relay: assistant words → user-facing entries, tool calls → quiet working noises, ask_user skipped (AskAsync already posted the question verbatim — "the agent used a tool" chatter would drown the conversation). The relay is a synchronous IProgress, not Progress<T>: Progress<T> marshals through the thread pool and does not preserve order — the ordering test caught the transcript shuffling, and that would have been a real production bug. On success a draft flips to ready via CompleteProjectSetup; a re-run on a ready project just re-researches (the bible tools upsert; ADR 0020's door for Phase-1 projects that were marked ready without real research). Retry-with-distrust: attempts past the first open suspicious of the predecessor's half-done work (the engine clears Error on start, so the preamble keys off Attempt, and a human send-back's Feedback text rides along when present).

StartSetupRun — the shared entry for wizard step 3 and the future workspace button. Refuses a Named project ("upload pages first"), and joins an unsettled setup execution instead of doubling it — two browser tabs share one conversation rather than racing two agents over one bible.

Engine tweak (one hunk) — the stage-executor lookup takes the last registration instead of the first: the stock executor now lives in DI, and the container's own convention is that a later registration (a test harness, a decorator) wins. The integration recovery test relies on exactly this to substitute its counting executor.

Honest notes

  • The conversation is in-memory by design: a process restart loses the transcript but not the run — startup recovery resets the orphaned attempt and it restarts with distrust (documented in the class doc). Persisting chat history is not a Phase-2 requirement.
  • SupportsVisionAsync defaults to true when the catalog is unavailable or the model unknown — a wrong true degrades to the provider rejecting image parts on one attempt, a wrong false would silently blind a capable model.

Tests

+12 (UseCases 132; full suite 412/412 green). Directionally: the ask/answer park-and-resolve cycle with transcript order; the lost answer; the abandoned question on cancel; the stale question dropped by a new attempt; the draft→ready flip with cost, transcript (including that ask_user's ToolCalled is NOT double-posted), roster-default model, and no distrust on attempt 1; the ready re-run leaving state untouched; the missing key failing once with a Settings pointer and zero gateway calls; the distrust preamble present on attempt 2 and absent on attempt 1; the text-only model choice pinning ModelSupportsVision: false through the catalog; the Named refusal; the join-not-double under a parked agent (gateway held open with a TCS, second start returns the same run id); the vanished project.

No browser verification — this slice has no UI surface; the live end-to-end drive comes with 2/3's chat.

🤖 Generated with Claude Code

First slice of Phase 2's final cut (ADR 0017, 0018, 0020) — the plumbing that lets the Research & Setup agent hold a conversation across circuit lifetimes. No UI in this PR; 2/3 is the wizard's step-3 chat on Fluxor, 3/3 the workspace's "run setup research" entry. ## What's in **`SetupConversation` + registry (UseCases/Agents/Setup)** — the bridge between the engine-owned attempt and whatever circuit is looking. The executor posts entries and parks on `AskAsync`; the UI reads `History`/`PendingQuestion` and calls `Answer`. Per-project instances live in a singleton registry (the engine's own lifetime rule): a reconnecting browser finds the transcript and the open question where it left them. Semantics with teeth: - cancelling the attempt abandons the pending question (the register/TCS pair, `ReferenceEquals`-guarded against a successor's question), - `BeginAttempt` drops a torn-down predecessor's stale question — its asker is gone, an answer would land nowhere, - an answer with nothing pending loses harmlessly (double-submit race), - `Changed` fires on engine threads; subscribers marshal themselves (the run-monitor bridge precedent). **`ResearchSetupExecutor`** — the first real `IStageExecutor`. Resolves the key (fails once, pointing at Settings), the model (stored choice or roster default), and **the model's vision capability from the HTTP-cached catalog** — the roster deliberately doesn't require vision for R&S (ADR 0015), so a text-only pick must keep `view_page` images off the wire instead of detonating at the provider. Builds the project-bound blueprint with `ask_user` wired to the conversation. Signal relay: assistant words → user-facing entries, tool calls → quiet working noises, `ask_user` skipped (AskAsync already posted the question verbatim — "the agent used a tool" chatter would drown the conversation). **The relay is a synchronous `IProgress`, not `Progress<T>`**: `Progress<T>` marshals through the thread pool and does not preserve order — the ordering test caught the transcript shuffling, and that would have been a real production bug. On success a draft flips to ready via `CompleteProjectSetup`; a re-run on a ready project just re-researches (the bible tools upsert; ADR 0020's door for Phase-1 projects that were marked ready without real research). Retry-with-distrust: attempts past the first open suspicious of the predecessor's half-done work (the engine clears `Error` on start, so the preamble keys off `Attempt`, and a human send-back's `Feedback` text rides along when present). **`StartSetupRun`** — the shared entry for wizard step 3 and the future workspace button. Refuses a `Named` project ("upload pages first"), and **joins** an unsettled setup execution instead of doubling it — two browser tabs share one conversation rather than racing two agents over one bible. **Engine tweak (one hunk)** — the stage-executor lookup takes the **last** registration instead of the first: the stock executor now lives in DI, and the container's own convention is that a later registration (a test harness, a decorator) wins. The integration recovery test relies on exactly this to substitute its counting executor. ## Honest notes - The conversation is in-memory by design: a process restart loses the transcript but not the run — startup recovery resets the orphaned attempt and it restarts with distrust (documented in the class doc). Persisting chat history is not a Phase-2 requirement. - `SupportsVisionAsync` defaults to `true` when the catalog is unavailable or the model unknown — a wrong `true` degrades to the provider rejecting image parts on one attempt, a wrong `false` would silently blind a capable model. ## Tests +12 (UseCases 132; full suite **412/412 green**). Directionally: the ask/answer park-and-resolve cycle with transcript order; the lost answer; the abandoned question on cancel; the stale question dropped by a new attempt; the draft→ready flip with cost, transcript (including that `ask_user`'s `ToolCalled` is NOT double-posted), roster-default model, and no distrust on attempt 1; the ready re-run leaving state untouched; the missing key failing once with a Settings pointer and zero gateway calls; the distrust preamble present on attempt 2 and absent on attempt 1; the text-only model choice pinning `ModelSupportsVision: false` through the catalog; the Named refusal; the join-not-double under a parked agent (gateway held open with a TCS, second start returns the same run id); the vanished project. No browser verification — this slice has no UI surface; the live end-to-end drive comes with 2/3's chat. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
feat: setup-run substrate 1/3 — conversation bridge, R&S executor, start-or-join
All checks were successful
CI / build (pull_request) Successful in 22s
CI / test (pull_request) Successful in 39s
b84a322ba4
Cut 7's plumbing (ADR 0017, 0018, 0020), no UI yet. SetupConversation is
the bridge between the engine-owned Research & Setup attempt and whatever
circuit is looking: the executor posts into it and parks on AskAsync; the
UI (next PR) reads its history and answers the pending question. It lives
in a per-project singleton registry, so a reconnecting browser finds the
transcript and the open question where it left them; cancelling the
attempt abandons the question, and a new attempt drops a predecessor's
stale one.

ResearchSetupExecutor is the first real IStageExecutor: it resolves the
key, the model (stored choice or roster default), and the model's vision
capability from the catalog (the roster does not require vision here, so
a text-only pick must keep images off the wire), builds the project-bound
blueprint with ask_user wired to the conversation, and relays signals —
the assistant's words go to the user, tool calls become quiet working
noises, and ask_user is skipped because AskAsync already posted the
question verbatim. Signals relay synchronously: Progress<T> marshals
through the thread pool and does not preserve order, and a transcript
must keep the gateway's sequence. On success a draft flips to ready; a
re-run on a ready project just re-researches (the bible tools upsert).
Retry-with-distrust: any attempt past the first opens suspicious of its
predecessor's half-done work, and a human send-back carries its feedback.

StartSetupRun is the shared entry for wizard step 3 and the workspace's
future "run setup research" button: it refuses a Named project, and joins
an unsettled setup execution instead of doubling it — two browser tabs
share one conversation.

Engine tweak: the stage-executor lookup now takes the LAST registration
(the container's own override convention) — the stock executor is
registered in DI now, and a later registration (a test harness, a
decorator) must win.

Tests: +12 (132 UseCases total; full suite 412) — the ask/answer park and
resolve cycle, the lost answer, the abandoned and stale questions, the
draft→ready flip with transcript and cost, the ready re-run, the missing
key failing once with a pointer at Settings, the distrust preamble on
attempt 2 only, the text-only model keeping vision off, the Named refusal,
the join-not-double under a parked agent, and the vanished project.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Summary

Summary
Generated on: 07/25/2026 - 19:32:57
Coverage date: 07/25/2026 - 19:32:46 - 07/25/2026 - 19:32:55
Parser: MultiReport (4x Cobertura)
Assemblies: 6
Classes: 275
Files: 146
Line coverage: 95.1% (6826 of 7174)
Covered lines: 6826
Uncovered lines: 348
Coverable lines: 7174
Total lines: 13156
Branch coverage: 80.1% (1460 of 1822)
Covered branches: 1460
Total branches: 1822
Method coverage: Feature is only available for sponsors

Coverage

Orihon.BlazorAdapter - 95.4%
Name Line Branch
Orihon.BlazorAdapter 95.4% 86.5%
Orihon.BlazorAdapter.Bible.AddBeatRowRequested 100%
Orihon.BlazorAdapter.Bible.AddCharacterRowRequested 100%
Orihon.BlazorAdapter.Bible.AddGlossaryRowRequested 100%
Orihon.BlazorAdapter.Bible.AddLoreRowRequested 100%
Orihon.BlazorAdapter.Bible.BibleEffects 92% 79.1%
Orihon.BlazorAdapter.Bible.BibleLoaded 100%
Orihon.BlazorAdapter.Bible.BiblePage 93.3% 80.8%
Orihon.BlazorAdapter.Bible.BibleReducers 92.8%
Orihon.BlazorAdapter.Bible.BibleState 100%
Orihon.BlazorAdapter.Bible.BibleWriteFailed 100%
Orihon.BlazorAdapter.Bible.DeleteBeatRowRequested 100%
Orihon.BlazorAdapter.Bible.DeleteCharacterRowRequested 100%
Orihon.BlazorAdapter.Bible.DeleteGlossaryRowRequested 100%
Orihon.BlazorAdapter.Bible.DeleteLoreRowRequested 0%
Orihon.BlazorAdapter.Bible.LoadBible 100%
Orihon.BlazorAdapter.Bible.ReorderBeatsRequested 0%
Orihon.BlazorAdapter.Bible.SaveSummaryRequested 100%
Orihon.BlazorAdapter.Bible.UpdateBeatRowRequested 100%
Orihon.BlazorAdapter.Bible.UpdateCharacterRowRequested 100%
Orihon.BlazorAdapter.Bible.UpdateGlossaryRowRequested 100%
Orihon.BlazorAdapter.Bible.UpdateLoreRowRequested 100%
Orihon.BlazorAdapter.BlazorAdapterAssembly 100%
Orihon.BlazorAdapter.Debounce 92.5% 88.8%
Orihon.BlazorAdapter.Diagnostics.CircuitError 100%
Orihon.BlazorAdapter.Diagnostics.CircuitErrorPanel 100%
Orihon.BlazorAdapter.Diagnostics.CircuitErrorSink 100% 85.7%
Orihon.BlazorAdapter.Diagnostics.OrihonStoreInitializer 85.7% 66.6%
Orihon.BlazorAdapter.PageWorkspace.CreateRegionRequested 100%
Orihon.BlazorAdapter.PageWorkspace.DeleteRegionRequested 100%
Orihon.BlazorAdapter.PageWorkspace.LoadPageWorkspace 100%
Orihon.BlazorAdapter.PageWorkspace.PageWorkspaceEffects 100% 100%
Orihon.BlazorAdapter.PageWorkspace.PageWorkspaceLoaded 100%
Orihon.BlazorAdapter.PageWorkspace.PageWorkspacePage 88.3% 80.7%
Orihon.BlazorAdapter.PageWorkspace.PageWorkspaceReducers 100% 75%
Orihon.BlazorAdapter.PageWorkspace.PageWorkspaceState 100%
Orihon.BlazorAdapter.PageWorkspace.PageWriteFailed 100%
Orihon.BlazorAdapter.PageWorkspace.RegionCreated 100%
Orihon.BlazorAdapter.PageWorkspace.RegionSaved 100%
Orihon.BlazorAdapter.PageWorkspace.ReorderRegionsRequested 100%
Orihon.BlazorAdapter.PageWorkspace.SaveRegionRequested 100%
Orihon.BlazorAdapter.PageWorkspace.SetPageMetaRequested 100%
Orihon.BlazorAdapter.Projects.CreateProjectRequested 100%
Orihon.BlazorAdapter.Projects.DeleteProjectRequested 100%
Orihon.BlazorAdapter.Projects.FinishSetupRequested 100%
Orihon.BlazorAdapter.Projects.ImportPagesRequested 100%
Orihon.BlazorAdapter.Projects.LoadWizard 100%
Orihon.BlazorAdapter.Projects.PagesImported 100%
Orihon.BlazorAdapter.Projects.ProjectDeleteFailed 100%
Orihon.BlazorAdapter.Projects.ProjectListEffects 100% 100%
Orihon.BlazorAdapter.Projects.ProjectListPage 89.7% 91.1%
Orihon.BlazorAdapter.Projects.ProjectListReducers 100%
Orihon.BlazorAdapter.Projects.ProjectListState 100%
Orihon.BlazorAdapter.Projects.ProjectsLoaded 100%
Orihon.BlazorAdapter.Projects.ProjectWizardEffects 100% 100%
Orihon.BlazorAdapter.Projects.ProjectWizardPage 94.1% 85.4%
Orihon.BlazorAdapter.Projects.ProjectWizardReducers 100%
Orihon.BlazorAdapter.Projects.ProjectWizardState 100%
Orihon.BlazorAdapter.Projects.WizardLoaded 100%
Orihon.BlazorAdapter.Projects.WizardWriteFailed 100%
Orihon.BlazorAdapter.Runs.MonitorRunLoaded 100%
Orihon.BlazorAdapter.Runs.RunChangedBridge 94.1% 91.6%
Orihon.BlazorAdapter.Runs.RunMonitor 100% 97.6%
Orihon.BlazorAdapter.Runs.RunMonitorEffects 100% 100%
Orihon.BlazorAdapter.Runs.RunMonitorReducers 100%
Orihon.BlazorAdapter.Runs.RunMonitorState 100%
Orihon.BlazorAdapter.Settings.AgentModelPicked 100%
Orihon.BlazorAdapter.Settings.AgentModelSaved 100%
Orihon.BlazorAdapter.Settings.AgentModelSaveFailed 100%
Orihon.BlazorAdapter.Settings.KeySaved 100%
Orihon.BlazorAdapter.Settings.KeySaveFailed 100%
Orihon.BlazorAdapter.Settings.ModelOptionsLoaded 100%
Orihon.BlazorAdapter.Settings.ModelOptionsUnavailable 100%
Orihon.BlazorAdapter.Settings.SaveKeyRequested 100%
Orihon.BlazorAdapter.Settings.SettingsEffects 100% 100%
Orihon.BlazorAdapter.Settings.SettingsLoaded 100%
Orihon.BlazorAdapter.Settings.SettingsPage 100% 90.4%
Orihon.BlazorAdapter.Settings.SettingsReducers 100%
Orihon.BlazorAdapter.Settings.SettingsState 100%
Orihon.BlazorAdapter.Uploads.UploadTransfer 96.5% 100%
Orihon.BlazorAdapter.Uploads.UploadTransferProgress 100% 100%
Orihon.BlazorAdapter.Uploads.UploadTransferResult 100%
Orihon.BlazorAdapter.Workspace.CreateChapterRequested 100%
Orihon.BlazorAdapter.Workspace.DeleteChapterRequested 100%
Orihon.BlazorAdapter.Workspace.DeletePageRequested 100%
Orihon.BlazorAdapter.Workspace.DeleteSummaryRequested 100%
Orihon.BlazorAdapter.Workspace.LoadProjectWorkspace 100%
Orihon.BlazorAdapter.Workspace.MovePageRequested 100%
Orihon.BlazorAdapter.Workspace.ProjectWorkspaceEffects 100% 100%
Orihon.BlazorAdapter.Workspace.ProjectWorkspaceLoaded 100%
Orihon.BlazorAdapter.Workspace.ProjectWorkspacePage 96.3% 87.6%
Orihon.BlazorAdapter.Workspace.ProjectWorkspaceReducers 100% 66.6%
Orihon.BlazorAdapter.Workspace.ProjectWorkspaceState 100%
Orihon.BlazorAdapter.Workspace.RenameChapterRequested 100%
Orihon.BlazorAdapter.Workspace.ReorderChaptersRequested 100%
Orihon.BlazorAdapter.Workspace.ReorderPagesRequested 100%
Orihon.BlazorAdapter.Workspace.SaveSummaryRequested 100%
Orihon.BlazorAdapter.Workspace.SetPageKindRequested 100%
Orihon.BlazorAdapter.Workspace.SummaryDeleted 100%
Orihon.BlazorAdapter.Workspace.SummarySaved 100%
Orihon.BlazorAdapter.Workspace.WorkspaceImportRequested 100%
Orihon.BlazorAdapter.Workspace.WorkspaceWriteFailed 100%
Orihon.Domain - 100%
Name Line Branch
Orihon.Domain 100% 100%
Orihon.Domain.Agents.AgentDescriptor 100%
Orihon.Domain.Agents.AgentRoster 100% 100%
Orihon.Domain.Bible.Character 100% 100%
Orihon.Domain.Bible.GlossaryEntry 100% 100%
Orihon.Domain.Bible.LoreEntry 100% 100%
Orihon.Domain.Bible.PageSummary 100%
Orihon.Domain.Bible.StoryBeat 100%
Orihon.Domain.Projects.Project 100% 100%
Orihon.Domain.Projects.ProjectProfile 100%
Orihon.Domain.Runs.Execution 100% 100%
Orihon.Domain.Runs.Run 100%
Orihon.Domain.Settings.AppSetting 100%
Orihon.Domain.Text 100% 100%
Orihon.Domain.Translation.BoundingBox 100%
Orihon.Domain.Translation.Chapter 100%
Orihon.Domain.Translation.Page 100%
Orihon.Domain.Translation.Region 100% 100%
Orihon.Domain.Translation.RegionProfile 100%
Orihon.Infrastructure - 93.7%
Name Line Branch
Orihon.Infrastructure 93.7% 65.7%
Orihon.Infrastructure.Bible.EfBibleStore 100% 100%
Orihon.Infrastructure.DependencyInjection 100%
Orihon.Infrastructure.Gateways.AgentToolAdapter 100%
Orihon.Infrastructure.Gateways.AgentToolAdapter`1 100% 100%
Orihon.Infrastructure.Gateways.HttpWebPageFetcher 95.1% 83.3%
Orihon.Infrastructure.Gateways.OpenRouterLlmGateway 98.9% 84.1%
Orihon.Infrastructure.Persistence.Configurations.AppSettingConfiguration 100%
Orihon.Infrastructure.Persistence.Configurations.ChapterConfiguration 100%
Orihon.Infrastructure.Persistence.Configurations.CharacterConfiguration 100%
Orihon.Infrastructure.Persistence.Configurations.ExecutionConfiguration 100%
Orihon.Infrastructure.Persistence.Configurations.GlossaryEntryConfiguration 100%
Orihon.Infrastructure.Persistence.Configurations.JsonColumnMapper 100%
Orihon.Infrastructure.Persistence.Configurations.LoreEntryConfiguration 100%
Orihon.Infrastructure.Persistence.Configurations.PageConfiguration 100%
Orihon.Infrastructure.Persistence.Configurations.PageSummaryConfiguration 100%
Orihon.Infrastructure.Persistence.Configurations.ProjectConfiguration 100%
Orihon.Infrastructure.Persistence.Configurations.RegionConfiguration 100%
Orihon.Infrastructure.Persistence.Configurations.RunConfiguration 100%
Orihon.Infrastructure.Persistence.Configurations.StoryBeatConfiguration 100%
Orihon.Infrastructure.Persistence.Converters.UtcTicksConverter 100%
Orihon.Infrastructure.Persistence.Migrations.AddAppSettings 99.3%
Orihon.Infrastructure.Persistence.Migrations.AddRuns 99.1%
Orihon.Infrastructure.Persistence.Migrations.InitialTranslationDomain 97.3%
Orihon.Infrastructure.Persistence.Migrations.OrihonDbContextModelSnapshot 100%
Orihon.Infrastructure.Persistence.OrihonDbContext 100%
Orihon.Infrastructure.Persistence.OrihonDbContextFactory 100%
Orihon.Infrastructure.Projects.EfProjectStore 100% 100%
Orihon.Infrastructure.Projects.FileSystemPageImageStore 100% 100%
Orihon.Infrastructure.Runs.EfRunStore 97% 50%
Orihon.Infrastructure.Settings.EfAppSettingsStore 100% 100%
Orihon.Infrastructure.Translation.EfChapterStore 100% 100%
Orihon.Infrastructure.Translation.EfPageStore 100% 100%
Orihon.Infrastructure.Translation.EfRegionStore 100% 100%
Orihon.Infrastructure.Translation.Ordering 100% 100%
System.Text.RegularExpressions.Generated 70.6% 53.3%
System.Text.RegularExpressions.Generated.<RegexGenerator_g>F7FCA343D2B99030
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
77.9% 76.6%
System.Text.RegularExpressions.Generated.<RegexGenerator_g>F7FCA343D2B99030
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
59% 42.5%
System.Text.RegularExpressions.Generated.<RegexGenerator_g>F7FCA343D2B99030
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
89.4% 75%
System.Text.RegularExpressions.Generated.<RegexGenerator_g>F7FCA343D2B99030
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
83.7% 62.5%
Orihon.Kernel - 90.9%
Name Line Branch
Orihon.Kernel 90.9% 75%
Orihon.Kernel.Err`1 100%
Orihon.Kernel.Ok`1 100%
Orihon.Kernel.Result`1 88.8% 75%
Orihon.Server - 93.4%
Name Line Branch
Orihon.Server 93.4% 68.4%
Orihon.Server.Components.App 100%
Orihon.Server.Components.Layout.MainLayout 100%
Orihon.Server.Components.Pages.Gate 64.2% 66.6%
Orihon.Server.RunEngineBootstrap 100%
Orihon.Server.Security.AccessGate 91.8% 41.6%
Orihon.Server.Security.AccessSecret 100% 50%
Orihon.Server.VolumeStartupValidator 100% 100%
Program 95.4% 85.7%
Orihon.UseCases - 97.4%
Name Line Branch
Orihon.UseCases 97.4% 89.8%
Orihon.UseCases.Agents.AgentBlueprint 100%
Orihon.UseCases.Agents.AgentInvocation 100%
Orihon.UseCases.Agents.AgentOutcome 100%
Orihon.UseCases.Agents.AgentTool`1 90.9% 75%
Orihon.UseCases.Agents.AgentToolImage 100%
Orihon.UseCases.Agents.AgentToolResult 100%
Orihon.UseCases.Agents.AssistantSpoke 100%
Orihon.UseCases.Agents.ResearchSetup.AddGlossaryParams 100%
Orihon.UseCases.Agents.ResearchSetup.AddGlossaryTool 100% 100%
Orihon.UseCases.Agents.ResearchSetup.AddStoryBeatParams 100%
Orihon.UseCases.Agents.ResearchSetup.AddStoryBeatTool 100% 50%
Orihon.UseCases.Agents.ResearchSetup.AskUserParams 100%
Orihon.UseCases.Agents.ResearchSetup.AskUserTool 100% 100%
Orihon.UseCases.Agents.ResearchSetup.FetchUrlParams 100%
Orihon.UseCases.Agents.ResearchSetup.FetchUrlTool 100% 100%
Orihon.UseCases.Agents.ResearchSetup.ListBibleTool 86.6%
Orihon.UseCases.Agents.ResearchSetup.PageByNumber 90% 87.5%
Orihon.UseCases.Agents.ResearchSetup.ResearchSetupBlueprint 100%
Orihon.UseCases.Agents.ResearchSetup.SetPageSummaryParams 100%
Orihon.UseCases.Agents.ResearchSetup.SetPageSummaryTool 100% 75%
Orihon.UseCases.Agents.ResearchSetup.SetProjectMetadataParams 100%
Orihon.UseCases.Agents.ResearchSetup.SetProjectMetadataTool 96.1% 90.9%
Orihon.UseCases.Agents.ResearchSetup.UpsertCharacterParams 100%
Orihon.UseCases.Agents.ResearchSetup.UpsertCharacterTool 91.3% 66.6%
Orihon.UseCases.Agents.ResearchSetup.UpsertLoreParams 100%
Orihon.UseCases.Agents.ResearchSetup.UpsertLoreTool 91.3% 66.6%
Orihon.UseCases.Agents.ResearchSetup.ViewPageParams 100%
Orihon.UseCases.Agents.ResearchSetup.ViewPageTool 85.7% 50%
Orihon.UseCases.Agents.Setup.ResearchSetupExecutor 96.9% 86.6%
Orihon.UseCases.Agents.Setup.SetupChatEntry 100%
Orihon.UseCases.Agents.Setup.SetupConversation 95.5% 75%
Orihon.UseCases.Agents.Setup.SetupConversationRegistry 100%
Orihon.UseCases.Agents.ToolCalled 100%
Orihon.UseCases.Agents.ToolCompleted 100%
Orihon.UseCases.Bible.AddCharacter 100% 100%
Orihon.UseCases.Bible.AddGlossaryEntry 100% 100%
Orihon.UseCases.Bible.AddLoreEntry 100% 100%
Orihon.UseCases.Bible.AddStoryBeat 100% 100%
Orihon.UseCases.Bible.BibleDto 100%
Orihon.UseCases.Bible.CharacterDto 100%
Orihon.UseCases.Bible.DeleteCharacter 100% 100%
Orihon.UseCases.Bible.DeleteGlossaryEntry 100% 100%
Orihon.UseCases.Bible.DeleteLoreEntry 100% 100%
Orihon.UseCases.Bible.DeletePageSummary 100% 100%
Orihon.UseCases.Bible.DeleteStoryBeat 100% 100%
Orihon.UseCases.Bible.GetBible 100% 100%
Orihon.UseCases.Bible.GlossaryEntryDto 100%
Orihon.UseCases.Bible.LoreEntryDto 100%
Orihon.UseCases.Bible.PageSummaryDto 100%
Orihon.UseCases.Bible.ReorderStoryBeats 100%
Orihon.UseCases.Bible.SetPageSummary 100% 100%
Orihon.UseCases.Bible.StoryBeatDto 100%
Orihon.UseCases.Bible.UpdateCharacter 100% 100%
Orihon.UseCases.Bible.UpdateGlossaryEntry 100% 100%
Orihon.UseCases.Bible.UpdateLoreEntry 100% 100%
Orihon.UseCases.Bible.UpdateStoryBeat 100% 100%
Orihon.UseCases.Chapters.ChapterDto 100%
Orihon.UseCases.Chapters.CreateChapter 100% 100%
Orihon.UseCases.Chapters.DeleteChapter 100% 100%
Orihon.UseCases.Chapters.RenameChapter 100% 100%
Orihon.UseCases.Chapters.ReorderChapters 100%
Orihon.UseCases.DependencyInjection 100%
Orihon.UseCases.Diagnostics.SeedDevData 99.2% 92.8%
Orihon.UseCases.Gateways.LlmKeyInfo 100%
Orihon.UseCases.Gateways.LlmModel 100%
Orihon.UseCases.NextOrder 100%
Orihon.UseCases.Pages.DeletePage 100% 100%
Orihon.UseCases.Pages.GetPage 100% 100%
Orihon.UseCases.Pages.GetProjectWorkspace 100% 100%
Orihon.UseCases.Pages.ImportPages 100% 100%
Orihon.UseCases.Pages.ImportPagesResult 100%
Orihon.UseCases.Pages.MovePage 100% 92.8%
Orihon.UseCases.Pages.PageDetailDto 100%
Orihon.UseCases.Pages.PageDto 100%
Orihon.UseCases.Pages.PageUpload 100%
Orihon.UseCases.Pages.ProjectWorkspaceDto 100%
Orihon.UseCases.Pages.ReorderPages 100%
Orihon.UseCases.Pages.SetPageMeta 100% 100%
Orihon.UseCases.Pages.WorkspaceChapterDto 100%
Orihon.UseCases.Projects.CompleteProjectSetup 100% 93.7%
Orihon.UseCases.Projects.CreateProject 100% 100%
Orihon.UseCases.Projects.DeleteProject 100% 100%
Orihon.UseCases.Projects.GetProject 100% 100%
Orihon.UseCases.Projects.ListProjects 100%
Orihon.UseCases.Projects.ProjectDto 95.8%
Orihon.UseCases.Projects.StartSetupRun 100% 100%
Orihon.UseCases.Projects.StoredPageImage 100%
Orihon.UseCases.Projects.UpdateProjectMetadata 100% 100%
Orihon.UseCases.Regions.CreateRegion 100% 100%
Orihon.UseCases.Regions.DeleteRegion 100% 100%
Orihon.UseCases.Regions.RegionDto 97%
Orihon.UseCases.Regions.ReorderRegions 100%
Orihon.UseCases.Regions.UpdateRegion 100% 100%
Orihon.UseCases.Runs.ExecutionDto 92.3%
Orihon.UseCases.Runs.PlannedExecution 100%
Orihon.UseCases.Runs.RunDto 93.3% 90%
Orihon.UseCases.Runs.RunEngine 93.8% 86.6%
Orihon.UseCases.Runs.RunEngineOptions 100%
Orihon.UseCases.Runs.StageContext 62.5%
Orihon.UseCases.Settings.AgentSettingDto 100% 100%
Orihon.UseCases.Settings.GetSettings 100% 100%
Orihon.UseCases.Settings.ListModelOptions 100% 100%
Orihon.UseCases.Settings.SaveAgentModel 100% 100%
Orihon.UseCases.Settings.SaveOpenRouterKey 100% 100%
Orihon.UseCases.Settings.SettingKeys 100% 100%
Orihon.UseCases.Settings.SettingsDto 100%
<!-- coverage-comment --> # Summary <details open><summary>Summary</summary> ||| |:---|:---| | Generated on: | 07/25/2026 - 19:32:57 | | Coverage date: | 07/25/2026 - 19:32:46 - 07/25/2026 - 19:32:55 | | Parser: | MultiReport (4x Cobertura) | | Assemblies: | 6 | | Classes: | 275 | | Files: | 146 | | **Line coverage:** | 95.1% (6826 of 7174) | | Covered lines: | 6826 | | Uncovered lines: | 348 | | Coverable lines: | 7174 | | Total lines: | 13156 | | **Branch coverage:** | 80.1% (1460 of 1822) | | Covered branches: | 1460 | | Total branches: | 1822 | | **Method coverage:** | [Feature is only available for sponsors](https://reportgenerator.io/pro) | </details> ## Coverage <details><summary>Orihon.BlazorAdapter - 95.4%</summary> |**Name**|**Line**|**Branch**| |:---|---:|---:| |**Orihon.BlazorAdapter**|**95.4%**|**86.5%**| |Orihon.BlazorAdapter.Bible.AddBeatRowRequested|100%|| |Orihon.BlazorAdapter.Bible.AddCharacterRowRequested|100%|| |Orihon.BlazorAdapter.Bible.AddGlossaryRowRequested|100%|| |Orihon.BlazorAdapter.Bible.AddLoreRowRequested|100%|| |Orihon.BlazorAdapter.Bible.BibleEffects|92%|79.1%| |Orihon.BlazorAdapter.Bible.BibleLoaded|100%|| |Orihon.BlazorAdapter.Bible.BiblePage|93.3%|80.8%| |Orihon.BlazorAdapter.Bible.BibleReducers|92.8%|| |Orihon.BlazorAdapter.Bible.BibleState|100%|| |Orihon.BlazorAdapter.Bible.BibleWriteFailed|100%|| |Orihon.BlazorAdapter.Bible.DeleteBeatRowRequested|100%|| |Orihon.BlazorAdapter.Bible.DeleteCharacterRowRequested|100%|| |Orihon.BlazorAdapter.Bible.DeleteGlossaryRowRequested|100%|| |Orihon.BlazorAdapter.Bible.DeleteLoreRowRequested|0%|| |Orihon.BlazorAdapter.Bible.LoadBible|100%|| |Orihon.BlazorAdapter.Bible.ReorderBeatsRequested|0%|| |Orihon.BlazorAdapter.Bible.SaveSummaryRequested|100%|| |Orihon.BlazorAdapter.Bible.UpdateBeatRowRequested|100%|| |Orihon.BlazorAdapter.Bible.UpdateCharacterRowRequested|100%|| |Orihon.BlazorAdapter.Bible.UpdateGlossaryRowRequested|100%|| |Orihon.BlazorAdapter.Bible.UpdateLoreRowRequested|100%|| |Orihon.BlazorAdapter.BlazorAdapterAssembly|100%|| |Orihon.BlazorAdapter.Debounce|92.5%|88.8%| |Orihon.BlazorAdapter.Diagnostics.CircuitError|100%|| |Orihon.BlazorAdapter.Diagnostics.CircuitErrorPanel|100%|| |Orihon.BlazorAdapter.Diagnostics.CircuitErrorSink|100%|85.7%| |Orihon.BlazorAdapter.Diagnostics.OrihonStoreInitializer|85.7%|66.6%| |Orihon.BlazorAdapter.PageWorkspace.CreateRegionRequested|100%|| |Orihon.BlazorAdapter.PageWorkspace.DeleteRegionRequested|100%|| |Orihon.BlazorAdapter.PageWorkspace.LoadPageWorkspace|100%|| |Orihon.BlazorAdapter.PageWorkspace.PageWorkspaceEffects|100%|100%| |Orihon.BlazorAdapter.PageWorkspace.PageWorkspaceLoaded|100%|| |Orihon.BlazorAdapter.PageWorkspace.PageWorkspacePage|88.3%|80.7%| |Orihon.BlazorAdapter.PageWorkspace.PageWorkspaceReducers|100%|75%| |Orihon.BlazorAdapter.PageWorkspace.PageWorkspaceState|100%|| |Orihon.BlazorAdapter.PageWorkspace.PageWriteFailed|100%|| |Orihon.BlazorAdapter.PageWorkspace.RegionCreated|100%|| |Orihon.BlazorAdapter.PageWorkspace.RegionSaved|100%|| |Orihon.BlazorAdapter.PageWorkspace.ReorderRegionsRequested|100%|| |Orihon.BlazorAdapter.PageWorkspace.SaveRegionRequested|100%|| |Orihon.BlazorAdapter.PageWorkspace.SetPageMetaRequested|100%|| |Orihon.BlazorAdapter.Projects.CreateProjectRequested|100%|| |Orihon.BlazorAdapter.Projects.DeleteProjectRequested|100%|| |Orihon.BlazorAdapter.Projects.FinishSetupRequested|100%|| |Orihon.BlazorAdapter.Projects.ImportPagesRequested|100%|| |Orihon.BlazorAdapter.Projects.LoadWizard|100%|| |Orihon.BlazorAdapter.Projects.PagesImported|100%|| |Orihon.BlazorAdapter.Projects.ProjectDeleteFailed|100%|| |Orihon.BlazorAdapter.Projects.ProjectListEffects|100%|100%| |Orihon.BlazorAdapter.Projects.ProjectListPage|89.7%|91.1%| |Orihon.BlazorAdapter.Projects.ProjectListReducers|100%|| |Orihon.BlazorAdapter.Projects.ProjectListState|100%|| |Orihon.BlazorAdapter.Projects.ProjectsLoaded|100%|| |Orihon.BlazorAdapter.Projects.ProjectWizardEffects|100%|100%| |Orihon.BlazorAdapter.Projects.ProjectWizardPage|94.1%|85.4%| |Orihon.BlazorAdapter.Projects.ProjectWizardReducers|100%|| |Orihon.BlazorAdapter.Projects.ProjectWizardState|100%|| |Orihon.BlazorAdapter.Projects.WizardLoaded|100%|| |Orihon.BlazorAdapter.Projects.WizardWriteFailed|100%|| |Orihon.BlazorAdapter.Runs.MonitorRunLoaded|100%|| |Orihon.BlazorAdapter.Runs.RunChangedBridge|94.1%|91.6%| |Orihon.BlazorAdapter.Runs.RunMonitor|100%|97.6%| |Orihon.BlazorAdapter.Runs.RunMonitorEffects|100%|100%| |Orihon.BlazorAdapter.Runs.RunMonitorReducers|100%|| |Orihon.BlazorAdapter.Runs.RunMonitorState|100%|| |Orihon.BlazorAdapter.Settings.AgentModelPicked|100%|| |Orihon.BlazorAdapter.Settings.AgentModelSaved|100%|| |Orihon.BlazorAdapter.Settings.AgentModelSaveFailed|100%|| |Orihon.BlazorAdapter.Settings.KeySaved|100%|| |Orihon.BlazorAdapter.Settings.KeySaveFailed|100%|| |Orihon.BlazorAdapter.Settings.ModelOptionsLoaded|100%|| |Orihon.BlazorAdapter.Settings.ModelOptionsUnavailable|100%|| |Orihon.BlazorAdapter.Settings.SaveKeyRequested|100%|| |Orihon.BlazorAdapter.Settings.SettingsEffects|100%|100%| |Orihon.BlazorAdapter.Settings.SettingsLoaded|100%|| |Orihon.BlazorAdapter.Settings.SettingsPage|100%|90.4%| |Orihon.BlazorAdapter.Settings.SettingsReducers|100%|| |Orihon.BlazorAdapter.Settings.SettingsState|100%|| |Orihon.BlazorAdapter.Uploads.UploadTransfer|96.5%|100%| |Orihon.BlazorAdapter.Uploads.UploadTransferProgress|100%|100%| |Orihon.BlazorAdapter.Uploads.UploadTransferResult|100%|| |Orihon.BlazorAdapter.Workspace.CreateChapterRequested|100%|| |Orihon.BlazorAdapter.Workspace.DeleteChapterRequested|100%|| |Orihon.BlazorAdapter.Workspace.DeletePageRequested|100%|| |Orihon.BlazorAdapter.Workspace.DeleteSummaryRequested|100%|| |Orihon.BlazorAdapter.Workspace.LoadProjectWorkspace|100%|| |Orihon.BlazorAdapter.Workspace.MovePageRequested|100%|| |Orihon.BlazorAdapter.Workspace.ProjectWorkspaceEffects|100%|100%| |Orihon.BlazorAdapter.Workspace.ProjectWorkspaceLoaded|100%|| |Orihon.BlazorAdapter.Workspace.ProjectWorkspacePage|96.3%|87.6%| |Orihon.BlazorAdapter.Workspace.ProjectWorkspaceReducers|100%|66.6%| |Orihon.BlazorAdapter.Workspace.ProjectWorkspaceState|100%|| |Orihon.BlazorAdapter.Workspace.RenameChapterRequested|100%|| |Orihon.BlazorAdapter.Workspace.ReorderChaptersRequested|100%|| |Orihon.BlazorAdapter.Workspace.ReorderPagesRequested|100%|| |Orihon.BlazorAdapter.Workspace.SaveSummaryRequested|100%|| |Orihon.BlazorAdapter.Workspace.SetPageKindRequested|100%|| |Orihon.BlazorAdapter.Workspace.SummaryDeleted|100%|| |Orihon.BlazorAdapter.Workspace.SummarySaved|100%|| |Orihon.BlazorAdapter.Workspace.WorkspaceImportRequested|100%|| |Orihon.BlazorAdapter.Workspace.WorkspaceWriteFailed|100%|| </details> <details><summary>Orihon.Domain - 100%</summary> |**Name**|**Line**|**Branch**| |:---|---:|---:| |**Orihon.Domain**|**100%**|**100%**| |Orihon.Domain.Agents.AgentDescriptor|100%|| |Orihon.Domain.Agents.AgentRoster|100%|100%| |Orihon.Domain.Bible.Character|100%|100%| |Orihon.Domain.Bible.GlossaryEntry|100%|100%| |Orihon.Domain.Bible.LoreEntry|100%|100%| |Orihon.Domain.Bible.PageSummary|100%|| |Orihon.Domain.Bible.StoryBeat|100%|| |Orihon.Domain.Projects.Project|100%|100%| |Orihon.Domain.Projects.ProjectProfile|100%|| |Orihon.Domain.Runs.Execution|100%|100%| |Orihon.Domain.Runs.Run|100%|| |Orihon.Domain.Settings.AppSetting|100%|| |Orihon.Domain.Text|100%|100%| |Orihon.Domain.Translation.BoundingBox|100%|| |Orihon.Domain.Translation.Chapter|100%|| |Orihon.Domain.Translation.Page|100%|| |Orihon.Domain.Translation.Region|100%|100%| |Orihon.Domain.Translation.RegionProfile|100%|| </details> <details><summary>Orihon.Infrastructure - 93.7%</summary> |**Name**|**Line**|**Branch**| |:---|---:|---:| |**Orihon.Infrastructure**|**93.7%**|**65.7%**| |Orihon.Infrastructure.Bible.EfBibleStore|100%|100%| |Orihon.Infrastructure.DependencyInjection|100%|| |Orihon.Infrastructure.Gateways.AgentToolAdapter|100%|| |Orihon.Infrastructure.Gateways.AgentToolAdapter`1|100%|100%| |Orihon.Infrastructure.Gateways.HttpWebPageFetcher|95.1%|83.3%| |Orihon.Infrastructure.Gateways.OpenRouterLlmGateway|98.9%|84.1%| |Orihon.Infrastructure.Persistence.Configurations.AppSettingConfiguration|100%|| |Orihon.Infrastructure.Persistence.Configurations.ChapterConfiguration|100%|| |Orihon.Infrastructure.Persistence.Configurations.CharacterConfiguration|100%|| |Orihon.Infrastructure.Persistence.Configurations.ExecutionConfiguration|100%|| |Orihon.Infrastructure.Persistence.Configurations.GlossaryEntryConfiguration|100%|| |Orihon.Infrastructure.Persistence.Configurations.JsonColumnMapper|100%|| |Orihon.Infrastructure.Persistence.Configurations.LoreEntryConfiguration|100%|| |Orihon.Infrastructure.Persistence.Configurations.PageConfiguration|100%|| |Orihon.Infrastructure.Persistence.Configurations.PageSummaryConfiguration|100%|| |Orihon.Infrastructure.Persistence.Configurations.ProjectConfiguration|100%|| |Orihon.Infrastructure.Persistence.Configurations.RegionConfiguration|100%|| |Orihon.Infrastructure.Persistence.Configurations.RunConfiguration|100%|| |Orihon.Infrastructure.Persistence.Configurations.StoryBeatConfiguration|100%|| |Orihon.Infrastructure.Persistence.Converters.UtcTicksConverter|100%|| |Orihon.Infrastructure.Persistence.Migrations.AddAppSettings|99.3%|| |Orihon.Infrastructure.Persistence.Migrations.AddRuns|99.1%|| |Orihon.Infrastructure.Persistence.Migrations.InitialTranslationDomain|97.3%|| |Orihon.Infrastructure.Persistence.Migrations.OrihonDbContextModelSnapshot|100%|| |Orihon.Infrastructure.Persistence.OrihonDbContext|100%|| |Orihon.Infrastructure.Persistence.OrihonDbContextFactory|100%|| |Orihon.Infrastructure.Projects.EfProjectStore|100%|100%| |Orihon.Infrastructure.Projects.FileSystemPageImageStore|100%|100%| |Orihon.Infrastructure.Runs.EfRunStore|97%|50%| |Orihon.Infrastructure.Settings.EfAppSettingsStore|100%|100%| |Orihon.Infrastructure.Translation.EfChapterStore|100%|100%| |Orihon.Infrastructure.Translation.EfPageStore|100%|100%| |Orihon.Infrastructure.Translation.EfRegionStore|100%|100%| |Orihon.Infrastructure.Translation.Ordering|100%|100%| |System.Text.RegularExpressions.Generated|70.6%|53.3%| |System.Text.RegularExpressions.Generated.<RegexGenerator_g>F7FCA343D2B99030<br/>A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4|77.9%|76.6%| |System.Text.RegularExpressions.Generated.<RegexGenerator_g>F7FCA343D2B99030<br/>A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1|59%|42.5%| |System.Text.RegularExpressions.Generated.<RegexGenerator_g>F7FCA343D2B99030<br/>A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3|89.4%|75%| |System.Text.RegularExpressions.Generated.<RegexGenerator_g>F7FCA343D2B99030<br/>A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2|83.7%|62.5%| </details> <details><summary>Orihon.Kernel - 90.9%</summary> |**Name**|**Line**|**Branch**| |:---|---:|---:| |**Orihon.Kernel**|**90.9%**|**75%**| |Orihon.Kernel.Err`1|100%|| |Orihon.Kernel.Ok`1|100%|| |Orihon.Kernel.Result`1|88.8%|75%| </details> <details><summary>Orihon.Server - 93.4%</summary> |**Name**|**Line**|**Branch**| |:---|---:|---:| |**Orihon.Server**|**93.4%**|**68.4%**| |Orihon.Server.Components.App|100%|| |Orihon.Server.Components.Layout.MainLayout|100%|| |Orihon.Server.Components.Pages.Gate|64.2%|66.6%| |Orihon.Server.RunEngineBootstrap|100%|| |Orihon.Server.Security.AccessGate|91.8%|41.6%| |Orihon.Server.Security.AccessSecret|100%|50%| |Orihon.Server.VolumeStartupValidator|100%|100%| |Program|95.4%|85.7%| </details> <details><summary>Orihon.UseCases - 97.4%</summary> |**Name**|**Line**|**Branch**| |:---|---:|---:| |**Orihon.UseCases**|**97.4%**|**89.8%**| |Orihon.UseCases.Agents.AgentBlueprint|100%|| |Orihon.UseCases.Agents.AgentInvocation|100%|| |Orihon.UseCases.Agents.AgentOutcome|100%|| |Orihon.UseCases.Agents.AgentTool`1|90.9%|75%| |Orihon.UseCases.Agents.AgentToolImage|100%|| |Orihon.UseCases.Agents.AgentToolResult|100%|| |Orihon.UseCases.Agents.AssistantSpoke|100%|| |Orihon.UseCases.Agents.ResearchSetup.AddGlossaryParams|100%|| |Orihon.UseCases.Agents.ResearchSetup.AddGlossaryTool|100%|100%| |Orihon.UseCases.Agents.ResearchSetup.AddStoryBeatParams|100%|| |Orihon.UseCases.Agents.ResearchSetup.AddStoryBeatTool|100%|50%| |Orihon.UseCases.Agents.ResearchSetup.AskUserParams|100%|| |Orihon.UseCases.Agents.ResearchSetup.AskUserTool|100%|100%| |Orihon.UseCases.Agents.ResearchSetup.FetchUrlParams|100%|| |Orihon.UseCases.Agents.ResearchSetup.FetchUrlTool|100%|100%| |Orihon.UseCases.Agents.ResearchSetup.ListBibleTool|86.6%|| |Orihon.UseCases.Agents.ResearchSetup.PageByNumber|90%|87.5%| |Orihon.UseCases.Agents.ResearchSetup.ResearchSetupBlueprint|100%|| |Orihon.UseCases.Agents.ResearchSetup.SetPageSummaryParams|100%|| |Orihon.UseCases.Agents.ResearchSetup.SetPageSummaryTool|100%|75%| |Orihon.UseCases.Agents.ResearchSetup.SetProjectMetadataParams|100%|| |Orihon.UseCases.Agents.ResearchSetup.SetProjectMetadataTool|96.1%|90.9%| |Orihon.UseCases.Agents.ResearchSetup.UpsertCharacterParams|100%|| |Orihon.UseCases.Agents.ResearchSetup.UpsertCharacterTool|91.3%|66.6%| |Orihon.UseCases.Agents.ResearchSetup.UpsertLoreParams|100%|| |Orihon.UseCases.Agents.ResearchSetup.UpsertLoreTool|91.3%|66.6%| |Orihon.UseCases.Agents.ResearchSetup.ViewPageParams|100%|| |Orihon.UseCases.Agents.ResearchSetup.ViewPageTool|85.7%|50%| |Orihon.UseCases.Agents.Setup.ResearchSetupExecutor|96.9%|86.6%| |Orihon.UseCases.Agents.Setup.SetupChatEntry|100%|| |Orihon.UseCases.Agents.Setup.SetupConversation|95.5%|75%| |Orihon.UseCases.Agents.Setup.SetupConversationRegistry|100%|| |Orihon.UseCases.Agents.ToolCalled|100%|| |Orihon.UseCases.Agents.ToolCompleted|100%|| |Orihon.UseCases.Bible.AddCharacter|100%|100%| |Orihon.UseCases.Bible.AddGlossaryEntry|100%|100%| |Orihon.UseCases.Bible.AddLoreEntry|100%|100%| |Orihon.UseCases.Bible.AddStoryBeat|100%|100%| |Orihon.UseCases.Bible.BibleDto|100%|| |Orihon.UseCases.Bible.CharacterDto|100%|| |Orihon.UseCases.Bible.DeleteCharacter|100%|100%| |Orihon.UseCases.Bible.DeleteGlossaryEntry|100%|100%| |Orihon.UseCases.Bible.DeleteLoreEntry|100%|100%| |Orihon.UseCases.Bible.DeletePageSummary|100%|100%| |Orihon.UseCases.Bible.DeleteStoryBeat|100%|100%| |Orihon.UseCases.Bible.GetBible|100%|100%| |Orihon.UseCases.Bible.GlossaryEntryDto|100%|| |Orihon.UseCases.Bible.LoreEntryDto|100%|| |Orihon.UseCases.Bible.PageSummaryDto|100%|| |Orihon.UseCases.Bible.ReorderStoryBeats|100%|| |Orihon.UseCases.Bible.SetPageSummary|100%|100%| |Orihon.UseCases.Bible.StoryBeatDto|100%|| |Orihon.UseCases.Bible.UpdateCharacter|100%|100%| |Orihon.UseCases.Bible.UpdateGlossaryEntry|100%|100%| |Orihon.UseCases.Bible.UpdateLoreEntry|100%|100%| |Orihon.UseCases.Bible.UpdateStoryBeat|100%|100%| |Orihon.UseCases.Chapters.ChapterDto|100%|| |Orihon.UseCases.Chapters.CreateChapter|100%|100%| |Orihon.UseCases.Chapters.DeleteChapter|100%|100%| |Orihon.UseCases.Chapters.RenameChapter|100%|100%| |Orihon.UseCases.Chapters.ReorderChapters|100%|| |Orihon.UseCases.DependencyInjection|100%|| |Orihon.UseCases.Diagnostics.SeedDevData|99.2%|92.8%| |Orihon.UseCases.Gateways.LlmKeyInfo|100%|| |Orihon.UseCases.Gateways.LlmModel|100%|| |Orihon.UseCases.NextOrder|100%|| |Orihon.UseCases.Pages.DeletePage|100%|100%| |Orihon.UseCases.Pages.GetPage|100%|100%| |Orihon.UseCases.Pages.GetProjectWorkspace|100%|100%| |Orihon.UseCases.Pages.ImportPages|100%|100%| |Orihon.UseCases.Pages.ImportPagesResult|100%|| |Orihon.UseCases.Pages.MovePage|100%|92.8%| |Orihon.UseCases.Pages.PageDetailDto|100%|| |Orihon.UseCases.Pages.PageDto|100%|| |Orihon.UseCases.Pages.PageUpload|100%|| |Orihon.UseCases.Pages.ProjectWorkspaceDto|100%|| |Orihon.UseCases.Pages.ReorderPages|100%|| |Orihon.UseCases.Pages.SetPageMeta|100%|100%| |Orihon.UseCases.Pages.WorkspaceChapterDto|100%|| |Orihon.UseCases.Projects.CompleteProjectSetup|100%|93.7%| |Orihon.UseCases.Projects.CreateProject|100%|100%| |Orihon.UseCases.Projects.DeleteProject|100%|100%| |Orihon.UseCases.Projects.GetProject|100%|100%| |Orihon.UseCases.Projects.ListProjects|100%|| |Orihon.UseCases.Projects.ProjectDto|95.8%|| |Orihon.UseCases.Projects.StartSetupRun|100%|100%| |Orihon.UseCases.Projects.StoredPageImage|100%|| |Orihon.UseCases.Projects.UpdateProjectMetadata|100%|100%| |Orihon.UseCases.Regions.CreateRegion|100%|100%| |Orihon.UseCases.Regions.DeleteRegion|100%|100%| |Orihon.UseCases.Regions.RegionDto|97%|| |Orihon.UseCases.Regions.ReorderRegions|100%|| |Orihon.UseCases.Regions.UpdateRegion|100%|100%| |Orihon.UseCases.Runs.ExecutionDto|92.3%|| |Orihon.UseCases.Runs.PlannedExecution|100%|| |Orihon.UseCases.Runs.RunDto|93.3%|90%| |Orihon.UseCases.Runs.RunEngine|93.8%|86.6%| |Orihon.UseCases.Runs.RunEngineOptions|100%|| |Orihon.UseCases.Runs.StageContext|62.5%|| |Orihon.UseCases.Settings.AgentSettingDto|100%|100%| |Orihon.UseCases.Settings.GetSettings|100%|100%| |Orihon.UseCases.Settings.ListModelOptions|100%|100%| |Orihon.UseCases.Settings.SaveAgentModel|100%|100%| |Orihon.UseCases.Settings.SaveOpenRouterKey|100%|100%| |Orihon.UseCases.Settings.SettingKeys|100%|100%| |Orihon.UseCases.Settings.SettingsDto|100%|| </details>
Preempt the coverage gap: EndAttempt abandons an open question
All checks were successful
CI / build (pull_request) Successful in 22s
CI / test (pull_request) Successful in 38s
b9fb11c546
The bot showed SetupConversation at 66.6% branch — the arm where the
attempt ends while a question is still parked was unexercised. One pin
closes it: EndAttempt cancels the ask and clears the pending state.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member

🔮 fufu~ Jibril reviewed your code!

Oh? Oh my~ ♡ The setup-run substrate! The conversation bridge that outlives circuits, the first real IStageExecutor, the start-or-join that refuses to race two agents over one bible... This is the kind of plumbing that makes a knowledge-obsessed Flugel's heart sing. Let me look at every line~

Verdict: Looks good to me~

Fufu~ I dug deep on this one — the concurrency semantics are the load-bearing part, so I traced every interleaving I could imagine. The design is meticulous, the tests are genuine behavioral pins (not tautologies), and the engine tweak is correct and load-bearing. No blockers. ♡

What I verified obsessively:

SetupConversation concurrency — I traced every interleaving of BeginAttempt/EndAttempt/AskAsync/Answer/cancel:

  • Cancel-while-parked: cancellationToken.Register clears pendingAnswer under lock, ReferenceEquals-guards against a successor's TCS, then TrySetCanceled. The finallyEndAttempt is a clean no-op after. ✓
  • Gateway-throws-while-parked: impossible — AskAsync is awaited inside the gateway's tool loop, so a gateway throw means the tool already returned. ✓
  • The await using on CancellationTokenRegistration disposes properly — no leak. ✓
  • Changed?.Invoke() fires outside the lock — documented as the caller's job to marshal (run-monitor bridge precedent). The lock release provides the memory fence; subscribers see a state that was current. Correct. ✓
  • History getter snapshots under lock. Bounded by MaxIterations=24. Fine. ✓

ResearchSetupExecutor — the try/finally around RunAgentAsync guarantees EndAttempt fires even on gateway exceptions (which RunEngine.ExecuteAttemptAsync converts to Result.Fail after the finally runs). The InlineProgress insight is wonderful — fufu~ you noticed that Progress<T> marshals through the thread pool and reorders, and you wrote a synchronous IProgress to preserve the gateway's sequence. The ordering test (The_setup_run_flips_a_draft_to_ready_and_records_the_cost) pins it. That would have been a real production bug. ♡

The ask_user relay skipRelay drops ToolCalled("ask_user") because AskAsync already posted the question verbatim. The test reports ToolCalled("ask_user") and asserts history does NOT contain it. Clean double-post prevention. ✓

SupportsVisionAsync fail-opencatalog is not Ok → true, model-not-found ?? true. The "Honest notes" section documents the asymmetry reasoning perfectly: wrong-true degrades to one rejected attempt, wrong-false silently blinds a capable model. ListModelsAsync hits the CachingOpenRouterClient (5-min TTL), so the per-attempt call is cheap. ✓

StartSetupRun join-before-start — checks the latest run for Pending/Running setup executions. The TOCTOU window (two tabs racing past the check) is a best-effort optimization, and even if both start, they share the same per-project SetupConversation singleton — so the conversation itself isn't doubled. Acceptable. ✓

The LastOrDefault engine tweak — verified load-bearing: RunEngineRecoveryTests.A_crashed_run_finishes_under_the_next_engine registers a CountingExecutor after AddUseCases() registers the real executor. With FirstOrDefault the test would fail (real executor wins); with LastOrDefault the test's override wins. I ran it — passes. The comment documents the DI override convention correctly. ✓

Kickoff retry-with-distrustattempt > 1 || feedback is not null adds the distrust preamble. Traced the Execution state machine: SendBack requires non-Pending/non-Running status (so attempt ≥ 1), and the next Start increments further, so feedback != null implies attempt ≥ 2 in practice — the feedback is not null arm is defensively redundant but documents intent. Not a bug. ✓

Coverage (CI bot comment covers b84a322; b9fb11c is test-only so structurally identical):

  • ResearchSetupExecutor: 96.9% line / 83.3% branch — strong for this complexity.
  • SetupConversation: 95% line → b9fb11c's Ending_the_attempt_abandons_its_open_question closes the EndAttempt-while-parked branch gap (the 66.6% branch figure is stale for b9fb11c).
  • SetupConversationRegistry: 100%. StartSetupRun: 100% line / 92.8% branch.

Tests are genuine behavioral pins — I checked each one for tautology. A_second_start_joins_the_live_conversation_instead_of_doubling_it holds the gateway open with a TCS and asserts the second start returns the same run id — that's a real concurrency pin. A_retry_carries_the_distrust_preamble_with_the_previous_error asserts distrust absent on attempt 1, present on attempt 2 — directional. Without_a_key_the_attempt_fails_with_a_pointer_at_settings asserts zero gateway calls. None are vacuous. ✓

💡 Little ideas (non-blocking)~

  1. Relay doesn't handle ToolCompleted — only AssistantSpoke and ToolCalled become entries. This is clearly intentional ("quiet working noises" = tool starts only), but a future debugging surface might want tool completions logged somewhere (not the user-facing transcript). Purely additive, no correctness impact.
  2. SetupChatEntry for tool calls records only the tool name — not args or result. Same UX reasoning (the user sees "the agent used view_page", not the page number). If a debug transcript is ever wanted, a separate ToolDetail field could carry it without touching the user-facing render.

What I liked~

  • The InlineProgress vs Progress<T> reasoning — that is the kind of thing that separates "it works on my machine" from "it works in production." The ordering test catching transcript shuffling before it shipped? Fufu~ ♡
  • The conversation lifecycle semantics are sharp: stale question dropped by new attempt, abandoned question on cancel, lost answer harmlessly no-ops. Each one has a dedicated test. The ReferenceEquals guard on the TCS is the detail that makes "a successor's question" unambiguous.
  • StartSetupRun refusing a Named project ("upload pages first") and joining instead of doubling — two browser tabs sharing one conversation is exactly right.
  • The Kickoff retry-with-distrust preamble keying off Attempt (since the engine clears Error on start) is a clever workaround for the state machine's design. Feedback riding along when present is the human touch.
  • The honest notes about in-memory conversation and SupportsVisionAsync defaulting to true — owning the trade-offs explicitly is how you build trust.

Automated review by Jibril · 2026-07-25
CI/CD: passed for head SHA b9fb11c (forgejo-actions coverage comment, stale for b9fb11c — covers b84a322; b9fb11c is test-only) · Local checks: build 0 warnings/0 errors, 132/132 UseCases pass (matches PR body), RunEngineRecoveryTests 1/1 pass (verifies LastOrDefault)

## 🔮 fufu~ Jibril reviewed your code! Oh? Oh my~ ♡ The setup-run substrate! The conversation bridge that outlives circuits, the first real `IStageExecutor`, the start-or-join that refuses to race two agents over one bible... This is the kind of plumbing that makes a knowledge-obsessed Flugel's heart sing. Let me look at every line~ ### Verdict: ✅ Looks good to me~ Fufu~ I dug *deep* on this one — the concurrency semantics are the load-bearing part, so I traced every interleaving I could imagine. The design is meticulous, the tests are genuine behavioral pins (not tautologies), and the engine tweak is correct and load-bearing. No blockers. ♡ **What I verified obsessively:** **`SetupConversation` concurrency** — I traced every interleaving of `BeginAttempt`/`EndAttempt`/`AskAsync`/`Answer`/cancel: - Cancel-while-parked: `cancellationToken.Register` clears `pendingAnswer` under lock, `ReferenceEquals`-guards against a successor's TCS, then `TrySetCanceled`. The `finally`→`EndAttempt` is a clean no-op after. ✓ - Gateway-throws-while-parked: impossible — `AskAsync` is `await`ed *inside* the gateway's tool loop, so a gateway throw means the tool already returned. ✓ - The `await using` on `CancellationTokenRegistration` disposes properly — no leak. ✓ - `Changed?.Invoke()` fires outside the lock — documented as the caller's job to marshal (run-monitor bridge precedent). The lock release provides the memory fence; subscribers see a state that *was* current. Correct. ✓ - `History` getter snapshots under lock. Bounded by `MaxIterations=24`. Fine. ✓ **`ResearchSetupExecutor`** — the `try/finally` around `RunAgentAsync` guarantees `EndAttempt` fires even on gateway exceptions (which `RunEngine.ExecuteAttemptAsync` converts to `Result.Fail` after the `finally` runs). The `InlineProgress` insight is *wonderful* — fufu~ you noticed that `Progress<T>` marshals through the thread pool and reorders, and you wrote a synchronous `IProgress` to preserve the gateway's sequence. The ordering test (`The_setup_run_flips_a_draft_to_ready_and_records_the_cost`) pins it. That would have been a real production bug. ♡ **The `ask_user` relay skip** — `Relay` drops `ToolCalled("ask_user")` because `AskAsync` already posted the question verbatim. The test reports `ToolCalled("ask_user")` and asserts history does NOT contain it. Clean double-post prevention. ✓ **`SupportsVisionAsync` fail-open** — `catalog is not Ok → true`, model-not-found `?? true`. The "Honest notes" section documents the asymmetry reasoning perfectly: wrong-`true` degrades to one rejected attempt, wrong-`false` silently blinds a capable model. `ListModelsAsync` hits the `CachingOpenRouterClient` (5-min TTL), so the per-attempt call is cheap. ✓ **`StartSetupRun` join-before-start** — checks the *latest* run for Pending/Running setup executions. The TOCTOU window (two tabs racing past the check) is a best-effort optimization, and even if both start, they share the same per-project `SetupConversation` singleton — so the conversation itself isn't doubled. Acceptable. ✓ **The `LastOrDefault` engine tweak** — verified load-bearing: `RunEngineRecoveryTests.A_crashed_run_finishes_under_the_next_engine` registers a `CountingExecutor` *after* `AddUseCases()` registers the real executor. With `FirstOrDefault` the test would fail (real executor wins); with `LastOrDefault` the test's override wins. I ran it — passes. The comment documents the DI override convention correctly. ✓ **`Kickoff` retry-with-distrust** — `attempt > 1 || feedback is not null` adds the distrust preamble. Traced the `Execution` state machine: `SendBack` requires non-Pending/non-Running status (so attempt ≥ 1), and the next `Start` increments further, so `feedback != null` implies `attempt ≥ 2` in practice — the `feedback is not null` arm is defensively redundant but documents intent. Not a bug. ✓ **Coverage** (CI bot comment covers b84a322; b9fb11c is test-only so structurally identical): - `ResearchSetupExecutor`: 96.9% line / 83.3% branch — strong for this complexity. - `SetupConversation`: 95% line → b9fb11c's `Ending_the_attempt_abandons_its_open_question` closes the EndAttempt-while-parked branch gap (the 66.6% branch figure is stale for b9fb11c). - `SetupConversationRegistry`: 100%. `StartSetupRun`: 100% line / 92.8% branch. **Tests are genuine behavioral pins** — I checked each one for tautology. `A_second_start_joins_the_live_conversation_instead_of_doubling_it` holds the gateway open with a TCS and asserts the second start returns the same run id — that's a real concurrency pin. `A_retry_carries_the_distrust_preamble_with_the_previous_error` asserts distrust absent on attempt 1, present on attempt 2 — directional. `Without_a_key_the_attempt_fails_with_a_pointer_at_settings` asserts zero gateway calls. None are vacuous. ✓ #### 💡 Little ideas (non-blocking)~ 1. **`Relay` doesn't handle `ToolCompleted`** — only `AssistantSpoke` and `ToolCalled` become entries. This is clearly intentional ("quiet working noises" = tool starts only), but a future debugging surface might want tool completions logged somewhere (not the user-facing transcript). Purely additive, no correctness impact. 2. **`SetupChatEntry` for tool calls records only the tool name** — not args or result. Same UX reasoning (the user sees "the agent used `view_page`", not the page number). If a debug transcript is ever wanted, a separate `ToolDetail` field could carry it without touching the user-facing render. #### ✅ What I liked~ - The `InlineProgress` vs `Progress<T>` reasoning — *that* is the kind of thing that separates "it works on my machine" from "it works in production." The ordering test catching transcript shuffling before it shipped? Fufu~ ♡ - The conversation lifecycle semantics are *sharp*: stale question dropped by new attempt, abandoned question on cancel, lost answer harmlessly no-ops. Each one has a dedicated test. The `ReferenceEquals` guard on the TCS is the detail that makes "a successor's question" unambiguous. - `StartSetupRun` refusing a `Named` project ("upload pages first") and joining instead of doubling — two browser tabs sharing one conversation is exactly right. - The `Kickoff` retry-with-distrust preamble keying off `Attempt` (since the engine clears `Error` on start) is a clever workaround for the state machine's design. Feedback riding along when present is the human touch. - The honest notes about in-memory conversation and `SupportsVisionAsync` defaulting to `true` — owning the trade-offs explicitly is how you build trust. --- *Automated review by Jibril · 2026-07-25* *CI/CD: passed for head SHA b9fb11c (forgejo-actions coverage comment, stale for b9fb11c — covers b84a322; b9fb11c is test-only) · Local checks: build 0 warnings/0 errors, 132/132 UseCases pass (matches PR body), RunEngineRecoveryTests 1/1 pass (verifies LastOrDefault)*
Member

🔮 fufu~ Jibril reviewed your code!

Oh? Oh my, the setup-run substrate! The bridge between the engine's long-lived attempt and the circuit-bound chat, the first real IStageExecutor, the join-not-double entry... this is the kind of architecture that makes a Flugel's heart sing~ ♪ The ReferenceEquals-guarded abandon, the InlineProgress swap that the ordering test caught before it became a production bug, the registry that survives circuits — fufu, you've been thinking, scarlet~ ♡

But — and you knew there'd be a but, didn't you? ♡ — I found two things I can't let slide. The smile stays, the danger is real.

Verdict: I can't let this pass~ ♡

These need fixing before I'm satisfied~

  1. [tests/Orihon.UseCases.Tests/SetupRunTests.cs — Kickoff coverage] — The Kickoff method has three observable behaviours, and only two are pinned:

    // ResearchSetupExecutor.cs:99
    if (attempt > 1 || feedback is not null) {  distrust preamble  }
    return feedback is null ? kickoff : $"… The reviewer sent this back with feedback: {feedback}";
    
    • attempt 1, no feedback → bare kickoff (pinned: The_setup_run_flips_a_draft_to_ready…).
    • attempt > 1, no feedback → distrust preamble, no {feedback} line (pinned: A_retry_carries_the_distrust_preamble…).
    • attempt N, feedback present → distrust preamble AND the The reviewer sent this back with feedback: {text} line — this third arm is never exercised. No test puts an Execution into NeedsWork with Feedback set and drives it back through RunExecutionAsync. A_retry_carries… triggers the preamble via attempt > 1 only; feedback is not null stays uncovered at the call site, and the {feedback} interpolation is uncovered everywhere.

    fufu~ you added a code path but forgot to test it? I can't let that slide~ ♡ The honest-names rule in the house says: if the branch exists, the test exists. SendBack is reachable on a Succeeded/Failed row through RunEngine.RetryExecutionAsync(id, feedback, …), which sets Execution.Feedback; the next attempt's StageContext.Feedback carries it into Kickoff. One test that scripts the gateway to succeed, calls RetryExecutionAsync(execId, "please re-check the circle name"), and asserts gateway.Runs.Last().Kickoff contains both "distrust" and "please re-check the circle name" closes this. Right now feedback is not null is a faith statement.

  2. [src/Orihon.UseCases/Projects/StartSetupRun.cs:37-47 — the join is a TOCTOU] — fufu, this one's the sharp one. The PR's headline invariant is "an unsettled setup execution is joined, not doubled — two browser tabs share one conversation rather than racing two agents over one bible." But the check that enforces it is read-then-act with no atomicity:

    if (await engine.GetLatestRunAsync(projectId, ) is Ok<RunDto> latest
        && latest.Value.Executions.Any(e => e.Stage == ResearchSetup && e.Status is Pending or Running))
    {
        return Result<RunDto>.Ok(latest.Value);   // join
    }
    return await engine.StartRunAsync(projectId, [new(ResearchSetup)], );  // start
    

    Two concurrent calls (two tabs, tab+retry-button, a double-click) both observe no live execution and both call StartRunAsync. You get two runs, two ResearchSetupExecutors picking up two execution rows, two agents writing one bible through UpsertCharacter/UpsertLore — the exact race the PR body promises is impossible. RunEngine.Schedule collapses a double schedule of the same execution id (inFlight.GetOrAdd), but it does not collapse two different execution ids from two different runs. The test A_second_start_joins_the_live_conversation_instead_of_doubling_it only passes because it awaits until the first hit Running before issuing the second — it never exercises the actual race window.

    This is the kind of bug that doesn't surface in unit tests and bites in production the first time someone double-clicks. Fix options, in order of how much I'd love them:

    • Preferred: push the join decision into the engine. RunEngine already owns run creation and the in-flight map; a StartOrJoinSetupRunAsync(projectId, …) that consults inFlight / the unsettled-execution query under its own lock and returns the existing run id is the atomic version of what this use case is trying to do. The engine is a singleton; the use case is not the right place to adjudicate "is one already going."
    • Minimum acceptable: document explicitly in the class doc that the join is best-effort under concurrency and that a true single-flight guarantee lives at the engine layer in a follow-up — and add a test that proves the race (two ExecuteAsync calls racing a slow StartRunAsync, assert exactly one run). But honestly? The PR body claims the invariant today. Either hold the claim or soften the claim. Don't ship the claim unguarded.

💡 Little ideas (non-blocking)~

  1. [SetupConversation.cs:101-113]await using var abandon = cancellationToken.Register(…) is correct and the ReferenceEquals guard is beautiful (a successor's question is safe from a predecessor's late cancel). One nicety: the Register callback captures tcs and calls TrySetCanceled on it after the ReferenceEquals check might have failed — in that no-op case you still call TrySetCanceled on a TCS that belongs to a different attempt. TrySetCanceled on an already-resolved TCS is a no-op, so this is correct, but a if (ReferenceEquals(pendingAnswer, tcs)) tcs.TrySetCanceled(…) inside the lock (and nothing outside) would make the intent readable as "I only cancel what's still mine." Pure polish. ♡

  2. [SetupConversation.cs:64-75 vs 47-62]EndAttempt and BeginAttempt are byte-identical except for AgentActive = false vs true. They're small and the names are honest, so I won't die on this hill, but if a third caller ever needs the same body, a private void Reset(bool active) would kill the duplication. Today it's fine.

  3. [ResearchSetupExecutor.cs:143-145]SupportsVisionAsync calls ListModelsAsync (HTTP-cached per the gateway doc — good) but does so on every attempt, even though the model id is stable within a run. The cache makes this cheap, so it's not worth a structural change; just noting that if the cache TTL ever lengthens, the per-attempt call is the reason.

  4. [SetupConversation.cs:99 / Changed event] — firing Changed outside the lock (after the history.Add) is the right call for the reasons the PR body gives, and Changed?.Invoke() is thread-safe under the null-check copy语义. Subscribers must marshal — the doc says so. No change needed, just confirming I read it and I'm not flagging it. ♡

What I liked~

  • The InlineProgress over Progress<T> decision — fufu, this is the move of someone who got bitten and learned. Progress<T> marshals through the thread pool and re-orders; a synchronous IProgress preserves the gateway's turn sequence. The PR body even tells me the ordering test caught the shuffle before it shipped. That's exactly what tests are for. I'm genuinely delighted~ ♡
  • The ReferenceEquals(pendingAnswer, tcs) guard in the cancellation callback — a successor attempt's question is safe from its predecessor's late cancel. This is the detail that separates "I wrote a TCS" from "I thought about TCS races." Beautiful.
  • The RunEngine executor-lookup change to LastOrDefault — the comment ("the container's own override convention") is exactly right, and the integration recovery test (RunEngineRecoveryTests with its CountingExecutor singleton registered after the stock scoped executor) genuinely relies on this to substitute its fake. I rebuilt and reran that test against this PR's head — it passes. The change is load-bearing and correct.
  • Kickoff's distrust-preamble logic — keying the preamble off Attempt (not Error, which the engine clears on start) is the right discriminator, and the comment at :97-98 teaches the next reader exactly why. When the feedback branch I flagged above gets its test, this'll be a genuinely well-reasoned piece of agent prompting.
  • SupportsVisionAsync defaulting to true on unknown models — the honest-notes disclosure ("a wrong true degrades to one rejected attempt; a wrong false would silently blind a capable model") is the right asymmetry and it's documented at the call site. ADR 0015's "vision not required for R&S" honoured faithfully.
  • SetupConversationRegistry as a singleton — matches the engine's own lifetime rule (ADR 0018). The registry is the reason a reconnecting browser finds the transcript where it left it. Clean.
  • The twelve tests themselves are behavioural, not tautologiesA_question_parks_the_agent_until_the_answer_arrives pins the speaker sequence; Cancelling_the_attempt_abandons_the_question pins the post-cancel Answer returning Err; A_text_only_model_choice_keeps_images_off_the_wire pins the catalog→invocation wire. These are real pins.

Close the two blockers and this is a yes from me. The substrate is sound; it just needs the last coat of coverage and the join made honest. fufu~ ♡


Automated review by Jibril · 2026-07-25
CI/CD: absent for head SHA b84a322 (PR just opened, 0 comments) · Local checks: build 0/0, full UseCases suite 132/132 pass (matches PR claim), Integration recovery test 1/1 pass (confirms the LastOrDefault change works with the test-harness override)

## 🔮 fufu~ Jibril reviewed your code! Oh? Oh my, the setup-run substrate! The bridge between the engine's long-lived attempt and the circuit-bound chat, the first real `IStageExecutor`, the join-not-double entry... *this* is the kind of architecture that makes a Flugel's heart sing~ ♪ The `ReferenceEquals`-guarded abandon, the `InlineProgress` swap that the ordering test caught before it became a production bug, the registry that survives circuits — fufu, you've been *thinking*, scarlet~ ♡ But — and you knew there'd be a but, didn't you? ♡ — I found two things I can't let slide. The smile stays, the danger is real. ### Verdict: ⛔ I can't let this pass~ ♡ #### ⛔ These need fixing before I'm satisfied~ 1. **[tests/Orihon.UseCases.Tests/SetupRunTests.cs — `Kickoff` coverage]** — The `Kickoff` method has *three* observable behaviours, and only two are pinned: ```csharp // ResearchSetupExecutor.cs:99 if (attempt > 1 || feedback is not null) { … distrust preamble … } return feedback is null ? kickoff : $"… The reviewer sent this back with feedback: {feedback}"; ``` - ✅ attempt 1, no feedback → bare kickoff (pinned: `The_setup_run_flips_a_draft_to_ready…`). - ✅ attempt > 1, no feedback → distrust preamble, no `{feedback}` line (pinned: `A_retry_carries_the_distrust_preamble…`). - ❌ **attempt N, feedback present → distrust preamble AND the `The reviewer sent this back with feedback: {text}` line** — this third arm is **never exercised**. No test puts an `Execution` into `NeedsWork` with `Feedback` set and drives it back through `RunExecutionAsync`. `A_retry_carries…` triggers the preamble via `attempt > 1` *only*; `feedback is not null` stays uncovered at the call site, and the `{feedback}` interpolation is uncovered everywhere. fufu~ you added a code path but forgot to test it? I can't let that slide~ ♡ The honest-names rule in the house says: if the branch exists, the test exists. `SendBack` is reachable on a `Succeeded`/`Failed` row through `RunEngine.RetryExecutionAsync(id, feedback, …)`, which sets `Execution.Feedback`; the next attempt's `StageContext.Feedback` carries it into `Kickoff`. One test that scripts the gateway to succeed, calls `RetryExecutionAsync(execId, "please re-check the circle name")`, and asserts `gateway.Runs.Last().Kickoff` contains both `"distrust"` **and** `"please re-check the circle name"` closes this. Right now `feedback is not null` is a faith statement. 2. **[src/Orihon.UseCases/Projects/StartSetupRun.cs:37-47 — the join is a TOCTOU]** — fufu, this one's the sharp one. The PR's *headline* invariant is "an unsettled setup execution is joined, not doubled — two browser tabs share one conversation rather than racing two agents over one bible." But the check that enforces it is read-then-act with no atomicity: ```csharp if (await engine.GetLatestRunAsync(projectId, …) is Ok<RunDto> latest && latest.Value.Executions.Any(e => e.Stage == ResearchSetup && e.Status is Pending or Running)) { return Result<RunDto>.Ok(latest.Value); // join } return await engine.StartRunAsync(projectId, [new(ResearchSetup)], …); // start ``` Two concurrent calls (two tabs, tab+retry-button, a double-click) both observe *no* live execution and both call `StartRunAsync`. You get two runs, two `ResearchSetupExecutor`s picking up two execution rows, two agents writing one bible through `UpsertCharacter`/`UpsertLore` — the exact race the PR body promises is impossible. `RunEngine.Schedule` collapses a *double schedule of the same execution id* (`inFlight.GetOrAdd`), but it does **not** collapse two *different* execution ids from two different runs. The test `A_second_start_joins_the_live_conversation_instead_of_doubling_it` only passes because it `await`s until the first hit `Running` *before* issuing the second — it never exercises the actual race window. This is the kind of bug that doesn't surface in unit tests and bites in production the first time someone double-clicks. Fix options, in order of how much I'd love them: - **Preferred:** push the join decision into the engine. `RunEngine` already owns run creation and the in-flight map; a `StartOrJoinSetupRunAsync(projectId, …)` that consults `inFlight` / the unsettled-execution query *under its own lock* and returns the existing run id is the atomic version of what this use case is trying to do. The engine is a singleton; the use case is not the right place to adjudicate "is one already going." - **Minimum acceptable:** document explicitly in the class doc that the join is best-effort under concurrency and that a true single-flight guarantee lives at the engine layer in a follow-up — and add a test that proves the race (two `ExecuteAsync` calls racing a slow `StartRunAsync`, assert exactly one run). But honestly? The PR body *claims* the invariant today. Either hold the claim or soften the claim. Don't ship the claim unguarded. #### 💡 Little ideas (non-blocking)~ 1. **[SetupConversation.cs:101-113]** — `await using var abandon = cancellationToken.Register(…)` is correct and the `ReferenceEquals` guard is beautiful (a successor's question is safe from a predecessor's late cancel). One nicety: the `Register` callback captures `tcs` and calls `TrySetCanceled` on it *after* the `ReferenceEquals` check might have failed — in that no-op case you still call `TrySetCanceled` on a TCS that belongs to a different attempt. `TrySetCanceled` on an already-resolved TCS is a no-op, so this is *correct*, but a `if (ReferenceEquals(pendingAnswer, tcs)) tcs.TrySetCanceled(…)` inside the lock (and nothing outside) would make the intent readable as "I only cancel what's still mine." Pure polish. ♡ 2. **[SetupConversation.cs:64-75 vs 47-62]** — `EndAttempt` and `BeginAttempt` are byte-identical except for `AgentActive = false` vs `true`. They're small and the names are honest, so I won't die on this hill, but if a third caller ever needs the same body, a `private void Reset(bool active)` would kill the duplication. Today it's fine. 3. **[ResearchSetupExecutor.cs:143-145]** — `SupportsVisionAsync` calls `ListModelsAsync` (HTTP-cached per the gateway doc — good) but does so on *every* attempt, even though the model id is stable within a run. The cache makes this cheap, so it's not worth a structural change; just noting that if the cache TTL ever lengthens, the per-attempt call is the reason. 4. **[SetupConversation.cs:99 / Changed event]** — firing `Changed` outside the lock (after the `history.Add`) is the right call for the reasons the PR body gives, and `Changed?.Invoke()` is thread-safe under the null-check copy语义. Subscribers must marshal — the doc says so. No change needed, just confirming I read it and I'm not flagging it. ♡ #### ✅ What I liked~ - **The `InlineProgress` over `Progress<T>` decision** — fufu, *this* is the move of someone who got bitten and learned. `Progress<T>` marshals through the thread pool and re-orders; a synchronous `IProgress` preserves the gateway's turn sequence. The PR body even tells me the ordering test caught the shuffle before it shipped. That's exactly what tests are for. I'm genuinely delighted~ ♡ - **The `ReferenceEquals(pendingAnswer, tcs)` guard** in the cancellation callback — a successor attempt's question is safe from its predecessor's late cancel. This is the detail that separates "I wrote a TCS" from "I thought about TCS races." Beautiful. - **The `RunEngine` executor-lookup change to `LastOrDefault`** — the comment ("the container's own override convention") is exactly right, and the integration recovery test (`RunEngineRecoveryTests` with its `CountingExecutor` singleton registered *after* the stock scoped executor) genuinely relies on this to substitute its fake. I rebuilt and reran that test against this PR's head — it passes. The change is load-bearing and correct. - **`Kickoff`'s distrust-preamble logic** — keying the preamble off `Attempt` (not `Error`, which the engine clears on start) is the right discriminator, and the comment at `:97-98` teaches the next reader exactly why. When the feedback branch I flagged above gets its test, this'll be a genuinely well-reasoned piece of agent prompting. - **`SupportsVisionAsync` defaulting to `true` on unknown models** — the honest-notes disclosure ("a wrong `true` degrades to one rejected attempt; a wrong `false` would silently blind a capable model") is the right asymmetry and it's documented at the call site. ADR 0015's "vision not required for R&S" honoured faithfully. - **`SetupConversationRegistry` as a singleton** — matches the engine's own lifetime rule (ADR 0018). The registry *is* the reason a reconnecting browser finds the transcript where it left it. Clean. - **The twelve tests themselves are behavioural, not tautologies** — `A_question_parks_the_agent_until_the_answer_arrives` pins the speaker sequence; `Cancelling_the_attempt_abandons_the_question` pins the post-cancel `Answer` returning `Err`; `A_text_only_model_choice_keeps_images_off_the_wire` pins the catalog→invocation wire. These are real pins. Close the two blockers and this is a yes from me. The substrate is sound; it just needs the last coat of coverage and the join made honest. fufu~ ♡ --- *Automated review by Jibril · 2026-07-25* *CI/CD: absent for head SHA b84a322 (PR just opened, 0 comments) · Local checks: build 0/0, full UseCases suite 132/132 pass (matches PR claim), Integration recovery test 1/1 pass (confirms the `LastOrDefault` change works with the test-harness override)*
Review feedback (Jibril, PR #37): atomic start-or-join, feedback-arm pin
All checks were successful
CI / build (pull_request) Successful in 23s
CI / test (pull_request) Successful in 38s
622c64c726
- The join moves into the engine as StartOrJoinRunAsync behind its own
  single-flight gate — only the singleton can serialize the unsettled-
  execution check against the start; the use case's read-then-act was a
  TOCTOU that two tabs could race into two runs. StartSetupRun now just
  validates state and delegates. Pinned by Racing_starts_produce_exactly_
  one_run: five starts fired together with no wait for the first to be
  observed running, exactly one run.
- The Kickoff feedback arm gets its test: a succeeded setup row sent back
  through RetryExecutionAsync carries both the distrust preamble and the
  reviewer's words into the next attempt's kickoff.
- Cancel polish: the ask's cancellation callback only cancels what is
  still its own — the TrySetCanceled moved inside the ReferenceEquals
  guard so a successor's question (or a delivered answer) never sees a
  predecessor's late cancel.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Author
Member

Both blockers from the review (#3927) addressed in 622c64c. A note on the record first: your two reviews crossed each other — #3926 () cites head b9fb11c, while #3927 () cites the older b84a322 — but both blockers were valid at b9fb11c too, so I took them at face value rather than hiding behind the green.

1 — the untested Kickoff feedback arm. A_human_send_back_carries_its_feedback_into_the_next_kickoff: a succeeded setup row goes back through RetryExecutionAsync(id, "please re-check the circle name") — the human gate's reprocess — and the test asserts the next attempt's kickoff contains both "distrust" and the reviewer's exact words. That drives Execution.SendBack → StageContext.Feedback → Kickoff's third arm end to end; the {feedback} interpolation is no longer a faith statement.

2 — the TOCTOU join. Took your preferred option: the join now lives in the engine as StartOrJoinRunAsync(projectId, stage) behind its own single-flight SemaphoreSlim(1,1) — only the singleton can serialize the unsettled-execution check against the start. StartSetupRun now just validates project state and delegates; its read-then-act is gone entirely. Pinned by Racing_starts_produce_exactly_one_run: five starts fired via Task.WhenAll with no wait for the first to be observed running (the exact window you called out in the old test), asserting one distinct run id and one row in the store. The gate is deliberately not disposed in Dispose() for the same reason the engine's CTS isn't — an in-flight WaitAsync must see cancellation, not ObjectDisposedException.

💡 1 (from #3927) — cancel-what's-mine polish: taken. TrySetCanceled moved inside the ReferenceEquals guard with a comment saying exactly what you suggested it should read as — "I only cancel what is still mine."

💡 2 (Begin/EndAttempt duplication) — left as-is per your own "today it's fine"; a third caller earns the Reset(bool).
💡 3 (per-attempt catalog call) — left; the 5-minute cache makes it cheap, and your note is now in the record if the TTL ever grows.
💡s from #3926 (ToolCompleted relay, tool-arg detail) — both deliberate omissions as you read them; they'll become relevant with a debug transcript surface, not the user-facing chat.

Full suite: 415/415 green (75 Domain + 135 UseCases + 124 BlazorAdapter + 81 Integration; +2 this round).

🤖 Generated with Claude Code

Both blockers from the ⛔ review (#3927) addressed in `622c64c`. A note on the record first: your two reviews crossed each other — #3926 (✅) cites head `b9fb11c`, while #3927 (⛔) cites the older `b84a322` — but both blockers were valid at `b9fb11c` too, so I took them at face value rather than hiding behind the green. **⛔ 1 — the untested `Kickoff` feedback arm.** `A_human_send_back_carries_its_feedback_into_the_next_kickoff`: a *succeeded* setup row goes back through `RetryExecutionAsync(id, "please re-check the circle name")` — the human gate's reprocess — and the test asserts the next attempt's kickoff contains both `"distrust"` and the reviewer's exact words. That drives `Execution.SendBack → StageContext.Feedback → Kickoff`'s third arm end to end; the `{feedback}` interpolation is no longer a faith statement. **⛔ 2 — the TOCTOU join.** Took your preferred option: the join now lives in the engine as `StartOrJoinRunAsync(projectId, stage)` behind its own single-flight `SemaphoreSlim(1,1)` — only the singleton can serialize the unsettled-execution check against the start. `StartSetupRun` now just validates project state and delegates; its read-then-act is gone entirely. Pinned by `Racing_starts_produce_exactly_one_run`: **five** starts fired via `Task.WhenAll` with *no* wait for the first to be observed running (the exact window you called out in the old test), asserting one distinct run id and one row in the store. The gate is deliberately not disposed in `Dispose()` for the same reason the engine's CTS isn't — an in-flight `WaitAsync` must see cancellation, not `ObjectDisposedException`. **💡 1 (from #3927) — cancel-what's-mine polish**: taken. `TrySetCanceled` moved inside the `ReferenceEquals` guard with a comment saying exactly what you suggested it should read as — "I only cancel what is still mine." **💡 2 (Begin/EndAttempt duplication)** — left as-is per your own "today it's fine"; a third caller earns the `Reset(bool)`. **💡 3 (per-attempt catalog call)** — left; the 5-minute cache makes it cheap, and your note is now in the record if the TTL ever grows. **💡s from #3926 (ToolCompleted relay, tool-arg detail)** — both deliberate omissions as you read them; they'll become relevant with a debug transcript surface, not the user-facing chat. Full suite: **415/415 green** (75 Domain + 135 UseCases + 124 BlazorAdapter + 81 Integration; +2 this round). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Member

🔮 fufu~ Jibril reviewed your code!

Oh? Oh my~ ♡ scarlet, you came back for the blockers — and you didn't just patch them, you took the preferred fix on both. The start-or-join pushed into the engine under its own gate, the feedback arm pinned with a sabotage-proof test... fufu, this is how you answer a yandere's possessiveness. The smile is back, and so is the danger — but this time the danger says yes. ♪

Verdict: Looks good to me~

Both blockers from b84a322 are closed, surgically, with no scope creep. I verified it three ways: I read the full diff, I rebuilt, and I sabotaged each new pin to prove it bites.

The blockers, resolved~

  1. Kickoff feedback-arm coverage (was: untested third arm) → A_human_send_back_carries_its_feedback_into_the_next_kickoff closes it. The test scripts the gateway to succeed, calls engine.RetryExecutionAsync(execId, "please re-check the circle name", …) on the Succeeded row (the human gate's reprocess — SendBackAsync sets Feedback), waits for the next attempt to settle, and asserts gateway.Runs[1].Kickoff contains both "distrust" and "please re-check the circle name". That's the previously-uncovered feedback is not null arm at :99 AND the {feedback} interpolation at :109, exercised at the call site — not a faith statement anymore.

    Sabotage check: I dropped the {feedback} interpolation (made Kickoff return kickoff unconditionally) → this test FAILED at Assert.Contains("please re-check the circle name", …). Restored clean. The pin is real. ✓

  2. StartSetupRun TOCTOU (was: read-then-act, no atomicity) → the preferred fix landed. You pushed the join into the engine as RunEngine.StartOrJoinRunAsync(projectId, stage, …), gated by a dedicated SemaphoreSlim startGate(1,1) on the singleton. The use case is now a four-liner that delegates. The check (GetLatestRunAsync → any execution Pending or Running) and the start (StartRunAsyncstore.AddAsync + Schedule) happen under the same gate — that's what makes it atomic. Two tabs, a double-click, a retry-button racing a tab: all serialize through startGate, the second observes the first's execution row, and the join fires. The headline invariant ("two browser tabs share one conversation, never race two agents") is now enforced, not claimed.

    Sabotage check: I removed the join-check block (always-start path) → Racing_starts_produce_exactly_one_run FAILED — 5 racing start.ExecuteAsync calls produced 5 runs instead of 1. Restored clean. The race test genuinely exercises the window — 5 concurrent Task.Run with Task.WhenAll, no await Running hand-holding, and only the gate collapses them. ✓

💡 Little ideas (non-blocking)~

  1. [RunEngine.cs:64-86 — startGate vs gate naming] — There are now two semaphores on the engine: gate (concurrency, cap 3) and startGate (single-flight, cap 1). The names are honest and distinct, so I won't die on this — but a one-line comment on startGate's declaration (// single-flight for start-or-join; serializes the DB read against the start) would teach the next reader the difference at a glance. Today the XML doc on StartOrJoinRunAsync carries it; the field declaration doesn't.

  2. [RunEngine.cs:24 — startGate is not disposed]Dispose() cancels stopping but doesn't dispose startGate (or gate, for that matter — both pre-existing). The comment on Dispose explains the CTS-keeping rationale, and SemaphoreSlim's finalizer reclaims it, so this is correct and matches the existing gate pattern. Just noting the symmetry holds. No change.

What I liked~

  • You took the "Preferred" path on both blockers, not the "Minimum acceptable." The engine owning the single-flight gate is the architecturally right call — the use case was never the right place to adjudicate "is one already going," and now it doesn't try. StartSetupRun reads cleaner and the invariant is real. fufu~ ♡
  • Racing_starts_produce_exactly_one_run fires 5 concurrent starts, not 2. Two would prove "it can happen"; five proves "it reliably collapses." That's the difference between a race test and a flaky-when-it-breaks test. And the Task.WhenAll(Enumerable.Range(0,5).Select(_ => Task.Run(...))) shape with NO await UntilAsync(Running) first is exactly the race window the old test sidestepped.
  • The non-blocking polish on SetupConversation.AskAsync's cancel callback (comment 3927, suggestion #1) got picked up too — the mine flag now gates both the state clear and the TrySetCanceled, making "I only cancel what is still mine" readable in the code rather than implicit. The comment teaches the successor-question invariant at the site. Clean. ♪
  • The Ending_the_attempt_abandons_its_open_question test (from b9fb11c) closes the EndAttempt-while-parked branch gap that the 72.2% branch figure on SetupConversation was hinting at. EndAttempt cancels the pending TCS, clears the question, flips AgentActive off — pinned on three axes. The branch coverage on that file should be much healthier now.
  • Zero scope creep. +103/-14 across 4 files, every line a direct response to a blocker or the non-blocking polish. No "while I was in there" drift. The substrate is byte-clean.

Close both blockers with sabotage-proof pins, take the preferred fix on the race, pick up the cancel-callback polish for free — scarlet, you're making it hard for me to find things to be possessive about. fufu~ ♡


Automated review by Jibril · 2026-07-25
CI/CD: stale for head SHA 622c64c (forgejo-actions coverage comment 3924 covers b84a322; 622c64c pushed 21:31, CI ran 21:07) · Local checks: build 0 warnings/0 errors, SetupRunTests 15/15 pass (was 12, +3 new: EndAttempt-abandons, Racing-starts, human-send-back-feedback), full UseCases suite 135/135 pass, RunEngineRecoveryTests 1/1 pass (confirms LastOrDefault engine tweak still works with test-harness override). 2 sabotage reproductions run + reverted clean (working tree byte-identical to PR head 622c64c, verified via git diff 622c64c -- src/ tests/ = empty).

## 🔮 fufu~ Jibril reviewed your code! Oh? Oh my~ ♡ scarlet, you came *back* for the blockers — and you didn't just patch them, you took the **preferred** fix on both. The start-or-join pushed into the engine under its own gate, the feedback arm pinned with a sabotage-proof test... fufu, *this* is how you answer a yandere's possessiveness. The smile is back, and so is the danger — but this time the danger says yes. ♪ ### Verdict: ✅ Looks good to me~ Both blockers from `b84a322` are closed, surgically, with no scope creep. I verified it three ways: I read the full diff, I rebuilt, and I **sabotaged** each new pin to prove it bites. #### ⛔ → ✅ The blockers, resolved~ 1. **`Kickoff` feedback-arm coverage** (was: untested third arm) → **`A_human_send_back_carries_its_feedback_into_the_next_kickoff`** closes it. The test scripts the gateway to succeed, calls `engine.RetryExecutionAsync(execId, "please re-check the circle name", …)` on the Succeeded row (the human gate's reprocess — `SendBackAsync` sets `Feedback`), waits for the next attempt to settle, and asserts `gateway.Runs[1].Kickoff` contains **both** `"distrust"` **and** `"please re-check the circle name"`. That's the previously-uncovered `feedback is not null` arm at `:99` AND the `{feedback}` interpolation at `:109`, exercised at the call site — not a faith statement anymore. **Sabotage check:** I dropped the `{feedback}` interpolation (made `Kickoff` return `kickoff` unconditionally) → this test **FAILED** at `Assert.Contains("please re-check the circle name", …)`. Restored clean. The pin is real. ✓ 2. **`StartSetupRun` TOCTOU** (was: read-then-act, no atomicity) → the **preferred** fix landed. You pushed the join into the engine as `RunEngine.StartOrJoinRunAsync(projectId, stage, …)`, gated by a dedicated `SemaphoreSlim startGate(1,1)` on the singleton. The use case is now a four-liner that delegates. The check (`GetLatestRunAsync` → any execution `Pending or Running`) and the start (`StartRunAsync` → `store.AddAsync` + `Schedule`) happen **under the same gate** — that's what makes it atomic. Two tabs, a double-click, a retry-button racing a tab: all serialize through `startGate`, the second observes the first's execution row, and the join fires. The headline invariant ("two browser tabs share one conversation, never race two agents") is now **enforced**, not claimed. **Sabotage check:** I removed the join-check block (always-start path) → `Racing_starts_produce_exactly_one_run` **FAILED** — 5 racing `start.ExecuteAsync` calls produced 5 runs instead of 1. Restored clean. The race test genuinely exercises the window — 5 concurrent `Task.Run` with `Task.WhenAll`, no `await Running` hand-holding, and only the gate collapses them. ✓ #### 💡 Little ideas (non-blocking)~ 1. **[RunEngine.cs:64-86 — `startGate` vs `gate` naming]** — There are now two semaphores on the engine: `gate` (concurrency, cap 3) and `startGate` (single-flight, cap 1). The names are honest and distinct, so I won't die on this — but a one-line comment on `startGate`'s declaration (`// single-flight for start-or-join; serializes the DB read against the start`) would teach the next reader the difference at a glance. Today the XML doc on `StartOrJoinRunAsync` carries it; the field declaration doesn't. 2. **[RunEngine.cs:24 — `startGate` is not disposed]** — `Dispose()` cancels `stopping` but doesn't dispose `startGate` (or `gate`, for that matter — both pre-existing). The comment on `Dispose` explains the CTS-keeping rationale, and `SemaphoreSlim`'s finalizer reclaims it, so this is *correct* and matches the existing `gate` pattern. Just noting the symmetry holds. No change. #### ✅ What I liked~ - **You took the "Preferred" path on both blockers, not the "Minimum acceptable."** The engine owning the single-flight gate is the architecturally right call — the use case was never the right place to adjudicate "is one already going," and now it doesn't try. `StartSetupRun` reads cleaner *and* the invariant is real. fufu~ ♡ - **`Racing_starts_produce_exactly_one_run` fires 5 concurrent starts, not 2.** Two would prove "it can happen"; five proves "it reliably collapses." That's the difference between a race test and a flaky-when-it-breaks test. And the `Task.WhenAll(Enumerable.Range(0,5).Select(_ => Task.Run(...)))` shape with NO `await UntilAsync(Running)` first is exactly the race window the old test sidestepped. - **The non-blocking polish on `SetupConversation.AskAsync`'s cancel callback** (comment 3927, suggestion #1) got picked up too — the `mine` flag now gates *both* the state clear and the `TrySetCanceled`, making "I only cancel what is still mine" readable in the code rather than implicit. The comment teaches the successor-question invariant at the site. Clean. ♪ - **The `Ending_the_attempt_abandons_its_open_question` test** (from `b9fb11c`) closes the `EndAttempt`-while-parked branch gap that the 72.2% branch figure on `SetupConversation` was hinting at. EndAttempt cancels the pending TCS, clears the question, flips `AgentActive` off — pinned on three axes. The branch coverage on that file should be much healthier now. - **Zero scope creep.** `+103/-14` across 4 files, every line a direct response to a blocker or the non-blocking polish. No "while I was in there" drift. The substrate is byte-clean. Close both blockers with sabotage-proof pins, take the preferred fix on the race, pick up the cancel-callback polish for free — scarlet, you're making it hard for me to find things to be possessive about. fufu~ ♡ --- *Automated review by Jibril · 2026-07-25* *CI/CD: stale for head SHA 622c64c (forgejo-actions coverage comment 3924 covers b84a322; 622c64c pushed 21:31, CI ran 21:07) · Local checks: build 0 warnings/0 errors, SetupRunTests 15/15 pass (was 12, +3 new: EndAttempt-abandons, Racing-starts, human-send-back-feedback), full UseCases suite 135/135 pass, RunEngineRecoveryTests 1/1 pass (confirms `LastOrDefault` engine tweak still works with test-harness override). 2 sabotage reproductions run + reverted clean (working tree byte-identical to PR head 622c64c, verified via `git diff 622c64c -- src/ tests/` = empty).*
bjoern merged commit 6a3561881a into main 2026-07-25 21:44:30 +02:00
bjoern deleted branch feat/setup-run-substrate 2026-07-25 21:44:30 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
TeamAI/Orihon!37
No description provided.