diff --git a/src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs b/src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs index 848d0b5c8d..caf8979e2f 100644 --- a/src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs +++ b/src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs @@ -52,6 +52,9 @@ sealed class PiHookCommand( /// ~3.5s of work this leaves once is reserved. static readonly TimeSpan Ceiling = TimeSpan.FromSeconds(5); + /// Upper bound on the nudge claim's store work, further capped by what the hook budget has left. + static readonly TimeSpan NudgeClaimBudget = TimeSpan.FromMilliseconds(750); + public Task Handle(string[] args) => Handle(args, Console.Out); /// Injected so the fragment the extension consumes is assertable without @@ -176,11 +179,18 @@ HookBudget budget // an injection whose once-per-session lease is already spent. pi.exec hands the extension // stdout regardless of exit code, so no commit gate is needed (unlike Copilot). var fragment = await SessionStartMemoryHookSupport.AwaitBounded(memoryTask, budget); - var workItemsNudge = HarnessNudgeEmitter.Combine( - WorkItemsNudgeEmitter.Resolve(HarnessId.Pi, sessionId, activeProfile?.DisableWorkItemsNudge is true, harnesses, PlanEntitlementStore.Get(Url, config, clock.Time.GetUtcNow())), - PlansNudgeEmitter.Resolve(HarnessId.Pi, sessionId, activeProfile?.DisablePlansNudge is true, harnesses), - HarnessNudgeEmitter.ResolveFragmentForHook(activeProfile?.DisableHarnessNudge is true, config, harnesses, clock.Time), - FirstRunNoticeEmitter.Resolve(activeProfile?.DisableFirstRunNotice is true, config, HarnessId.Pi, harnesses)); + // Pi re-fires session_start for the same file on restart and resume, and the extension keeps + // what it is handed, so the nudges are gated by a durable once-per-session claim keyed on the + // file (resume deduped, fork eligible). Taken only now, after the POST: pi.exec discards all + // stdout from a hook it times out, so a claim taken earlier could be spent on output that + // never arrives. Same contract gate as the memory fetch. + var workItemsNudge = MemoryContractOf(args) >= 1 && await ClaimNudgesAsync(file, budget) + ? HarnessNudgeEmitter.Combine( + WorkItemsNudgeEmitter.Resolve(HarnessId.Pi, sessionId, activeProfile?.DisableWorkItemsNudge is true, harnesses, PlanEntitlementStore.Get(Url, config, clock.Time.GetUtcNow())), + PlansNudgeEmitter.Resolve(HarnessId.Pi, sessionId, activeProfile?.DisablePlansNudge is true, harnesses), + HarnessNudgeEmitter.ResolveFragmentForHook(activeProfile?.DisableHarnessNudge is true, config, harnesses, clock.Time), + FirstRunNoticeEmitter.Resolve(activeProfile?.DisableFirstRunNotice is true, config, HarnessId.Pi, harnesses)) + : null; await WriteMemoryFragment(stdout, fragment, workItemsNudge); if (!AgentHookPoster.ShouldSpawnAfter(outcome, Url)) return outcome == HookPostOutcome.Failed ? 1 : 0; @@ -192,6 +202,13 @@ await watchers.EnsureWatcherRunning(sessionId, file, return 0; } + /// False on a repeat, an unavailable store, or no budget left. A claim that runs out of + /// budget is left uncompleted, so it expires and a later start still delivers. + Task ClaimNudgesAsync(string file, HookBudget budget) => + budget.Remaining is { Ticks: > 0 } left + ? NudgeLease.TryClaimAsync(config, clock.Time, HarnessId.Pi, file, left < NudgeClaimBudget ? left : NudgeClaimBudget) + : Task.FromResult(false); + async Task HandleSessionEnd(string sessionId, string file, string? cwd, string? reason) { // Kill watcher + inline-drain BEFORE the POST so the server computes // stats over the full transcript — capped so a slow drain can't starve diff --git a/test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs b/test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs index e5559b3d83..cf92b7fce3 100644 --- a/test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs @@ -1,7 +1,9 @@ +using Microsoft.Extensions.Time.Testing; using Capacitor.Cli.Commands.Harness; using Capacitor.Cli.Commands; using Capacitor.Cli.SessionStartMemory; using Capacitor.Cli.Core.Harness; +using Capacitor.Cli.Core.Harness.Pi; using Capacitor.Cli.Core; using Capacitor.Cli.PrDetection; @@ -24,8 +26,8 @@ public class PiSessionStartMemoryTests { // cannot see, TUnit injecting it after construction. // The server URL is the resolution's, so a test proving the url guard fires hands in the bad one // here rather than as an argument. - PiHookCommand Hook(string serverUrl = "http://localhost:5100") => - new(Config.Root, Resolutions.At(serverUrl, Config.Root), new HookClock(TimeProvider.System), Home, TestHarnesses.Under(Home), HostedAgent.Terminal, new FixedCapacitorHttpClient(), TestWatchers.For(Config.Root, Resolutions.At(serverUrl, Config.Root), new FixedCapacitorHttpClient()), router: new GitProviderRouter(), workdir: new WorkingDirectory(AppContext.BaseDirectory)); + PiHookCommand Hook(string serverUrl = "http://localhost:5100", TimeProvider? time = null) => + new(Config.Root, Resolutions.At(serverUrl, Config.Root), new HookClock(time ?? TimeProvider.System), Home, TestHarnesses.Under(Home), HostedAgent.Terminal, new FixedCapacitorHttpClient(), TestWatchers.For(Config.Root, Resolutions.At(serverUrl, Config.Root), new FixedCapacitorHttpClient()), router: new GitProviderRouter(), workdir: new WorkingDirectory(AppContext.BaseDirectory)); static string Render(string? fragment) => PiHookCommand.RenderMemoryOutput(fragment); // Byte-identical to pre-feature behaviour on every no-index path (opt-out, failure, spent lease): @@ -125,4 +127,72 @@ await Assert.That(await Hook().StartMemoryIndexTask("/abs/file.jsonl", "/scope", await Assert.That(await Hook().StartMemoryIndexTask("/abs/file.jsonl", "/scope", disabled: false, guidelinesDisabled: false, TimeSpan.Zero, null)).IsNull(); } + + const string NudgeMarker = "declare_work_item"; + + /// A session file Pi would write, and the kcap-workitems bridge the nudge's availability + /// gate looks for, so the only thing that can suppress the nudge is the once-per-session claim. + string NewSessionFile() { + var bridge = TestHarnesses.Under(Home).Of().Paths.KcapMcpExtension; + Directory.CreateDirectory(Path.GetDirectoryName(bridge)!); + File.WriteAllText(bridge, """const KCAP_MCP_SERVERS = ["workitems"];"""); + + var id = Guid.NewGuid(); + var dir = Directory.CreateDirectory(Path.Combine(Home.Path, "pi-sessions")).FullName; + var file = Path.Combine(dir, $"2026-09-28T00-00-00-000Z_{id}.jsonl"); + File.WriteAllText(file, $$"""{"type":"session","id":"{{id}}","cwd":"{{dir.Replace("\\", "/")}}","timestamp":"2026-09-28T00:00:00.000Z"}""" + "\n"); + return file; + } + + /// The nudges need no server, so an unpostable URL keeps the lifecycle POST and the memory + /// fetch off the network, where each would otherwise spend its retry budget. + async Task SessionStart(string file, string reason, int memoryContract = 1, PiHookCommand? hook = null) { + var stdout = new StringWriter(); + await (hook ?? Hook("ftp://unreachable")).Handle([ + "--event", "session-start", "--file", file, "--cwd", Path.GetDirectoryName(file)!, + "--reason", reason, "--memory-contract", memoryContract.ToString(System.Globalization.CultureInfo.InvariantCulture) + ], stdout); + return stdout.ToString(); + } + + [Test] + public async Task A_resumed_session_is_not_nudged_again() { + var file = NewSessionFile(); + + await Assert.That(await SessionStart(file, "startup")).Contains(NudgeMarker); + await Assert.That(await SessionStart(file, "resume")).DoesNotContain(NudgeMarker); + await Assert.That(await SessionStart(file, "reload")).DoesNotContain(NudgeMarker); + } + + [Test] + public async Task A_forked_session_file_is_nudged_afresh() { + var original = NewSessionFile(); + await SessionStart(original, "startup"); + + await Assert.That(await SessionStart(NewSessionFile(), "fork")).Contains(NudgeMarker); + } + + /// An extension that does not read stdout would discard the nudge, so it must not spend + /// the claim the next, capturing, session_start needs. + [Test] + public async Task An_extension_that_discards_stdout_does_not_spend_the_claim() { + var file = NewSessionFile(); + + await Assert.That(await SessionStart(file, "startup", memoryContract: 0)).DoesNotContain(NudgeMarker); + await Assert.That(await SessionStart(file, "resume")).Contains(NudgeMarker); + } + + /// pi.exec discards everything a hook it timed out wrote, so a start that reaches the claim + /// with no budget left must leave it for the next start rather than spend it on output that may + /// never arrive. + [Test] + public async Task A_start_with_no_budget_left_leaves_the_claim_for_the_next_start() { + var file = NewSessionFile(); + var time = new FakeTimeProvider(DateTimeOffset.UtcNow); + var late = Hook("ftp://unreachable", time); + time.Advance(TimeSpan.FromSeconds(10)); + + await Assert.That(await SessionStart(file, "startup", hook: late)).DoesNotContain(NudgeMarker); + await Assert.That(await SessionStart(file, "resume")).Contains(NudgeMarker); + } }