Skip to content

[AI-2531] Nudge a Pi session once, not on every restart or resume - #1212

Closed
realtonyyoung wants to merge 2 commits into
mainfrom
tonyyoung/ai-2531-pi-nudge-once-per-session
Closed

realtonyyoung wants to merge 2 commits into
mainfrom
tonyyoung/ai-2531-pi-nudge-once-per-session

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

Closes #785 — AI-2531

What & why

Pi re-fires session_start for the same session file on every restart, reload and resume, and the kcap extension keeps whatever stdout hands it, so the SessionStart nudges (work items, plans, harness, first-run) piled up one copy per restart. They now sit behind NudgeLease, the durable once-per-session claim the Kiro fix introduced, keyed on the session file path so a resume is deduped and a fork (new file) is nudged afresh — the same identity the memory lease already uses.

Where to look

Unlike Kiro, the claim is awaited (bounded by the hook budget) at the output site rather than deferred to a second write: it starts beside the memory fetch and overlaps the lifecycle POST, so it has almost always resolved, and Pi's extension reads stdout as one fragment. The claim is only taken when the extension declares it reads stdout (--memory-contract >= 1), so an older kcap.ts cannot spend it on output it discards.

Verification

  • A_resumed_session_is_not_nudged_again (startup → resume → reload on one file) and An_extension_that_discards_stdout_does_not_spend_the_claim drive the real hook; both fail against the unfixed PiHookCommand and pass with it. A_forked_session_file_is_nudged_afresh covers the fork arm.
  • Neighbouring classes green locally: Pi* 77 (+2 skipped), *Nudge* 68, NudgeLeaseTests 6.

🤖 Generated with Claude Code

Pi re-fires session_start for the same file and keeps what the extension is handed, so the
nudges sit behind the durable once-per-session claim, keyed on the file like the memory lease.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Sep 28, 2026

Copy link
Copy Markdown

AI-2531

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Nudge Pi sessions once across restarts and resumes

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Prevent duplicate SessionStart nudges when Pi restarts, resumes, or reloads the same session file.
• Preserve fresh nudges for forks and avoid spending claims when the extension discards stdout.
• Add hook-level tests for repeated sessions, forks, and stdout-contract compatibility.
Diagram

graph TD
  E["Pi session start"] --> G{"Reads stdout?"} -->|yes| C["Durable claim"] -->|claimed| N["Resolve nudges"] --> O["Stdout output"]
  G -->|yes| M["Memory fetch"] --> O
  E --> P["Lifecycle POST"] --> O
  C -->|denied| O
  G -->|no| P
Loading
High-Level Assessment

Reusing the existing durable NudgeLease is the best fit: the session file distinguishes resumes from forks, while the stdout-contract gate protects older extensions. Reason-based gating cannot reliably identify repeated callbacks, and in-memory deduplication would not survive restarts.

Files changed (2) +89 / -5

Bug fix (1) +32 / -5
PiHookCommand.csGate Pi SessionStart nudges on a bounded, durable claim +32/-5

Gate Pi SessionStart nudges on a bounded, durable claim

• Starts a once-per-session-file nudge claim only when the extension declares it reads stdout. Awaits the claim within the hook's remaining budget before combining nudges with memory output, suppressing duplicates without blocking indefinitely.

src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs

Tests (1) +57 / -0
PiSessionStartMemoryTests.csExercise repeated-session and stdout-contract nudge behavior +57/-0

Exercise repeated-session and stdout-contract nudge behavior

• Adds hook-level tests showing that resume and reload do not repeat a nudge, a forked session file receives one, and an extension that discards stdout does not spend the claim.

test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs

@qodo-code-review

qodo-code-review Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Slow posts lose a session's nudges ✓ Resolved
Description
HandleSessionStart starts NudgeLease.TryClaimAsync before awaiting the lifecycle POST, but
writes stdout only after that POST returns. If the POST stalls past Pi’s ten-second execution
timeout, the extension discards the invocation after the lease has completed, and later starts for
the same file cannot deliver the nudge.
Code

src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs[R177-179]

+        var nudgeClaim = MemoryContractOf(args) >= 1
+            ? NudgeLease.TryClaimAsync(config, clock.Time, HarnessId.Pi, file, NudgeClaimBudget)
+            : Task.FromResult(false);
Relevance

●● Moderate

Budget-sensitive asynchronous work is usually bounded, but no close precedent confirms protecting
claims from a stalled lifecycle POST.

PR-#790
PR-#1030

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new claim starts before the awaited POST, while output is written afterward. The lease is
durably marked completed; the POST uses a default 30-second retry timeout, but Pi’s extension allows
only ten seconds and obtains stdout only from a completed execution.

src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs[177-198]
src/Capacitor.Cli/NudgeLease.cs[25-35]
src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryLeaseStore.cs[49-61]
src/Capacitor.Cli/Commands/AgentHookPoster.cs[119-139]
src/Capacitor.Cli.Core/HttpClientExtensions.cs[96-104]
src/Capacitor.Cli.Core/Harness/Pi/PiExtensionInstaller.cs[60-72]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Pi completes its once-per-session nudge claim before awaiting a lifecycle POST that can outlive the extension’s execution timeout. The extension then receives no stdout, while later starts find the claim spent.
## Fix Focus Areas
- src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs[177-198]
- src/Capacitor.Cli.Core/Harness/Pi/PiExtensionInstaller.cs[60-72]
## Recommended Fix
Overlap the lifecycle POST with the claim, deliver and flush the claimed nudge before waiting for a potentially slow POST, and bound remaining POST work so the hook finishes within Pi’s timeout. Preserve the existing spool and watcher outcome handling.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. A test heading repeats its assertion ✓ Resolved
Description
The new SessionStart heading comment only summarizes that the following tests check nudges once
per session file. The test names and assertions immediately below already convey that behavior,
leaving the comment without a separate constraint or rationale.
Code

test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs[130]

+    // ── The SessionStart nudges fire once per session file ─────────────────────────────────────
Relevance

●●● Strong

The heading restates immediately adjacent test behavior, matching accepted precedent to trim
redundant explanatory comments.

PR-#617

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2762993 disallows comments that merely restate clear code; the added heading repeats the
purpose expressed by the adjacent test names and assertions.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs[130-130]
test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs[159-183]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new test heading restates the behavior expressed by the tests below it.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs[130-130]

## Recommended Fix
Remove the heading; retain the comments that explain non-obvious test setup or constraints.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. New tools miss their session nudge ✗ Dismissed
Description
HandleSessionStart claims the session whenever the extension accepts stdout, before checking
whether any nudge emitter can produce content. If the tools are unregistered on the first start and
become available before a restart of the same session file, the initial empty result has already
spent the claim and the newly eligible nudge is suppressed.
Code

src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs[R177-179]

+        var nudgeClaim = MemoryContractOf(args) >= 1
+            ? NudgeLease.TryClaimAsync(config, clock.Time, HarnessId.Pi, file, NudgeClaimBudget)
+            : Task.FromResult(false);
Relevance

●●● Strong

Recent accepted precedent requires preserving one-shot output markers when stdout cannot consume
them; this is the same claim-consumption risk.

PR-#1015

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new claim depends only on the stdout contract, and NudgeLease completes it independently of the
later emitter results. Work Items and Plans both return null without their registered tools; the
store refuses another claim once completed.

src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs[173-197]
src/Capacitor.Cli/NudgeLease.cs[25-35]
src/Capacitor.Cli/WorkItemsNudgeEmitter.cs[36-48]
src/Capacitor.Cli/PlansNudgeEmitter.cs[20-28]
src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryLeaseStore.cs[49-61]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Pi permanently completes its nudge lease even when every nudge emitter returns no content. A later start of that session cannot show guidance after tools become available.
## Fix Focus Areas
- src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs[173-197]
- src/Capacitor.Cli/NudgeLease.cs[25-35]
## Recommended Fix
Add a non-consuming eligibility check before taking the claim, or support leaving the lease retryable when resolution produces no nudge. Keep first-run and harness-offer marker consumption behind a successful claim, and test an initially unavailable tool becoming available on resume.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Pi nudge logic sits outside its vendor folder ✗ Dismissed
Description
The new HandleSessionStart nudge claim and output branch are added to
Commands/Harness/PiHookCommand.cs instead of the Pi vendor directory. Pi-specific command behavior
is split across locations, so a later change confined to Harness/Pi could miss the
once-per-session gate.
Code

src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs[R177-179]

+        var nudgeClaim = MemoryContractOf(args) >= 1
+            ? NudgeLease.TryClaimAsync(config, clock.Time, HarnessId.Pi, file, NudgeClaimBudget)
+            : Task.FromResult(false);
Evidence
Rule 2762984 places vendor-specific commands in Capacitor.Cli/Harness/<Vendor>/; the added
Pi-specific claim is in Commands/Harness/.

Rule 2762984: Place vendor-specific harness code only in the correct Harness/&lt;Vendor&gt;/ assembly and directory
src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs[173-179]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new Pi-specific nudge logic is in a command file outside the required Pi vendor directory.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs[173-179]

## Recommended Fix
Move the Pi command into `src/Capacitor.Cli/Harness/Pi/` and update its namespace and references.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
Review mode: ⚖️ Balanced: This changes durable once-per-session claiming, async timeout behavior, and stdout contract gating in a runtime hook, so it carries meaningful correctness and lifecycle risks despite being localized.

Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs Outdated
Comment thread test/Capacitor.Cli.Tests.Unit/Harness/Pi/PiSessionStartMemoryTests.cs Outdated
Comment thread src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs Outdated
Comment thread src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs Outdated
pi.exec discards all stdout from a hook it times out, so a claim taken before a stalled POST
was spent on nudges that never arrived.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@realtonyyoung

Copy link
Copy Markdown
Collaborator Author

Closing: live runs against real Pi 0.85.1 show the premise of AI-2531 doesn't hold, and this change regresses. The kcap extension keeps the injected text only in its per-process cache (cleared on every session shutdown) and Pi never persists it into the session file, so re-emitting the nudge on each session_start is the agent's only copy, not a duplicate. Measured system prompt per turn — main: fresh 1 nudge, restart+resume 1, 1, in-process switch 1; this PR: fresh 1, then 0, 0, 0. Details on AI-2531.

🤖 Generated with Claude Code

@realtonyyoung
realtonyyoung deleted the tonyyoung/ai-2531-pi-nudge-once-per-session branch September 29, 2026 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pi re-injects the work-items nudge on every restart or resume

1 participant