Repository navigation
fix(core): create a session lazily so a turn is never silently discarded - #115
Merged
Merged
Conversation
Microsoft.SourceLink.GitHub 8.0.0 pulled in Microsoft.Build.Tasks.Git 8.0.0, which has picked up advisory GHSA-23fw-v26w-5fgq. With TreatWarningsAsErrors on and no audit suppression, NU1902 became a fatal restore error in all six packable projects (Gemini, OpenAI, Anthropic, Ollama, Mtplx, Tools.Builtin), so a clean tree no longer restored. The package is redundant on .NET 10: SourceLink has shipped in the SDK since .NET 8. Removing it rather than suppressing the warning keeps the no-suppressed-warnings rule intact and fixes the cause instead of the symptom. Verified behaviour-preserving: packing Dmon.Protocol still produces the .snupkg and still embeds <repository type url branch commit> in the nuspec, so PublishRepositoryUrl / EmbedUntrackedSources / ContinuousIntegrationBuild continue to drive source linking via the SDK. Full Release build succeeds with zero warnings; test suite passes with 0 failures across 45 assemblies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
A turn submitted with no active session is silently discarded: TurnHandler.PersistNewHistoryEntriesAsync returns early when CurrentSession is null with no log, warning or host notice, and nothing in the core ever creates a session implicitly — SessionCreateCommand is reachable only from the console host's /new slash command. A user who starts the TUI and types loses the whole conversation without being told. This is not a recorded decision. It contradicts session-storage's unconditional "SHALL append the turn(s) to messages.jsonl", and agent-core's "not discarded after the turn". Found live: a real tool-calling turn ran, answered correctly, and persisted nothing. A second defect compounds it — /new itself is silent, violating console-host's "displays the new session context". The change creates sessions lazily on the first turn that needs one (not eagerly at startup: the store already holds 764 empty session directories out of 769, and eager creation would add one per launch), adds a non-command sessionStarted event to announce a session the host never asked for, and gives the console host one display path shared by explicit and implicit creation. Reusing SessionCreatedResultEvent would contradict ADR-015, which is explicit that a ResultEvent's id correlates to the command that produced it. ADR-015 already retains ErrorEvent as a non-command event, so sessionStarted extends that model rather than amending it — no superseding ADR needed, and task 1.1 gates on confirming that reading. Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
Adds `SessionStartedEvent : Event` carrying `SessionMeta`, registered on the `Event` polymorphic table with discriminator `sessionStarted`, and regenerates `docs/protocol/schema.json` so the leaf appears in the machine-readable export. Per ADR-015 §2 the event is a notification, not a command result: it derives directly from `Event` rather than `ResultEvent` and carries no correlation `id`, because an implicitly created session has no originating command to correlate to. Nothing emits the event yet — lazy creation lands in section 3. Tasks: 1.1, 1.2, 1.3 Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
Adds `ISessionHandler.CreateAndActivateAsync(string? agent, CancellationToken)`, implemented by `SessionHandler`: it creates the session via `ISessionStore`, sets it active, fires the session-activity notification, and returns the `SessionMeta` without emitting anything. Event emission stays the caller's decision. `session.create` now routes through the seam and then emits `SessionCreatedResultEvent` as before — create, activate, notify, emit, in that order, with a single activity notification. No existing test expectation changed. The seam is on the interface rather than private to `SessionHandler` because the implicit path in section 3 reaches it through `ISessionHandler`. This is what makes "indistinguishable from an explicitly created session" structural rather than a promise (design D4). The eight test fakes implementing the interface are updated in the same commit to keep the tree green. Tasks: 2.1, 2.2 Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
A turn submitted with no active session was silently discarded: `PersistNewHistoryEntriesAsync` returned early when `CurrentSession` was null, with no log and no host notice, and nothing in the core ever created a session implicitly. A user who started the TUI and simply typed lost the conversation without being told. `TurnHandler.SubmitAsync` now creates and activates a session via the section-2 seam when none is active, before the turn executes (design D2) — so the asset provisioner and the system-prompt build see a real session id — and emits `sessionStarted` carrying the new `SessionMeta`. The session binds to `agent: null`, matching what an explicit `session.create` resolves to by default from the same host (design D5). `sessionStarted` is emitted only when the core creates a session on its own initiative; `session.create` continues to report via `session.createResult` and emits no `sessionStarted`. Both are asserted by absence, not just presence, so moving the emission into the seam would redden the suite. The persist guard stays as a defensive check but is now loud: the `CurrentSession is null` branch logs a warning carrying the number of discarded history entries. The `_sessionStore is null` branch stays silent — a core running without persistence is a legitimate configuration. Tasks: 3.1, 3.2, 3.3 Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
Section 5.3 tabulates the notification events a `turn.submit` produces, and `sessionStarted` now joins that class. It does not actually reach gateway clients — `create` and `attach` both leave a session active before any `turn.submit` is accepted, so the core never creates one on its own initiative on this path — but the guide is the source a non-.NET client is built from, and saying nothing would leave it silently under-describing the core's turn stream for hosts speaking JSONL/stdio directly. Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
Adds `LazySessionCreationRealStackTests`, wired against a real `SessionHandler` over a real `SessionStore` and `AttachmentStore` rooted at a temp directory, isolated only at the `ISessionDirectoryResolver` path-computation seam. The integration-test fakes cannot prove any of this: they mint a `SessionMeta` in memory and call no `ISessionStore`, so the creating and appending stores are two objects that never meet. A fake-based test would still fail against pre-change code — appearing to satisfy 3.4's requirement — while proving nothing about persistence. - 3.4 drives a real tool call and asserts from disk that the new session's `messages.jsonl` carries both the `ToolCallPart` and the `ToolResultPart`. Verified red against `24b819b`'s `TurnHandler`. - 3.5 constructs the handlers as production does, runs no turn, and asserts the sessions root is never created — the guard against regressing to eager creation (design D1). - 3.7 forks and loads an implicitly created session and an explicitly created one, comparing each fork against its own source session. Also asserts the real seam cannot return a `SessionMeta` without setting `CurrentSession`, which is the state block 3A's guard test can only reach with a hard-coded fake. Tasks: 3.4, 3.5, 3.7 Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
After the gateway's two-step `session.create` -> path-less `session.load` handshake, a submitted turn must create no second session and emit no `sessionStarted`. Asserts structural absence of the event, an unchanged on-disk session-directory count (1 before and after), and an unchanged active session. Tested at core level rather than through the gateway harness: that harness backs `DriveSessionHandshakeAsync` with a fake core process replaying scripted stdout, so a turn driven through it never reaches `TurnHandler.SubmitAsync`. Driving a real core would mean a live-process integration test. What therefore remains on source inspection is only that the real handshake completes create-then-load before returning, which existing wire-level tests in `Dmon.Network.Tests` already cover. Tasks: 3.6 Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
`sessionStarted` was emitted on the turn's cancellation token. Because `EventEmitter.EmitAsync` honours its token, a `turn.abort` or a shutdown landing between the session being created and the event being emitted would throw `OperationCanceledException` — swallowed by design in the dispatcher — while the session was already created, on disk, and active. The host would never learn the session existed, and would never recover: every later turn finds `CurrentSession` non-null, so the event is never re-emitted, and for the rest of that core's life the host writes to a session id it was never told about. That is the core/host divergence this change exists to remove. The emit now uses `CancellationToken.None`, matching the convention already used in this file for `turnEnd` and the provider-switch events, which must reach the host even when the turn aborted. The creation call keeps the turn token — a cancelled creation means there is no session to announce. Also corrects an overclaiming comment on the no-directory test: the real core start path runs `BootstrapService`, which creates the sessions root, so that test asserts something narrower than "the core creates nothing". No assertion changed. Findings: supervisor review of section 3 (24b819b..1ce9690) Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
`/new` tracked the new session silently — the one command a user must know in
order to avoid losing their conversation gave no sign it worked, violating the
standing `console-host` requirement that it "displays the new session context".
`TrackActiveSession` now owns the display as well as the tracking, so every
route that makes a session active announces it: `[Session] {verb}: {id}` for
created, forked, cloned and loaded, plus the new `sessionStarted` event. Putting
the display in the shared method rather than in each case is what removes the
class of bug where one route is silent (design D6) — a future route cannot be
quiet by omission.
`/new` and an implicitly started session both use the verb "Started" through the
same format string, so explicit and implicit start are indistinguishable to the
user, asserted by comparing rendered scrollback rather than format strings.
Tasks: 4.1, 4.2, 4.3
Change: lazy-session-creation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
Corrects the section-4 record: the display fires after the ActiveSessionId assignment, not before, and the [Session] prefix grammar collision was introduced by this change rather than inherited. Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
The event is additive on the wire, so hosts that do not know it must be undisturbed by it. The design called for a regression test rather than the source inspection the proposal relied on. 5.1 drives the real `Dmon.Runtime` correlation loop, feeding `sessionStarted` mid-flight — after a command is issued, before its result — and asserts the pending command still completes with its correctly correlated result. The interleaved event carries a different session id, so a weakened guard fails on an asserted value rather than merely on the absence of an exception. 5.2 pushes the event through `Dmon.Desktop`'s real subscription wiring and asserts it is ignored without throwing and without disturbing the tracked session. `SessionViewModel.TrackActiveSession` is a switch statement with no default arm, so an unmatched event falls through silently. Tasks: 5.1, 5.2 Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
All five implementation sections are closed with supervisor approval. Gates 6.1-6.3 verified on this tree: build clean, all 20 suites green, change valid. 6.4 is human-in-the-loop and remains for the Product Owner, along with a decision the section-5 supervisor surfaced: Dmon.Desktop's reload-reattach path is unreachable by construction, so after this change a Desktop reload silently splits one conversation across two session directories. Tasks: 6.1, 6.2, 6.3 Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
demo/ holds the agent we show to an audience — first outing is a lightning talk at XP Manchester. The intent is that it is updated whenever a feature becomes ready to show the world, rather than being rebuilt from scratch each time. What it demonstrates: the conversation runs entirely on a local MLX model (gemma-4-e4b on the OptiQ-4bit quant), so nothing said to it leaves the machine; only the grounded web-search query egresses, to Gemini, and only when the model chooses to call the tool. Two shape decisions are forced rather than chosen, and both are written into the file header so they are not rediscovered the hard way: - #:project, not #:package. The published 0.2.0 packages predate the headline verb — Dmon.Providers.Mlx 0.2.0 exports only AddMlxFirstline/AddMlxEscalation, not UseMlx (PR #104). Local project refs are the only way to demo merged-but-unreleased work. - Agent.cs, not Dmon.cs. CoreResolver tier 1 claims any cwd Dmon.cs and builds it via BuildFileBasedProgramAsync, which passes -p:ManagePackageVersionsCentrally=false and so strips the versions off #:project refs to this repo's CPM-managed projects, failing NU1015 in every one. The Agent.cs name keeps tier 1 from claiming it, so a host launched with cwd=demo/ falls through to tier 2 (DMON_CORE_PATH). sandbox-code/Agent.cs carries the same scar. Builds and publishes clean. A live tool-calling turn is human-in-the-loop and not yet verified; the run recipe is in the file header. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
The Product Owner ran the TUI against the demo composition root, typed a question without `/new`, and saw `[Session] Started: 43f846a9-...`. The session directory holds a valid meta.json (agent: null, as design D5 predicts) and a three-record messages.jsonl: the user text, the assistant text plus its `web_search` tool call, and the tool result — with the oversized result offloaded to attachments/ and back-referenced. That is the proposal's root-cause scenario, live: a real tool-calling turn that previously ran to completion and persisted nothing now persists all of it. All 20 tasks are ticked. Tasks: 6.4 Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
The desktop-host spec already requires reload to re-open the active session directory, unconditionally. The implementation gates that on an _activeSessionId that is only written by results of commands Dmon.Desktop never sends, so the clause has never executed. This change gives that latent violation a cost: a reload now silently splits one conversation across two session directories. Adds a MODIFIED desktop-host delta (both existing scenarios kept verbatim, one added) and tasks 7.1-7.3, at the Product Owner's direction. Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
The desktop-host spec requires reload to "re-open the active session directory", unconditionally. That clause had never executed: it is gated on `_activeSessionId`, which was written only by results of session.create/fork/ clone/load — commands Dmon.Desktop never sends. Desktop has been in breach of that SHALL since it shipped. This change gave the breach a cost. After lazy creation, a Desktop turn creates a real session with real persisted content, reload silently skips the reattach, and the next turn creates a second session — one conversation split across two directories with no signal. `TrackActiveSession` now also tracks `sessionStarted`, so the session the core creates on its own initiative becomes the host's active session and reload sends `session.load` for it. No session-identity display line is added: Desktop has no session chrome, and adding one is a separate design question. Also restores a guarantee this block would otherwise have removed — the switch's tolerance of a genuinely unhandled event type, which lost its dedicated test when `sessionStarted` graduated to a handled case. Tasks: 7.1, 7.2, 7.3 Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
The section-7 commit overwrote the DEVLOG's `## NEXT` with a different change's handoff notes from a different repository, destroying this change's owed-before-archive record: the tech-debt files, the standing-spec sync note, every parked item, and the false-green concurrency warning. Cause: `$TMPDIR` is shared across sessions and repositories. The `## NEXT` rewrites were staged at `$TMPDIR/devlog/next.md`; a concurrent dmon-home session wrote its own file to that path mid-run, and the section-7 append reused it without rewriting it in the same command. Restored from `6c2b84c` and re-written for a closed section 7, staged in the session-specific scratchpad with the file written in the same command that consumes it. Adds the section-7 supervisor's notes: a fourth tech-debt item (CoreSessionService can permanently complete its shared event subject on a reload race), D6's principle being unhonoured on Desktop, the missing memory barrier on `_activeSessionId`, and a correction to this change's own overstated claim that Desktop had been violating the reload SHALL since it shipped. Findings: supervisor review of section 7 (6c2b84c..844b744) Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
Five notes plus index lines, moving them off a DEVLOG `## NEXT` that archives with the change: - no full-stack gateway test harness — `Dmon.Network.Tests` fakes the core with a scripted-stdout replayer, so every gateway task must test one level down - `SpySessionStore` turn tests cannot see persistence — the creating and appending stores never meet, so such a test can fail against pre-change code while proving nothing; use block 3B's `FakeResolver` pattern - an aborted `session.create` orphans a `meta.json`-less directory — pre-existing but newly reachable without user action, and a candidate mechanism for the 99.3% empty-session litter, with the cheap check to confirm or rule it out - a Desktop reload can permanently kill the session's event stream — a race completes the never-recreated event subject; newly consequential because section 7 gave users a reason to reload - the standing spec would inherit a cross-repo tolerance claim — due at archive, then delete Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
Syncs the three delta specs into the standing specs and moves the change to `openspec/changes/archive/2026-09-09-lazy-session-creation/`. - `agent-core` 16 -> 18 requirements: implicit session creation on first turn, and the `sessionStarted` non-command event. - `console-host` 9 -> 10: session start is surfaced to the user. - `desktop-host` 11 -> 11: the `/reload` requirement is modified in place, keeping both existing scenario headers verbatim and adding a third. The `agent-core` delta was amended before syncing so the standing spec carries the by-construction argument for `sessionStarted` tolerance — a gateway-fronted session is always active before `turn.submit` is accepted, so the core never emits the event on that path — rather than the proposal's untestable claim about how another repository's client handles unknown events. That discharges the tech-debt note, which is deleted here. `openspec validate --all --strict` is unchanged against origin/main: the same 8 pre-existing stale-Purpose failures, none in a spec this change touched, confirmed via a worktree baseline. Change: lazy-session-creation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A turn submitted when no session was active was silently discarded.
TurnHandler.PersistNewHistoryEntriesAsyncreturned early whenCurrentSessionwas null — no log, no warning, no host notice — and nothing in the core ever created a session implicitly. Sincesession.createwas reachable only from the TUI's/new, a user who started the terminal and simply typed lost the entire conversation without being told.That contradicted an accepted spec rather than implementing a decision:
session-storagerequires unconditionally that "at turn completion the orchestration point SHALL append the turn(s) tomessages.jsonl", with no "if a session is active" qualifier.A second defect sat alongside it:
/newitself was silent, so the one command a user had to know in order to avoid losing their work gave no sign it had worked.What changed
sessionStartedevent carryingSessionMetaannounces a session the host never asked for. It derives fromEvent, notResultEvent, and carries no correlation id — per ADR-015 §2, which partitions the surface into command results and id-less notifications.[Session] {verb}: {id}for created, forked, cloned, loaded and started — with the display insideTrackActiveSessionrather than per-case, so a future route cannot be silent by omission.Dmon.Desktopnow trackssessionStarted, fixing a conformance defect: thedesktop-hostspec requires reload to "re-open the active session directory" unconditionally, but that clause had never executed — it was gated on a field only written by results of commands Desktop never sends. Without this, a reload after the fix would silently split one conversation across two session directories.Additive on the wire;
x-protocolVersionstays0.2(ADR-011 D5 makes compatibility an identicalMajor.Minor, so bumping for an additive event would force hosts into lockstep for no break).Verification
Live, by the Product Owner — the scenario the change exists for. A tool-calling turn typed without
/newproduced[Session] Started: 43f846a9-…, and that session'smessages.jsonlholds all three records: the user text, the assistant text plus itsweb_searchtool call, and the tool result — with the oversized result offloaded toattachments/and back-referenced.meta.jsoncame back withagent: null, matching what/newproduces, as design D5 predicted.Automated:
make buildclean withTreatWarningsAsErrors; all 20 suites green (Dmon.Core.Tests626,Dmon.Terminal.Tests194,Dmon.Desktop.Tests56,Dmon.Network.Tests223,Dmon.Runtime.Tests45). The end-to-end persistence test runs against a realSessionStoreon a real filesystem and was verified red against the pre-changeTurnHandler.openspec validate --all --strictis unchanged againstmain— the same 8 pre-existing stale-Purposefailures, none in a spec this change touches, confirmed with a worktree baseline.Review notes
sessionStartedwas emitted on the turn's cancellation token, so an abort landing between creation and emission would have left a live session the host was never told about — permanently, since later turns findCurrentSessionnon-null. Fixed toCancellationToken.None, matching the convention already used forturnEnd.demo/is cherry-picked from the unmergeddemo/xpmanchesterbranch (commit9f2784e,Agent.cs+build.sh), because task 6.4's verification recipe depended on it.openspec/changes/archive/2026-09-09-lazy-session-creation/.tech-debt/rather than fixed here — a missing full-stack gateway harness, weakerSpySessionStoretests, an aborted-create orphaning a directory, and a pre-existingCoreSessionServicereload race that this change makes newly consequential.The full build record, including every reviewer and supervisor finding, is in the archived
DEVLOG.md.🤖 Generated with Claude Code
https://claude.ai/code/session_01QQc6G2N3hMBcLNWJhJXyvP