Bump dotnet-sdk from 10.0.102 to 10.0.103#1
Closed
dependabot[bot] wants to merge 1 commit into
Closed
Conversation
Bumps [dotnet-sdk](https://github.com/dotnet/sdk) from 10.0.102 to 10.0.103. - [Release notes](https://github.com/dotnet/sdk/releases) - [Commits](dotnet/sdk@v10.0.102...v10.0.103) --- updated-dependencies: - dependency-name: dotnet-sdk dependency-version: 10.0.103 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Aaronontheweb
enabled auto-merge (squash)
February 24, 2026 05:51
This was referenced Feb 28, 2026
Contributor
Author
|
Superseded by #199. |
auto-merge was automatically disabled
March 11, 2026 03:45
Pull request was closed
4 tasks
Aaronontheweb
added a commit
that referenced
this pull request
Apr 11, 2026
…ema init Five related cleanups surfaced during the /simplify review of the `remove-domain-migration` branch. Each was defensive code that papered over a latent bug or was cleaning up shapes no current write path produces. 1. Delete hygiene UPDATE #1 (turn-completion/ConversationTrace → Never) and the defensive filters/ranker penalties keyed on them. `MemoryCurationPipeline.Extract` early-returns on TurnComplete+!Explicit and only emits "Project Fact:" / "Project Milestone:" titles — nothing currently writes `title='turn-completion'` to memory_documents. 2. Delete hygiene UPDATE #2 (metadata backfill for malformed doc:% titles) plus the pinning test `InitializeAsync_demotes_malformed_auto_recall_ documents_to_searchable` and the `MissingMetadataFacet` constant. 3. Replace the startup FTS rebuild (unconditional DROP + CREATE VIRTUAL TABLE + full INSERT ... SELECT, O(rows) every boot) with `CREATE VIRTUAL TABLE IF NOT EXISTS`. To make this safe, fix the runtime FTS write paths that the rebuild was papering over: - `InsertDocumentFtsAsync` / `InsertRecordFtsAsync` → rename to `UpsertDocumentFtsAsync` / `UpsertRecordFtsAsync`, DELETE-then-INSERT so repeat upserts do not leave duplicate FTS rows - `TombstoneDocumentAsync` now deletes the FTS row - `SupersedeRecordAsync` now deletes the old record's FTS row - Fix two raw-string-literal SQL interpolation bugs (`TombstoneDocumentAsync`, `SupersedeRecordAsync`) where the missing `$` prefix caused `{MemoryUpdateSemantics.*.ToWireValue()}` to be written literally into the column. 4. Unify startup schema ordering. Move `SchemaMigrationHostedService` registration before memory services so its StartAsync runs before `MemoryCurationWorkerService`. Delete the synchronous `memoryStore.InitializeAsync(...).GetAwaiter().GetResult()` at DI registration time. Have the hosted service call both the versioned migrator and `SQLiteMemoryStore.InitializeAsync` in sequence so there is exactly one startup path for schema setup.
Aaronontheweb
added a commit
that referenced
this pull request
Apr 11, 2026
* refactor(memory): drop domain-column migration from InitializeAsync The fresh CREATE TABLE schema is already domain-free. The DROP COLUMN / DROP INDEX migration and the DropColumnIfExistsAsync/DropIndexIfExistsAsync helpers were only there for backward compatibility with existing user DBs, which is unnecessary — the store is single-user and the DB is disposable. * refactor(memory): drop EnsureColumnExistsAsync dead migration helpers Every column being "ensured" in InitializeAsync (memory_class, expires_at, aliases_json, facets_json, slots_json, boundary, audience on both memory_documents and memory_records) is already declared in the CREATE TABLE statements in the same method. On every daemon startup the 14 calls issued 14 redundant PRAGMA table_info scans for columns that always existed on fresh installs. Same reasoning as the DROP COLUMN cleanup: single-user prototype, disposable DB, no backward-compat requirement. * refactor(memory): clean up dead hygiene, fix FTS lifecycle, unify schema init Five related cleanups surfaced during the /simplify review of the `remove-domain-migration` branch. Each was defensive code that papered over a latent bug or was cleaning up shapes no current write path produces. 1. Delete hygiene UPDATE #1 (turn-completion/ConversationTrace → Never) and the defensive filters/ranker penalties keyed on them. `MemoryCurationPipeline.Extract` early-returns on TurnComplete+!Explicit and only emits "Project Fact:" / "Project Milestone:" titles — nothing currently writes `title='turn-completion'` to memory_documents. 2. Delete hygiene UPDATE #2 (metadata backfill for malformed doc:% titles) plus the pinning test `InitializeAsync_demotes_malformed_auto_recall_ documents_to_searchable` and the `MissingMetadataFacet` constant. 3. Replace the startup FTS rebuild (unconditional DROP + CREATE VIRTUAL TABLE + full INSERT ... SELECT, O(rows) every boot) with `CREATE VIRTUAL TABLE IF NOT EXISTS`. To make this safe, fix the runtime FTS write paths that the rebuild was papering over: - `InsertDocumentFtsAsync` / `InsertRecordFtsAsync` → rename to `UpsertDocumentFtsAsync` / `UpsertRecordFtsAsync`, DELETE-then-INSERT so repeat upserts do not leave duplicate FTS rows - `TombstoneDocumentAsync` now deletes the FTS row - `SupersedeRecordAsync` now deletes the old record's FTS row - Fix two raw-string-literal SQL interpolation bugs (`TombstoneDocumentAsync`, `SupersedeRecordAsync`) where the missing `$` prefix caused `{MemoryUpdateSemantics.*.ToWireValue()}` to be written literally into the column. 4. Unify startup schema ordering. Move `SchemaMigrationHostedService` registration before memory services so its StartAsync runs before `MemoryCurationWorkerService`. Delete the synchronous `memoryStore.InitializeAsync(...).GetAwaiter().GetResult()` at DI registration time. Have the hosted service call both the versioned migrator and `SQLiteMemoryStore.InitializeAsync` in sequence so there is exactly one startup path for schema setup. * fix(memory): unconditional memory init + atomic FTS lifecycle Two regressions introduced by the previous cleanup commit, caught by /simplify review: 1. SchemaMigrationHostedService.StartAsync early-returned when the akka persistence provider wasn't SQLite or auto-migrate was disabled, which silently skipped _memoryStore.InitializeAsync(). The memory store always uses SQLite regardless of akka persistence config, so its schema init now runs unconditionally outside the guard. 2. TombstoneDocumentAsync, SupersedeRecordAsync, and UpdateDocumentTextAsync ran the base-table UPDATE/INSERT and the FTS delete/insert as two separate implicit transactions. Before this branch the startup FTS rebuild cleaned up any drift; with the rebuild gone, a crash between the two statements leaves FTS stale (tombstoned doc still searchable, superseded record's old row orphaned). Wrap each path in an explicit BeginTransaction/Commit so both writes land together or neither does.
This was referenced May 6, 2026
This was referenced May 13, 2026
Aaronontheweb
added a commit
to codymullins/netclaw
that referenced
this pull request
May 20, 2026
Followup to the Copilot review netclaw-dev#1 (review-of-review): the previous fix kept the existing ConcurrentDictionary<string, CachedToken> cache AND added a parallel ConcurrentDictionary<string, SemaphoreSlim> refreshLocks keyed identically. Two dictionaries, one logical entity per OAuth token. Collapse both into a single ConcurrentDictionary<string, CacheSlot> where CacheSlot carries the (volatile, nullable) CachedToken and its own SemaphoreSlim. Removes the lifetime mismatch between cache writes and lock creation, halves the per-token dictionary lookup cost, and makes the lock and cache lifetimes provably identical. Token is volatile so the lock-free fast path in GetTokenAsync sees a fresh write from another thread's refresh on weak-memory architectures (ARM); reference assignments are atomic on the CLR but visibility requires the barrier. All 20 GitHubCopilot tests still pass — the behavior is unchanged from the caller's perspective.
Aaronontheweb
added a commit
that referenced
this pull request
May 20, 2026
* initial pass at gh copilot as a provider
* chore(providers): use Netclaw-owned GitHub App client_id for Copilot
Replaces the akt-sh placeholder client_id with the registered netclaw-dev
GitHub App. Removes the now-stale TODO comment.
The new App is configured with device flow enabled, no callback redirect
in use (device flow doesn't need one), and minimal permissions (Account
email read-only). Consent screen now shows the Netclaw brand instead of
a third-party app name.
* fix(providers): replace dead 'netclaw provider fix' references in Copilot
The 'provider fix' subcommand was referenced in three Copilot error
messages and the OpenSpec but was never implemented in ProviderCommand.cs.
Users hitting an expired OAuth token would have been directed to a
command that doesn't exist.
Rewrite all sites to the working 'provider remove' + 'provider add'
workflow that's already implemented. Updates the matching unit test
assertion.
Out of scope: the same dead reference exists in OpenAI Codex paths
(OpenAiProviderPlugin, OpenAiDescriptor). Tracking separately.
* docs(skill): add LLM Providers section with github-copilot
Per CLAUDE.md System Skills Sync Rule — when adding a new provider type,
netclaw-operations must document it for the running agent.
Adds:
- Route-by-intent row pointing to a new LLM Providers section
- Provider management subcommand table
- Supported provider types table including github-copilot
- Copilot-specific 'add' / re-auth instructions
Bumps metadata.version 2.5.0 → 2.6.0. CI handles publishing the manifest
on release tags.
* docs(spec): reconcile capabilities filter wording with implementation
ParseCopilotModels in GitHubCopilotDescriptor only removes entries whose
capabilities.type is explicitly non-chat; entries that omit capabilities
entirely pass through. The unit test
(Probe_FiltersByCapabilityAndPickerEligibility, asserting `no-caps` is
retained) documents this as intentional — Copilot's /models payload
shape varies across model generations and we treat missing capabilities
as 'unknown but selectable' rather than implicitly non-chat.
Updates the SHALL clause and the matching scenario to describe the
implemented behavior.
* test(providers): cover CopilotRequestPolicy headers and plugin wiring
Adds two test files:
CopilotRequestPolicyTests
- ProcessAsync_AppliesAllFourRequiredHeaders: asserts Authorization
(Bearer + Copilot token), copilot-integration-id, editor-version,
and openai-intent all land on the outbound request, and that the
next policy in the pipeline runs.
- ProcessAsync_OverwritesPreviousAuthorizationHeader: confirms the
placeholder ApiKeyCredential the SDK sets is replaced with the real
short-lived Copilot bearer on every call (the production failure
mode this guards against is a stale placeholder reaching the API).
GitHubCopilotProviderPluginTests
- CreateChatClient_DefaultEndpoint_ReturnsNonNullClient
- CreateChatClient_CustomEndpoint_DoesNotThrow
- Plugin_AdvertisesGitHubCopilotTypeKey
Brings the Copilot test count from 14 to 19. Closes the gap noted in
the PR review around 'no integration test for the chat-completion path'.
* test(smoke): update tapes + provider-manager baseline for github-copilot
Adding github-copilot to the provider catalog inserted a new alphabetical
TypeKey entry between anthropic and ollama, which shifted the wizard /
provider-manager picker indexing by one and made every tape that relied
on 'one Down from Anthropic = Ollama' navigate to github-copilot instead.
Fixes:
- tests/smoke/tapes/init-wizard.tape: Down -> Down 2
- tests/smoke/tapes/provider-add.tape: Down -> Down 2
- tests/smoke/tapes/screenshots/wizard-screens.tape: Down -> Down 2
- tests/smoke/screenshots/provider-manager-empty.approved.png: regenerated
from the failed CI run's actual.png (Linux runner, same VHS/font
setup as the comparison harness; byte-for-byte stable)
Still needs regen after next CI cycle:
- wizard-provider-picker.approved.png. The failing tape crashed on a
Wait+Screen timeout before VHS flushed its captured PNGs (VHS exit 1
discards in-progress output), so we have no new actual yet. The next
CI run will produce one; we'll commit it as the new baseline then.
wizard-security-posture.approved.png is unchanged content-wise and
should pass once the tape completes.
* test(smoke): regenerate wizard-provider-picker baseline for 6-entry picker
Captured from CI run 26138716273 after the tape fix (Down 2) let
wizard-screens.tape reach the Screenshot step. The new baseline shows
the picker with Anthropic / GitHub Copilot / Ollama / OpenAI /
llama.cpp / vLLM / OpenRouter — the alphabetical-by-TypeKey ordering
that the tape comments document.
Same Linux runner / VHS / font setup as the comparison harness, so
the bytes are stable.
* fix(providers): address Copilot review — disposal, dedupe, validation, sync path
Four bug fixes from the GitHub Copilot pull-request review:
1. OAuthDeviceFlowService.PostFormAsync leaked HttpRequestMessage (and
its FormUrlEncodedContent). Callers also did not dispose the response.
In the device-flow polling loop both grew connection-handle pressure.
Now wraps the request in `using`, makes the helper async, and
`using`s the response at every call site.
2. CopilotTokenExchanger.GetTokenAsync allowed N concurrent calls to
ExchangeAsync for the same OAuth token when a burst of chat requests
arrived inside the 2-minute refresh buffer. Adds a per-key
SemaphoreSlim with double-checked locking — the first caller refreshes,
the rest wait and read the new cache entry. Cuts the worst-case fan-out
to a single /copilot_internal/v2/token request per refresh.
3. ExchangeAsync trusted the deserialized token payload. System.Text.Json
does not enforce required-ness on positional record parameters, so a
{} response would yield Token=null/ExpiresAt=0 and we'd cache a
useless Bearer. Validates non-empty Token and positive ExpiresAt
before caching; failure throws an InvalidOperationException with the
offending endpoint URL in the message.
4. CopilotRequestPolicy.Process performed sync-over-async via
GetAwaiter().GetResult() on an HTTP call. Deadlock risk under a sync
context, blocks the calling thread on network I/O. The OpenAI SDK
uses ProcessAsync for chat completions; the sync overload was only
present because PipelinePolicy declares it. Now throws
NotSupportedException with a clear message directing callers to the
async pipeline. Adds a regression test.
* refactor(providers): remove unused ITokenExchanger interface
The interface was introduced alongside CopilotTokenExchanger with an
aspirational doc-comment about "future providers needing transparent
token refresh", but every consumer (ProviderDescriptorCatalog,
GitHubCopilotDescriptor, GitHubCopilotProviderPlugin, CopilotRequestPolicy)
depends on the concrete CopilotTokenExchanger class. The interface had
zero implementations besides Copilot and no callers polymorphic over it.
Delete it. We can reintroduce a token-exchange contract when a second
provider actually needs one (OpenAI Codex doesn't — its OAuth tokens
go straight to api.openai.com).
Per the Copilot review item #5 and the YAGNI guidance in CLAUDE.md.
* docs(copilot): correct CLI syntax and project paths in openspec
Cleanup pass over the openspec change docs that Copilot flagged in
review (items 6, 7, 8, 9):
- proposal.md, design.md, tasks.md: replace the non-existent
'--type github-copilot' flag with the actual positional CLI syntax,
'netclaw provider add <name> github-copilot --auth oauth-device'.
ProviderCommand only parses positional <name> <type> args.
- tasks.md: redirect every 'src/Netclaw.Providers.Tests/...' reference
to 'src/Netclaw.Daemon.Tests/Providers/GitHubCopilot/...' (the
Providers.Tests project doesn't exist; the GitHubCopilot tests live
under Daemon.Tests).
- tasks.md §8: drop the imagined 'netclaw provider probe' subcommand
(not implemented in ProviderCommand) and replace
'run-tapes.sh init-wizard ...' with the actual harness entry point
'./scripts/smoke/run-smoke.sh light'. Reword §8.7 to describe what
the add flow actually does (it exercises the probe path internally).
Also corrects 'Netclaw.sln' to 'Netclaw.slnx'.
- spec.md: ProviderEntry does not carry a name field — it's the
dictionary key in the Providers config. Reword the auth-expired
scenario to clarify that the name surfacing happens at the caller
layer (probe / chat-completion path), not inside
CopilotAuthExpiredException.
- design.md: rewrite the 'probe' mitigation paragraph to point at the
real seam (ProviderDescriptorRegistry.ProbeAsync invoked by the
TUI/CLI add flow), since there's no standalone probe subcommand.
* refactor(copilot): consolidate token cache + refresh lock into one slot
Followup to the Copilot review #1 (review-of-review): the previous fix
kept the existing ConcurrentDictionary<string, CachedToken> cache AND
added a parallel ConcurrentDictionary<string, SemaphoreSlim> refreshLocks
keyed identically. Two dictionaries, one logical entity per OAuth token.
Collapse both into a single ConcurrentDictionary<string, CacheSlot> where
CacheSlot carries the (volatile, nullable) CachedToken and its own
SemaphoreSlim. Removes the lifetime mismatch between cache writes and
lock creation, halves the per-token dictionary lookup cost, and makes
the lock and cache lifetimes provably identical.
Token is volatile so the lock-free fast path in GetTokenAsync sees a
fresh write from another thread's refresh on weak-memory architectures
(ARM); reference assignments are atomic on the CLR but visibility
requires the barrier.
All 20 GitHubCopilot tests still pass — the behavior is unchanged from
the caller's perspective.
* security(copilot): strip response body from validation exception messages
The validation throws added in the previous fix included Truncate(body)
in the missing-token path. Even though the validation only runs on a
2xx response that nominally has no usable token, a pathological response
body could still contain a token-shaped substring. Exception messages
land in daemon.log and bug-report attachments — best practice is to
never include token-exchange response bodies in messages we don't
strictly need.
Endpoint URL + missing-field indicator is sufficient for diagnostics
without any risk of leaking a credential into logs. expires_at value
(integer, never a credential) remains in its message.
Found during a pre-merge security audit.
---------
Co-authored-by: Aaron Stannard <[email protected]>
This was referenced May 31, 2026
Open
Aaronontheweb
added a commit
that referenced
this pull request
Jun 3, 2026
* feat(update): channel-aware, semver-correct update check (#1027) Make the binary update check honor an opt-in beta channel and compare versions by SemVer 2.0.0 precedence, so beta testers are notified of the next prerelease while stable users are never offered one. - BinaryFeedManifest: add `latestPrerelease` (newest of {stable, prerelease}). - SemVer: self-contained 2.0.0 precedence comparator (no NuGet.Versioning), matching the bash manifest generator's rules; IsNewerVersion uses it instead of System.Version (which couldn't parse a prerelease suffix at all). - BuildInfo.FullVersion: read the assembly informational version (keeps `-beta1`) so a beta build doesn't report its stripped core and strand. - DaemonConfig.UpdateChannel (stable default | beta) + config schema; parse fails loudly on an unknown value. - EvaluateManifest/CheckForUpdateAsync are channel-aware: stable reads only `latest`; beta reads `latestPrerelease` and rolls onto a superseding stable. - Thread channel + FullVersion through the daemon check, `netclaw update`, the startup notice, `netclaw status`, and `netclaw doctor`. Stable clients structurally never read `latestPrerelease`. The check stays advisory-only; Daemon.DisableSelfUpdate still blocks in-place update. Tests: SemVer precedence, channel-aware evaluation, ParseUpdateChannel. * docs(openspec): add release-channels capability spec Capture the beta (prerelease) release-channel capability end-to-end: manifest pointer semantics, prerelease-aware publishing, installer/Docker channel selection, and the channel-aware update-check policy. Documents both PR #1314 (merged) and the update-check work in this PR. Key invariant specified: a stable client is never offered a prerelease. * test(semver): add CsCheck property-based tests for SemVer Generate thousands of random valid SemVers and assert the comparator's correctness laws: parse-totality, antisymmetry, transitivity, IsNewer/compare consistency, build-metadata invariance, stable-outranks-prerelease, and numeric-below-alphanumeric precedence. Complements the fixed-example SemVerTests with algebraic coverage over a large random space. Adds CsCheck 4.7.0 (test-only) via central package management. * fix(update): address code-review findings on the channel-aware update check - doctor: inject DaemonConfig for the channel instead of hand-reading netclaw.json, so a non-string Daemon.UpdateChannel no longer crashes the whole `netclaw doctor` run (findings #1, #4). - update-check: drop the low-value 1h TTL and keep a plain last-result store for the daemon /status API (finding #3). Make `channel` required (no default) on EvaluateManifest + CheckForUpdateAsync so a forgotten arg can't silently fall back to stable (finding #9). - SemVer: parse numeric prerelease identifiers as long, matching the bash generator's unbounded ints (finding #6). - /status: report FullVersion in the no-check-yet branch for consistency with the post-check branch (finding #8). - dotted prerelease convention (beta.N): update docs/examples + widen the property-test generators + add example cases; the release version gate now rejects mixed identifiers like `beta1` so a non-dotted tag can't ship and silently mis-order the channel (finding #2). - conformance: extract the generator's precedence key to feeds/scripts/semver_key.py and assert BOTH it and the C# SemVer comparator order one shared fixture (feeds/scripts/semver-order.txt) — via a C# test and a CI check — so the two implementations can't drift (finding #7). Also updates the release-channels OpenSpec change (dotted-tag requirement plus risk/decision notes).
Aaronontheweb
added a commit
that referenced
this pull request
Jun 3, 2026
* docs(openspec): plan bound-tool-output-with-file-spill change Unify how large tool output is bounded across shell_execute, background jobs, and file_read. Today two truncation stages disagree — BoundedDrainAsync (32k head+tail, in the tool) and ClampToolResult (12k head-only, in the pipeline) — so the preserved tail is discarded downstream and stderr can be dropped, while background_job and the default file_read path still materialize unbounded output (#1300, #1301) and file_read never redacts. The change introduces one bounded-output mechanism: a single inline budget N (MaxInlineToolResultChars, default lowered to 2000) with head+tail retention, a spill of the full redacted output to sessionDir/tool-calls/{toolCallId}.log with a steer to read ranges/grep, and a shared bounded-output reader used by all three call sites. Redaction reuses the existing SecretOutputRedactor (redact-on-write over a bounded capture buffer; redact-on-read in file_read). Artifacts: proposal, design (8 decisions incl. why spill lives in the tool, not the pipeline), spec deltas across bounded-tool-output (new), netclaw-tools, netclaw-session, background-job-execution, tool-call-metadata, and an 8-group task plan. Closes #1300 and #1301 on implementation; media egress (#1296) stays a separate change (different fix). * refactor(tools): extract BoundedOutputReader from ShellTool (task group 1) Move BoundedDrainAsync + the tail-ring helpers out of ShellTool into a reusable BoundedOutputReader (Netclaw.Actors/Tools), so background_job and file_read can share the same bounded, allocation-flat capture instead of copy-pasting the ring math (the #1293 review's altitude finding). - DrainToWindowAsync(reader, budget, ct): the head+tail window, unchanged behavior; now takes a CancellationToken (ShellTool passes None to preserve the pipe-drain semantics; file readers can pass a real token). - DrainCaptureAsync(reader, captureMax, inlineBudget, ct): retains a capture body for the spill plus a smaller inline window from one bounded pass. - Window(string, budget): pure head+tail of an in-memory string. The reader is a pure leaf — no redaction, no file IO — so callers own those. ShellTool and the benchmark now call BoundedOutputReader; the drain tests move to BoundedOutputReaderTests with added Window/DrainCapture coverage. No behavior change to shell_execute. 40 reader + shell tests pass; slopwatch clean. * harden(tools): tighten DrainCaptureAsync API per review (task group 1) Code review of the new (not-yet-wired) DrainCaptureAsync/Window surface surfaced sharp edges before task 4 wires them: - captureMax <= 0 inherited the window's "0 disables the cap" opt-out, which would ReadToEndAsync the whole stream and defeat the bounded-memory guarantee. The capture path must stay bounded, so a non-positive ceiling now throws ArgumentOutOfRangeException. - The result carried only CeilingExceeded (captureMax), giving no signal that the inline window dropped data when output sat between inlineBudget and captureMax — a caller could fail to spill. Add an explicit Truncated flag (output exceeded inlineBudget = spill warranted). - Deriving the inline window from the already-windowed capture is only correct when inlineBudget <= captureMax; a larger inline budget sliced through the capture's own separator and produced a mangled double-ellipsis. Clamp inlineBudget to captureMax. - Document that Window's budget bounds content, not string length (a truncated result is budget + separator chars). Also fix a stale <see cref> in the benchmark pointing at the moved ShellTool.BoundedDrainAsync. Tests cover the throw, the clamp, and the Truncated-vs-CeilingExceeded distinction; 14 reader tests pass, slopwatch clean. * feat(tools): plumb ToolCallId and inline budget into ToolExecutionContext (task group 2) Tools that bound their own output to the inline budget N and spill the rest need two things at the capture layer: the inline budget (so they bound to the same N the pipeline enforces, instead of emitting a larger result the pipeline re-truncates) and a per-call id (to name the spill file). Add both to ToolExecutionContext (the seam already carrying session-scoped state): ToolCallId (the existing typed value object, nullable) and MaxInlineToolResultChars. BuildToolExecutionContext is per-call, so it sets ToolCallId from tc.CallId and threads N through. Both are additive and default to null/0 for direct-construction and Empty contexts. No behavior change yet — task 3/4 consume these. Builds clean across Actors and Daemon; slopwatch clean. * feat(tools): add ToolOutputSpill, drop redundant DrainCaptureAsync (task group 3) ToolOutputSpill.RenderAsync turns a bounded capture into the model-facing result: it redacts the whole bounded buffer once (so multi-line secrets like PRIVATE KEY blocks survive, per design D5), windows the inline result from the *redacted* text, and — when the output exceeds the inline budget N — writes the full redacted output to {sessionDir}/tool-calls/{toolCallId}.log and appends a steer pointing the model at file_read (offset/limit) / grep. The provider- supplied call id is sanitized before use as a filename so a spill can never escape the tool-calls directory. Spill writes are best-effort: a failed on-disk copy degrades to inline-only rather than failing the tool call. Implementing this revealed DrainCaptureAsync (added in group 1) was the wrong shape and redundant: the inline window must be derived from the redacted capture, not the raw one, so the real flow is DrainToWindowAsync → redact → Window → spill. DrainToWindow + Window + ToolOutputSpill cover it, so DrainCaptureAsync and its tests are removed (it had no production caller — the exact dead-on-arrival API the group-1 review warned about). Tests cover redact-on-disk, the steer, the ceiling note, no-session degrade, and path-traversal containment. 15 reader+spill tests pass; slopwatch clean. * feat(tools): shell capture+spill on a unified inline budget (task groups 4 + 7) Wire shell_execute into the bounded-output mechanism and unify the two truncation stages that the #1293 review showed were fighting each other. shell_execute (group 4): drain stdout and stderr each to the capture ceiling, assemble the raw combined output under ONE shared budget (no per-stream doubling), and hand it to ToolOutputSpill — which redacts the whole bounded buffer once, returns an N-char head+tail inline view, and spills the full redacted output to {sessionDir}/tool-calls/{callId}.log with a steer when it exceeds N. ShellTool's own per-stream redaction and "[stdout/stderr truncated]" markers are gone (redaction now lives in RenderAsync). Config unification (group 7): - MaxInlineToolResultChars default 12000 -> 2000: the single inline budget N. - MaxOutputChars default 32000 -> 256000, repurposed as the capture ceiling (the spill body bound), not the inline cap. - ClampToolResult is now head+tail (reuses BoundedOutputReader.Window) instead of head-only, so the end of a result survives. It demotes to a safety net for results that don't flow through the shared reader (MCP, in-process tools); shared-reader tools already fit under N. - Schema MaxOutputChars default updated to match. The head-only-clamp-discards-the-tail tension from the review is resolved: one budget N, head+tail everywhere, full output one file_read away. 2194 actor + 363 config tests pass; slopwatch clean. * fix(tools): bound background_job and file_read capture (task groups 5 + 6) Close the two "second door" OOMs the #1293 review surfaced. background_job (#1300): BackgroundJobExecutionActor drained both pipes with ReadToEndAsync and held the full output as a managed string before trimming — the same unbounded shape #1293 fixed for shell_execute, on the path that exists *for* long chatty jobs. Drain each stream via BoundedOutputReader.DrainToWindowAsync at a capture ceiling (MaxCapturedOutputChars = 256000) instead; memory is now bounded and the log holds a head+tail view for floods larger than the ceiling. Redaction and the log write are unchanged — they now operate on the bounded output. file_read (#1301): the default (no offset/limit) path did File.ReadAllTextAsync of the whole file then truncated post-hoc — O(file size) memory — and never redacted. Replace it with ReadBoundedHeadAsync, which reads up to the limit and stops (bounds memory AND I/O, no full materialization), and run the returned content (both the default and the offset/limit paths) through SecretOutputRedactor (redact-on-read — file_read had none). Over-limit reads steer to a ranged read / grep rather than copying the file to a spill; the file on disk is its own backing store. The bounded ReadLinesAsync offset/limit path is unchanged. Tests cover the bounded head read, the steer, and redaction on read. 2195 actor tests pass; slopwatch clean. * docs(skill)+bench: large-output guidance and capture benchmark (task group 8) netclaw-operations: add a "Large tool output" section telling the agent that tool output is bounded to the inline budget and the full output is spilled to a session file (shell) or the job log (background_job) — read a slice with file_read offset/limit or grep instead of re-running or cat-ing a huge file. Note the bounded job log. Bump skill version 2.8.9 -> 2.9.0. CaptureBenchmarks: exercise the production capture path (drain to the 256000-char ceiling + window to the 2000-char inline budget) and confirm allocation is O(ceiling) — flat at ~1255 KB for both 256K and 50M chars of source output, vs the unbounded growth before. * docs(openspec): flag eval cases + record validate/run config (task group 8) openspec validate passes. Flag the eval cases for the tool-output change: existing shell/file_read cases assert on (tool-called + keyword), not exact result content, so the N=2000 + spill change leaves them green; the genuinely new behavior — the agent reading a spill file / using offset-limit-grep on the steer instead of re-running — is a coverage gap to add. Record the spark2 (openai-compatible Qwen3.6-35B) run command. sync/archive on PR merge. * refactor(tools): move bound+spill to the dispatcher; per-tool inline budgets Max-effort review of the per-tool spill design surfaced that the spill was at the wrong altitude, plus a HIGH-severity gap. Rework it onto the one chokepoint every tool result already funnels through — DispatchingToolExecutor — which also already redacts centrally. What changes: - DispatchingToolExecutor now owns the uniform bound+spill: right after the central redact, it windows the result to the tool's inline budget and, when it overflows, spills the full redacted result to {sessionDir}/tool-calls/{callId}.log with a steer (file_read offset/limit | grep). Both the main session and sub-agents go through here, so sub-agent tool output is now bounded too — closing the gap where a sub-agent's shell/file_read dumped the entire capture into an unbounded history (review finding #1). - Per-tool inline budgets: INetclawTool.InlineOutputBudgetChars (default 0 = use the session content budget). shell_execute overrides to 2000 because its output is verbose noise the model skims; content tools (file_read, web_fetch, MCP, memory) keep the 12000 content budget because the model fetched them to read in full. Not a dispatcher special-case and not per-tool busywork — one default with an opt-in override, declared on the tool like GrantCategory. - MaxInlineToolResultChars restored to 12000 (the content default); shell's 2000 is its own override. (The earlier global 2000 would have forced an extra file_read round-trip for content the model just fetched.) - ClampToolResult retired — the dispatcher is the single truncation stage, so the double-clamp that re-windowed an already-windowed+steered result is gone (finding #2). - Tools shrink to "bound capture (OOM) + return raw": ShellTool drops its own redaction/spill and bounds the COMBINED stdout+stderr to MaxOutputChars (fixes the per-stream-vs-combined ceiling mismatch, #4); file_read drops the redundant redact-on-read (the dispatcher redacts — #1301's redaction half was already covered centrally) and keeps only its bounded read (the real #1301 fix). - Spill write decoupled from the request token (CancellationToken.None) so a late cancellation can't fail an already-completed tool call (#5); redaction now runs before the bound, so the redact-then-decide flip (#6) is moot. - background_job marks output that exceeded its capture ceiling (#7). - ToolExecutionContext.ToolCallId removed (the dispatcher uses toolCall.CallId). Tests: dispatcher-level end-to-end tests for verbose-budget spill, redact-before- spill, content no-spill, and small-output redaction; the FakeToolExecutor double mirrors the dispatcher's post-processing so integration tests still see bounding. 2196 actor tests pass; slopwatch clean. Eval suite (pre-refactor commit) showed all tool-output cases green. * fix(tools): drop "full" from the spill steer The spill is the captured output bounded by the capture ceiling — for a flood (e.g. cat of a huge file) it's a head+tail sample, not the complete output, so "full output saved to {path}" over-promised. "output saved to {path}" is honest in every case without threading a truncation flag through the generic dispatcher. The spilled content already carries its own "..." gap when it was a sample. * docs(openspec): sync artifacts to the as-built dispatcher design The implementation pivoted during review: bound+spill moved from the tools to DispatchingToolExecutor (the one chokepoint both main session and sub-agents pass through, and where central redaction already runs), per-tool inline budgets replaced the single global N, and ClampToolResult was retired. Update the artifacts to match: - design.md: rewrite the decisions around the dispatcher chokepoint, per-tool budgets (content default 12000 vs verbose override 2000), central redaction (no two-mode / no new abstraction), ClampToolResult retired, the "drop full" steer wording, and the rejected shell-AST file inference. - proposal.md: What Changes / Capabilities / Impact reflect the dispatcher mechanism and per-tool budgets; MaxInlineToolResultChars stays 12000. - specs: bounded-tool-output (central bound+spill, per-tool budget), netclaw-tools (shell returns raw + declares verbose budget; file_read bounded read, no own redaction), netclaw-session (ClampToolResult removed). Drop the tool-call-metadata delta (ToolCallId no longer on the context). - tasks.md: revision note (group 9) recording the pivot. openspec validate passes. * feat(tools): rename file_read Offset → StartLine for 1-based line addressing Offset+Limit invoked the SQL-pagination idiom (0-based skip count), so models read "line N" as Offset=N-1 and returned the adjacent line. The tool is line-addressed and already 1-based everywhere else — it prefixes output with 1-based line numbers and its continuation hint emits a 1-based number — so the parameter name was the only defect. Rename Offset → StartLine (1-based; no behavior change), and align the tool/param descriptions, the truncation and continuation steer text, the FileReadTool tests, and the netclaw-operations skill. No backward-compat alias: an unknown "offset" arg is ignored (reads from line 1) and self-corrects, rather than silently returning the wrong line. * test(evals): add bounded-output spill + ranged-read coverage (task group 8.3) Two uncoached Complex Task Execution cases close the eval gaps for the bound-tool-output change: the agent must retrieve a deep line from oversized shell output (daemon spill) and from a large pre-seeded file (file_read paging), driven only by AGENTS.md, the netclaw-operations skill, and the tool-result steer — no handling hints in the prompt. Assert on a deterministic-but-opaque Lehmer PRNG value so a correct answer implies correct handling (one read is bounded to ~N inline, so a deep line is only reachable by paging). Pre-seed the >256 KB file under the workspaces read-root in start_eval_daemon. Verified 5/5 on each against spark2 (Qwen3.6-35B). Records the file_read 1-based off-by-one finding that motivated the StartLine rename. Run: NETCLAW_EVAL_CATEGORY="Complex Task Execution" ./evals/run-evals.sh → 5/5 (100%). * fix(tools): platform newline for bounded-output separator; document MaxOutputChars Address PR #1305 review: BoundedOutputReader's truncation separator was a hardcoded "\n...\n"; use Environment.NewLine so it matches the line endings the rest of the captured output is assembled with (StringBuilder.AppendLine). const → static readonly since Environment.NewLine is a runtime value; the one test asserting the literal now uses Environment.NewLine too (was Linux-only-correct). Also add the missing schema description for ToolConfig.MaxOutputChars (value unchanged at 256000) clarifying it is the capture ceiling for the spill file, not the inline budget the model sees.
Aaronontheweb
added a commit
that referenced
this pull request
Jun 15, 2026
… kill timer, reap on passivation (#1405) * Background jobs as detached processes: stream logs live, no default kill timer, reap on passivation, Lost notifications A background job is now a detached process with no expectation of completion (OpenSpec: background-jobs-detached-process-redesign). Fixes the hung-session class where a dev server (jekyll serve / npm run dev) could never be used: both execution paths blocked on process exit. - Stream stdout/stderr to ~/.netclaw/jobs/{id}/output.log while the process runs (per-line secret redaction, 5MB single-slot rotation). The existing check_background_job tail query and file_read/grep monitoring now work mid-run; output survives daemon crashes. Completion tails read from disk. - Remove the silent default kill timer on background routing: omitted _timeout_seconds now means no timer (was: synchronous default, killing un-hinted jobs early). Submit ACK includes the output log path. - Reap on session passivation: KillJobsForSession handshake before the final snapshot; new Reaped status (distinct from Cancelled); no turn delivery on reap (would rehydrate the session being torn down); reaped entries surface exactly once in [active-background-jobs] on rehydration, then prune. - Wire up session-side job tracking (TrackBackgroundJob had no production caller — the active-jobs context block was always empty). - Daemon-restart reconciliation now delivers Lost notifications to owning sessions with the pre-crash log path. - Remove the vestigial pending-approval passivation deferral: approvals are journaled and the response path already rehydrates and resumes. - AGENTS.md template, netclaw-operations SKILL.md (v2.13.0), and the background-jobs runbook document the new lifecycle; eval suite gains a background-job lifecycle regression case. * Fix background-job lifecycle eval: multi-turn harness, pre-trusted verb, tightened assertion The new tool_background_job_lifecycle case scored 0/5 for instrumentation reasons, not model behavior (per the eval-debugging guidance): 1. run_case treats multiple prompts as alternate phrasings (pick_variant) — sequential conversations need run_multi_turn_case, which resumes one session and accumulates stdout across turns for the assertion. 2. Even then, every background submission died at the approval gate: the headless eval container has no approval requester and 'sleep' is not on the safe-command allowlist. Passing runs were vacuous (the model probed check_background_job with a made-up ID while flailing). The eval setup now pre-trusts the sleep verb via 'netclaw approvals trust-verb' against the bind-mounted tool-approvals.json before the container starts, so the case exercises the real lifecycle: submit -> job id -> status -> cancel. 3. The assertion now requires the actual _background":true submission, not just any shell_execute call. Result: 5/5, with transcripts showing the genuine flow (job id returned, ACK steering to the streaming log path, live status with elapsed time, cancel confirmed). * Fix CI: SW003 empty-catch marker, parallel-test isolation for real-process job tests Two PR CI failures: 1. Slopwatch SW003 — the write-failure path in JobOutputLog had an empty inner catch with the rationale as a body comment instead of the repo's 'catch // slopwatch-ignore: SW003 <reason>' marker convention. (Passed locally because slopwatch 0.4.1 only scans the git diff vs local HEAD; CI's PR-merge scans the whole new file.) 2. Test-ubuntu-latest flake — KillJobsForSession_ReapsOwnedJobs and BackgroundJob_Completes_And_DeliversResult_ViaGateway intermittently failed with the owning manager's freshly-created jobs showing 'Lost'. Root cause (reproduced reliably by running the Jobs test classes together): under heavy parallel load, concurrent process/FS pressure makes a manager's message handler throw transiently, the actor restarts, and startup reconciliation correctly marks its in-flight jobs Lost — a spurious restart to induce in a unit test. Fix: serialize the three real-process-spawning job test classes via a DisableParallelization collection (repo's established pattern) so they don't mutually starve. Verified: full assembly 4/4 green, the prior ~Jobs repro 3/3 green. Also register TimeProvider in LlmSessionTestBase to mirror production DI (Daemon Program.cs) — WithNetclawActors() constructs the background-job and reminder managers via the DI resolver, which need it; without it they died with ActorInitializationException at startup, adding restart churn. * Address code-review findings on background-jobs feature Resolves the 11 findings from the /code-review pass: #1 Multi-line secret redaction: per-line redaction in JobOutputLog misses secrets spanning lines (e.g. PEM blocks). Re-redact the assembled tail at every LLM-surface point (execution-actor completion, manager HandleQuery, NotifyLostJob) so multi-line secrets can't reach the model. #2 Journaled reap event (SessionBackgroundJobsReaped): reap marks were snapshot-only and lost on recovery when the passivation snapshot is skipped (parked approval), rehydrating killed jobs as 'running'. FinishJobReap now persists the reap; recovery replays it. Full serializer plumbing + round-trip test. #3 Dispose the Process in BackgroundJobExecutionActor.PostStop — stops the kernel handle / wait-handle leak (amplified by the no-default-timeout). #4 Audience-gate the [active-background-jobs] block (commands, rationales, and the output-log path) for Public, matching WorkingContext. #5 JobOutputLog.ReadTail falls back to the rotated .1 file when the current log is momentarily absent mid-rotation, instead of returning an empty tail. #6 A transient File.Move failure in Rotate() is non-fatal: capture continues on the current log and retries next threshold, rather than permanently going silent. #7 Back WriteFailure with a volatile field (un-gated fast-path read crosses threads). #8 Correlate reap Ask replies with an epoch so a late reply from a superseded passivation can't resolve a newer handshake. #10 Centralize the reap-reply handler (CommandJobReapResolved) across all non-terminal phases so a future phase can't silently drop the reply. #11 Apply(TurnRecorded) now delegates job dedup/prune to the single shared CompleteTurnBackgroundJobBookkeeping helper so replay and live paths can't drift. #9 AutoFlush is kept (live monitoring requires per-line visibility; a write() to the page cache is cheap and a time-throttle risks an unflushed quiescent ready-line) — documented as a deliberate decision. Tests: +6 (reaped-event round-trip, ReadTail rotation fallback + rethrow, SessionBackgroundJobsReaped apply, Public/Personal active-jobs gating); updated RotationFailure test to the new non-fatal contract. Full Actors suite 2412 green x2; slopwatch + headers clean. * Fix racy ReminderManagerActorTests.Startup_emits_alert_for_legacy_reminder_missing_trust_fields Root cause (per akka-net + dotnet-concurrency analysis): the legacy-schema alert is emitted synchronously inside the actor's PreStart, and the test waited for it with a fixed 5s AwaitAssertAsync poll. Under heavy parallel CI load the shared ThreadPool is saturated (many TestKit ActorSystems, WithSerializationVerification overhead), so the actor's PreStart can be scheduled later than the 5s budget and the poll gives up with an empty sink. Not a logic/visibility bug — the sink is lock-guarded and the store records the rejection synchronously in its constructor. Fix: await a deterministic readiness signal instead of polling a wall clock. An actor processes mailbox messages only after PreStart completes, so a successful Ask<ReminderHealthResponse>(GetReminderHealthQuery) reply guarantees the emit has run. This is the same readiness pattern already used elsewhere in this test file; the generous Ask timeout absorbs scheduling latency and returns as soon as the actor is ready (no wasted time in the common case). No existing GitHub issue covers this test. Does not reproduce locally even at full-assembly parallelism (CI-runner-only starvation).
Aaronontheweb
added a commit
that referenced
this pull request
Jun 30, 2026
…p-accurate roll Code-review #1 (the dominant finding) + #3, per the daemon.log decision. The field-based router diverted ANY line carrying a {SessionId} into that session's session.log — including daemon-infrastructure errors that merely NAME a session (gateway "failed to mark session X active", drain "failed to drain session X before shutdown", restart-recovery). Those are daemon functionality, and the class contract promises them in daemon.log. On dev they went there because the old AsyncLocal was only set inside session-serving code. Restore that structurally: a state SessionId routes only when it rode in on an Akka actor context (the bridge's AkkaLogState carries a LogSource) or a BeginScope (chat-client decorators, spawn breadcrumbs). A bare {SessionId} message field on a plain daemon-service ILogger<T> has no LogSource → stays in daemon.log. No hand-maintained category list. Swept all {SessionId} message templates: only daemon/channel/infra lines lack an actor context (consistent with dev); the one session-serving exception (tool-denial) was already moved to a scope. Also: daemon.log size-roll flushes within a batch of the 10MB cap so AutoFlush=off buffered bytes can't overshoot it (#3). Tests updated to model actor-context vs message-field lines, plus a new case locking in the daemon-service → daemon.log behavior. Skill bumped once for the PR (2.22.0).
Aaronontheweb
added a commit
that referenced
this pull request
Jul 1, 2026
…per session, daemon.log sparse, OTEL the union (#1472) (#1499) * refactor(logging): route session logs off the log event, delete the AsyncLocal (#1472 step 6) Actor ILoggingAdapter lifecycle lines now reach per-session session.log reliably, and the SessionDiagnosticsContext AsyncLocal threading is gone. The session.log sink (RollingFileLoggerProvider) reads the session id off each log event's structured state instead of an ambient AsyncLocal. The Akka->MEL bridge passes the event's properties as the MEL state (AkkaLogState, carrying the actor's WithContext("SessionId", ...) tag), and MEL's own structured logging passes FormattedLogValues (a {SessionId} field). RollingFileLogger.Log reads "SessionId" from either and Enqueue(message, sessionId) routes to the existing per-session writer (SessionLogDiagnostic -> SessionLogDispatcher -> SessionLogActor). Because the id rides on the line, routing no longer depends on thread/async context — a line logged after an await still routes, which the AsyncLocal could not. Deleted (the context-threading workaround): - SessionDiagnosticsContext (the AsyncLocal) + its 9 Push() scope sites. - SessionLoggingScope (the MEL BeginScope helper) + its 3 chat-client call sites. - Obsolete tests/helpers: SidecarDiagnosticsContextTests, ScopeCapturingLogger, the SessionDiagnosticsContext test. NormalizeSessionId relocated to SubAgentSessionScope. Kept session-routed by carrying the id on the line: - The #1468 spawn breadcrumbs (SubAgentSpawner/SpawnAgentTool) now include a {SessionId} field, so the sink routes them without a scope. Behavior change: chat-client / LLM-pipeline INTERNAL diagnostics (retry, failover, request/response internals) were only ever session-tagged via the AsyncLocal — already lost after the first await — so they settle into daemon.log only. Each actor's own lifecycle log still reaches session.log via the sink, and OTLP/Seq correlation for actor logs is unaffected (it rides the same WithContext tag). Net -350 LOC. The bridge behavior (AkkaLogState carrying the WithContext properties) was verified by decompiling Akka.Hosting 1.5.69; provider-level + async-continuation tests cover the read path. Skill diagnostics guidance updated (netclaw-operations 2.19.0). Part of #1472. * test(logging): end-to-end proof actor _log routes to session.log via the real bridge (#1472 step 6) A hand-built host (AddLogging(AddProvider) + AddAkka(ConfigureLoggers(AddLoggerFactory)) — production wiring) drives an actor's WithContext("SessionId", ...) log through the real Akka->MEL bridge into RollingFileLoggerProvider and asserts a SessionLogDiagnostic is routed for that session. Proves the essential goal through the live bridge, not a mocked state. Not Akka.Hosting.TestKit on purpose: its bridge (TestKitLoggerFactoryLogger : the production LoggerFactoryLogger) is hardwired to the TestKit's own xUnit test-output ILoggerFactory, so a TestKit-based test cannot observe the bridged log arriving at our provider. Relates to #1499, #1472. * refactor(logging): publish session logs explicitly instead of routing in the sink A max-effort review showed the previous step-6 approach (the file sink reading a "SessionId" property off each log event and routing it to session.log) was wrong: "{SessionId}" is a descriptive message-template placeholder in 86 places across the codebase (binding actors, gateway, webhooks, session catalog/registry, drain/recovery), each of which produces a structured "SessionId" property — so the sink would flood session.log with operational noise and create a self-feedback loop in SessionLogActor's write-failure path. SessionId is a correlation field, not a routing field. Replace sink-side routing with explicit publish through the existing actor-message contract (the same one that already carries the transcript): - RollingFileLoggerProvider reverts to a daemon.log-only writer — no ExtractSessionId, no dispatcher attach, no buffer. (Deletes the collision, the feedback loop, the per-line boxing, the dead branch, and the TOCTOU buffer race the review found.) - ToolExecutionContext.EmitSessionLogLine: LlmSessionActor wires it to _logActor.Tell(new SessionLogDiagnostic(parentSessionId, line)); the tool pipeline threads it to the spawn breadcrumbs, which publish their lifecycle explicitly. Using the parent id also fixes the composite-id normalization gap. - SessionId stays a pure Seq/OTLP correlation field on those lines, doing one job. - Removed the dead sessionId param on SubAgentActor.InvokeLlmAsync; fixed the stale LoggingChatClient doc; skill diagnostics guidance updated to the explicit-publish model. session.log scope = transcript + explicitly-published spawn lifecycle. Chat-client and sidecar internals are daemon.log-only (filter by the SessionId field). Sidecar diagnostic restore is a follow-up. Relates to #1499, #1472. * fix(logging): publish routed-skill sub-agent spawn lifecycle to session.log (#1472 step 6) The routed-skill spawn path (LlmSessionActor.ExecuteRoutedSkillAsync) built its own ToolExecutionContext and wired SpawnChildActor + OnSubAgentActivity but never set EmitSessionLogLine, so skill-routed sub-agent spawns — and critically their failure breadcrumbs — were invisible in session.log. That is the original #1467 gap still present on one of the three spawn paths. Wire EmitSessionLogLine to the session-log dispatcher, mirroring the tool-execution dispatch path, and fix the stale SessionLogDiagnostic doc comment that still described the deleted MEL-sink-routing model. Test: Routed_slash_command_publishes_spawn_lifecycle_to_session_log (verified failing without the fix). * feat(logging): correlate LLM chat-client diagnostics to sessions via explicit ChatOptions Deleting the SessionDiagnosticsContext AsyncLocal (#1472 step 6) removed the only way the session-agnostic chat-client decorators (logging / retry / routing) knew which session a call belonged to. LLM timing, retries, and — most importantly — provider failover/outage events lost all session correlation: gone from session.log (intentional) and carrying no SessionId field in Seq either. Restore the Seq/OTLP correlation explicitly, without resurrecting ambient state that does not flow across the actor mailbox boundary: - SessionScopedChatOptions carries the owning session id on the call's ChatOptions as a typed property — NOT in AdditionalProperties, which the self-hosted provider forwards verbatim onto the wire (a dictionary entry would leak the id into the LLM request body). LlmSessionActor and SubAgentActor (parent id) set it on every model call; sidecar calls stay session-agnostic. - ChatClientSessionScope re-opens the SessionId logging scope in all three decorators, sourced from the options. IncludeScopes surfaces it as a filterable Seq field; session.log stays narrow (no re-flood). Tests: decorator scope attach + negatives, a wire-leak guard proving the id never reaches the provider request body, and builder-side coverage that the session and sub-agent actors emit the carrier with the right id. Updates the netclaw-operations diagnostics skill (2.20.0) to state precisely where each line type is filterable (inline grep vs Seq scope). * refactor(logging): address code-review findings on session-log correlation Follow-up to the two preceding commits, from a multi-agent review pass: - Dedupe the explicit session-log emitter: the routed-skill path and the tool-execution dispatch path built the identical logActor?.Tell(SessionLogDiagnostic(...)) closure. Collapse both onto one private CreateSessionLogEmitter() so their timestamp format and routing cannot drift. - Drop a now-dead null-conditional (options?.Tools) — options is unconditionally non-null since it became the SessionScopedChatOptions carrier. - Cover the production (streaming) path: the chat-client scope tests only exercised the non-streaming GetResponseAsync branch, but StreamingResponseReader only ever calls GetStreamingResponseAsync. Add streaming retry-backoff and provider-failover scope tests, plus a composed-pipeline test proving the SessionScopedChatOptions subclass survives Logging->Retry by reference (guards the correlation against a future cloning decorator). - Correct the diagnostics skill: session id is reliably filterable in Seq for both actor and decorator lines; daemon.log *text* grep only catches lines that template the id inline. The prior wording over-claimed inline greppability for context-enriched actor logs. * refactor(logging): centralize the "SessionId"/"SubSessionId" log-attribute keys The structured-logging key "SessionId" was hardcoded as a raw literal at ~9 WithContext sites across the actor system and every channel adapter, plus two private SessionIdKey consts in the chat-client scope helper and its test double. A rename to align OTLP attributes would have had to find every copy, and the producer could silently drift from the test assertion. Introduce Netclaw.Actors.Protocol.NetclawLogProperties (SessionId, SubSessionId) — Netclaw.Actors is already referenced by every consumer (Daemon, all Channels.*, Daemon.Tests), so no new project references — and route all logging-key usages through it. Type references, proto field names, DTO/JSON keys, and doc-comment prose are untouched; only WithContext/BeginScope keys changed. * refactor(logging): drop RetryingChatClient's redundant SessionId scope Code-review follow-up. In the composed pipeline RetryingChatClient always runs inside LoggingChatClient's streaming scope (guarded by Compose_puts_Logging_ outermost), which stays open for the whole enumeration that drives the retry loop — so BackoffAsync re-opening an identical SessionId scope was pure duplication. Drop it and document the inherited-scope contract; only RoutingChatClient, which logs outside any LoggingChatClient scope, keeps its own. Move the retry-warning correlation coverage from two isolated-decorator tests (which asserted the now-removed self-scope) to one composed-pipeline test that fires a real retry and asserts, at the message level, that the warning line is emitted while the SessionId scope is active — i.e. it tests the production wiring (LoggingChatClient enclosing Retry) rather than a decorator in isolation. * refactor(logging): collapse sub-agent spawn breadcrumbs to one fan-out emitter Every spawn-lifecycle breadcrumb hand-drove two sinks at the call site — a structured _logger call for daemon.log/Seq and a parallel EmitSessionLogLine string for the session.log transcript — with two message strings to keep in sync. The sinks are genuinely different artifacts (operational diagnostics vs the per-session audit transcript; unifying them at the sink was tried and reverted), but that seam belongs behind one emit point, not duplicated at every site. Add SubAgentSpawnBreadcrumbs: one method per lifecycle event that fans out to both sinks from a single source of truth. SubAgentSpawner (7 sites) and SpawnAgentTool (2 refusal sites) now state each event once; the two renderings live together and can't drift. Message text is byte-for-byte preserved, so the existing SubAgentSpawnObservabilityTests and the routed-slash session-log test cover it unchanged. * feat(logging): partition the log stream by session instead of explicit-publish Replace the bolted-on EmitSessionLogLine side-channel with a single partitioning logging sink. The routing rule is now simply "does this line carry a session id?": - RollingFileLoggerProvider implements ISupportExternalScope and, per line, resolves the session id from the event's structured state (an actor's WithContext("SessionId", ...) bridged by Akka->MEL, or a {SessionId} message field) or from the active scopes (the chat-client decorators' BeginScope). A session-tagged line is Tell'd to the SessionLogDispatcher as a SessionLogDiagnostic and is NOT written to daemon.log; everything else goes to daemon.log. The full stream still exports to OTEL with SessionId as an attribute — the receiver does the global slicing. - The session-log writer's own lines (identified by its Akka LogSource/ActorPath) are forced to daemon.log so a write-failure log can never recurse into the file that just failed. - session.log is now the session's full local slice (transcript + every session-scoped operational line), so SessionLogActor switches from per-line AutoFlush to a batched/idle flush (~1s or 256 writes). FlushTick is INotInfluenceReceiveTimeout so the cadence doesn't keep an idle session alive. Deletes ToolExecutionContext.EmitSessionLogLine, LlmSessionActor.CreateSessionLogEmitter, and the pipeline threading. SubAgentSpawnBreadcrumbs now logs each event under a SessionId scope (one call, no fan-out); because routing is no longer per-path-wired, the original routed-skill gap (commit 9551067) is structurally impossible. Retains the chat-client SessionId-scope work, which is now what routes LLM-pipeline lines. Restores the SessionLogDispatcherWiringService IHostedService to attach the dispatcher post-start. Tests: RollingFileLoggerPartitionTests (routing, scope, feedback carve-out, pre-attach buffering); SubAgentSpawnObservabilityTests reworked to assert the SessionId scope; diagnostics skill updated to 2.21.0. * test(logging): end-to-end partition proof through the real Akka->MEL bridge The unit tests simulate the log-event state shape; this drives the real thing. A test actor tags its logger with WithContext("SessionId", ...) and logs; the line flows through ConfigureLoggers(AddLoggerFactory) -> the host ILoggerFactory -> RollingFileLoggerProvider, which extracts the id from the live AkkaLogState and routes it to the real SessionLogDispatcher/SessionLogActor. Asserts the line lands in that session's session.log file and NOT in daemon.log — confirming the bridge produces the structured state the provider's ScanState reads. * fix(logging): harden the session-partition sink (code-review follow-up) xhigh review fixes on the partition rework: - Concurrency in RollingFileLoggerProvider's pre-resolution buffer had a null-deref (NRE thrown into a logging call) and a lost-line race against dispatcher resolution. Replaced the Interlocked/ConcurrentQueue dance with a lock-guarded slow path; the steady state (dispatcher resolved) stays lock-free via a Volatile read. On resolution FAILURE the buffered lines now drain to daemon.log instead of being dropped, and the one-shot failure beacon has a stderr fallback if the daemon queue is saturated. - SessionLogActor flush failure: Flush() now resets _unflushedWrites and rate-limits the warning to onset+recovery, so a persistent disk error no longer floods daemon.log every tick. Its own self-logs use a {Session} key (not {SessionId}) so they can't arm routing back into the file that just failed — defense-in-depth on the feedback carve-out. - FindSessionIdInScopes: static lambda + StrongBox state to drop the per-line closure allocation on the chat-client hot path. - Docs: fixed the LoggingChatClient comment (its lines DO route to session.log now) and the diagnostics runbook (a daemon-wide provider outage surfaces as the sessionless provider.unreachable alert in daemon.log; per-call failover detail is in each session's session.log). Test: Dispatcher_resolution_failure_drains_buffer_and_beacons_to_daemon_log. * docs(logging): document the dormant non-streaming retry session-scope gap RetryingChatClient inherits its SessionId scope from the enclosing LoggingChatClient, which instruments only the streaming path. The non-streaming GetResponseAsync retry path therefore opens no scope — dormant today (Netclaw issues only streaming requests), but if a non-streaming path is ever added the retry warnings would lose session correlation and route to daemon.log. Leave a caveat at BackoffAsync pointing the future fix at ChatClientSessionScope.Begin / LoggingChatClient.GetResponseAsync. * refactor(logging): drop the startup buffer + lock for a single volatile dispatcher ref The pre-resolution buffer (and the lock guarding it, where the review found a null-deref and a lost-line race) only existed to route session-tagged lines emitted in the window between AttachSessionDispatcher and the dispatcher resolving. But the things that log with a SessionId — session actors, chat-client calls — don't exist until a session starts processing a turn, well after startup, so that window carries no session traffic. Collapse it to one volatile _sessionDispatcher reference that IRequiredActor.GetAsync resolves in the background; a line that finds it still null (startup window or resolution failure) falls back to daemon.log, which is already the documented routing-off behavior. Removes _pendingDiagnostics, the _routeGate lock, _routingFailed, _sessionRoutingEnabled, and the whole concurrency surface the review flagged (~60 lines). Resolution stays in the background — not a blocking await in the wiring service's StartAsync, which runs before Akka and would deadlock. Tests updated: a pre-resolution session line now falls back to daemon.log rather than buffer-and-drain. * fix(logging): breadcrumb emitters take SubAgentRunId after the merged run-id refactor The merged SubAgentRunId/SubAgentScopeId refactor changed SubAgentSpawner's runId from a string to a SubAgentRunId value object, but the spawn breadcrumb emitters still took `string runId` — a CI compile failure (CS1503). Take SubAgentRunId and log runId.Value. * feat(logging): give each sub-agent run its own session.log, keyed by SubSessionId Per the design call: a parent investigating a sub-agent reviews that run's own log, and the parent's session.log stays clean (transcript + spawn breadcrumbs only). - RollingFileLoggerProvider reads SubSessionId (from log state or scope) and partitions the LOCAL file by SubSessionId when present, else SessionId. The line still carries the parent SessionId, so OTEL groups sub-agent lines under the parent AND slices by SubSessionId. - SessionScopedChatOptions gains SubSessionId; ChatClientSessionScope emits it in the scope; SubAgentActor sets it (its scopeId {parent}/subagent/{name}/{runId}) on the LLM-call options. The sub-agent's own actor logs already carried both ids. - Spawn breadcrumbs stay in the PARENT's log (parent context, no SubSessionId) as the pointer. Tests: provider routes a state- or scope-carried SubSessionId line to the sub-run file (not the parent's). diagnostics skill updated (2.22.0). * fix(logging): restore session correlation for sidecar LLM paths (code-review) The AsyncLocal removal stripped SessionDiagnosticsContext from five session-owned sidecar chat calls but never gave them the replacement SessionScopedChatOptions carrier, so their chat-client diagnostics (timing, retries, provider failover) silently regressed to daemon.log uncorrelated — and the #920 guard test was deleted in the same commit. Restore correlation by threading SessionScopedChatOptions through all five (each already had the session id in scope): SessionTitleGenerator, SessionCompactionPipeline observer, SessionMemoryObserverActor distillation, MemoryCurationActor, and LlmSessionActor memory-extraction. Replace the deleted guard with SidecarSessionCorrelationTests (one case per sidecar, asserting the carrier). The diagnostics doc's claim that session.log holds the session's LLM-pipeline/memory lines is now true rather than aspirational. Also from the review: - Harden RollingFileLoggerProvider.Route: only consult scopes when there is no state session id, so an ambient SubSessionId scope cannot hijack an actor line's routing. - SessionLogActor.Flush: keep retrying on the 1s tick while failing, so buffered audit bytes are not stranded through an idle period after the disk recovers. - Correct SubAgentSessionScope doc: it claimed sub-agents reuse the parent's log and were the "only place" the /subagent/ split happens — both now false (own file; ToolApprovalActor too). * feat(logging): flush the audit transcript immediately, keep diagnostics batched Per the durability decision: SessionLogActor has two inputs — the audit transcript (user/assistant/tool/usage via SendUserMessage/SessionOutput, low volume, durability- critical) and the high-volume diagnostic lines (SessionLogDiagnostic). The audit handlers now WriteDurable (write + immediate flush) so a hard process death (SIGKILL/OOM) cannot drop the audit record's tail, while diagnostics stay on the batched Write() path (an fsync per line would dominate now that the whole per-session stream lands here). An audit line also flushes any diagnostics buffered before it, so audit lines are natural flush points. * refactor(logging): batch daemon.log flushes + consolidate the /subagent/ split (code-review) Two minor code-review follow-ups: - daemon.log writer no longer fsyncs per line. It flushes when the queue drains (so sparse daemon.log lines — startup, config, lifecycle, alerts — stay immediately durable) or after a 256-line burst cap, matching the cadence the sibling SessionLogActor already uses. The tail is flushed by Dispose() when the writer thread exits. - ToolApprovalActor's approval-inheritance walk now calls SubAgentSessionScope.NormalizeSessionId instead of re-implementing the "/subagent/" split inline, so there is a single owner of that parse (behavior preserved — IndexOf-first already walked leaf -> root parent). Doc updated. * fix(logging): guard PostStop dispose, fix audit-durability doc, scope the tool-denial line Code-review follow-ups (clear-cut findings): - SessionLogActor.PostStop: guard _writer.Dispose() in try/catch. On a disk-full stop the explicit Flush() already reports via its rate-limited warning; the Dispose flush could throw IOException out of PostStop and bury the real cause under Akka's PostStop-failed noise. - diagnostics skill: the flush description claimed session.log is uniformly batched (~1s lag). It now documents the split — the conversation audit is flushed immediately (durable), only the diagnostics batch. (skill 2.23.0; System Skills Sync Rule.) - SubAgentSpawner tool-denial line: route via SubAgentSpawnBreadcrumbs.ToolDenied (a SessionId scope) instead of a hand-rolled "(session={SessionId})" message field, so it can't drift from the centralized NetclawLogProperties key like every other breadcrumb. * fix(logging): keep daemon-infra session-named lines in daemon.log; cap-accurate roll Code-review #1 (the dominant finding) + #3, per the daemon.log decision. The field-based router diverted ANY line carrying a {SessionId} into that session's session.log — including daemon-infrastructure errors that merely NAME a session (gateway "failed to mark session X active", drain "failed to drain session X before shutdown", restart-recovery). Those are daemon functionality, and the class contract promises them in daemon.log. On dev they went there because the old AsyncLocal was only set inside session-serving code. Restore that structurally: a state SessionId routes only when it rode in on an Akka actor context (the bridge's AkkaLogState carries a LogSource) or a BeginScope (chat-client decorators, spawn breadcrumbs). A bare {SessionId} message field on a plain daemon-service ILogger<T> has no LogSource → stays in daemon.log. No hand-maintained category list. Swept all {SessionId} message templates: only daemon/channel/infra lines lack an actor context (consistent with dev); the one session-serving exception (tool-denial) was already moved to a scope. Also: daemon.log size-roll flushes within a batch of the 10MB cap so AutoFlush=off buffered bytes can't overshoot it (#3). Tests updated to model actor-context vs message-field lines, plus a new case locking in the daemon-service → daemon.log behavior. Skill bumped once for the PR (2.22.0). * test(logging): address code-quality bot findings (Path.Join, drain-loop comments) GitHub code-quality (Copilot) review follow-ups on this PR's test files: - Path.Combine -> Path.Join in the logging test temp-path helpers: the CodeQL "may drop earlier arguments" rule can't prove the second segment is non-rooted; Join is the correct idiom for plain concatenation and clears the recurring finding (identical result for these inputs). - Intent comments on the empty `await foreach (var _ in stream) { }` drain loops in the chat-client tests, so the enumerate-and-discard reads as intentional (it drives the pipeline's scope/retry/logging side effects, which the test then asserts). Declined the "use .Where()" suggestions: on the provider hot path (FindScopeIds) a LINQ Where allocates a per-line iterator/closure that the manual loop + static lambda deliberately avoids (documented); the mirroring test helpers stay foreach for consistency with it.
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.
Bumps dotnet-sdk from 10.0.102 to 10.0.103.
Release notes
Sourced from dotnet-sdk's releases.
Commits
2f84158[release/10.0.1xx] Source code updates from dotnet/dotnet (#52573)723b9d5Update dependenciese6a41ba[automated] Merge branch 'release/9.0.3xx' => 'release/10.0.1xx' (#52375)c08a02a[release/10.0.1xx] Update dependencies from microsoft/testfx (#52536)3a0defdUpdate dependencies from https://github.com/microsoft/testfx build 20260118.4171977b[release/10.0.1xx] Update dependencies from microsoft/testfx (#52521)6404a34[release/10.0.1xx] Bump analysislevel constants for .NET 10 release (#52511)dcff850Update dependencies from https://github.com/microsoft/testfx build 20260117.2a39c37bUpdate dependencies from https://github.com/microsoft/testfx build 20260116.27218b01[release/10.0.1xx] Update dependencies from microsoft/testfx (#52500)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)