Skip to content

[AI-3317] Serve Pi the team-memory index on every session start - #1214

Merged
realtonyyoung merged 2 commits into
mainfrom
tonyyoung/ai-3317-pi-memory-index-on-resume
Sep 29, 2026
Merged

realtonyyoung merged 2 commits into
mainfrom
tonyyoung/ai-3317-pi-memory-index-on-resume

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

AI-3317 (no GitHub issue — filed in Linear only)

What & why

A Pi session got the team-memory index only on its first session_start: every restart, resume or in-process session switch lost it for good. The hook spent a once-per-file lease, but kcap.ts holds the fragment only in memory and drops it at every session_shutdown (Pi fires one on exit and before a switch), and Pi persists none of the injected text. Pi's lifecycle now declares HostKeepsContext: false, the policy answers a new EligibleEveryStart, and the orchestrator fetches without a lease for it, so each start gets a fresh copy.

Where to look

SessionStartMemoryOrchestrator's new arm: no lease is read or written, a failed fetch leaves the next start to try again, and the commit gate is still honoured. Re-delivery cannot duplicate: the extension replaces its cache and guards the per-turn append with includes(). Every other harness keeps HostKeepsContext at its default true, so its behaviour is unchanged.

Verification

  • Live, real Pi 0.85.1 + real kcap.ts + this branch's kcap, with a stand-in provider logging each turn's system prompt. Memory index per turn, main → this PR: fresh 1 → 1; restart + resume 0 → 1; again 0 → 1; in-process switch_session 0 → 1 (three turns, all 1). Never 2, and the session file holds none of it.
  • Removing the policy's EligibleEveryStart arm fails four tests, including the lease-keeping control case.
  • *SessionStartMemory* 186/186 and PiSessionStartMemoryTests 20/20 locally.

🤖 Generated with Claude Code

kcap.ts holds the fragment only in memory and drops it at every session_shutdown, so a
once-per-file lease left every restart, resume and switch without the index.

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

linear-code Bot commented Sep 29, 2026

Copy link
Copy Markdown

AI-3317

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Restore Pi team-memory index on every session start

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Fetch Pi’s team-memory index on every start so resumes and session switches retain context.
• Preserve lease-based delivery for hosts that keep injected context.
• Test repeated delivery, failed-fetch retries, and Pi lifecycle eligibility.
Diagram

graph TD
  Ext["Pi extension"] --> Hook["Pi hook"] --> Orch["Memory orchestrator"] --> Policy["Lifecycle policy"] --> Decision{"Keep context?"}
  Decision -->|"No: every start"| Provider["Context provider"]
  Decision -->|"Yes: existing path"| Lease["Lease store"] --> Provider
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Persist the fragment in Pi session state
  • ➕ Could avoid refetching on restart or resume.
  • ➖ Requires durable storage and restoration outside the current extension contract.
  • ➖ Can serve stale context instead of a fresh index.

Recommendation: Use the PR’s lifecycle-driven refetch. Pi discards its in-memory fragment on shutdown and persists none of the injected text; fetching on each start restores delivery without changing Pi session storage or other hosts’ lease semantics.

Files changed (6) +92 / -19

Enhancement (1) +6 / -2
SessionStartMemoryContracts.csRepresent hosts that need context on every start +6/-2

Represent hosts that need context on every start

• Adds an every-start eligibility decision and an optional HostKeepsContext lifecycle property. Its default remains true so existing harnesses retain their behavior.

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContracts.cs

Bug fix (3) +20 / -10
PiHookCommand.csMark Pi memory context as transient +10/-10

Mark Pi memory context as transient

• Pi’s lifecycle now declares that its host does not keep injected context, making each eligible session start eligible for delivery. Comments no longer describe once-per-file lease behavior for Pi.

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

SessionStartMemoryLifecyclePolicy.csSelect every-start delivery for transient context +2/-0

Select every-start delivery for transient context

• After rejecting invalid, non-top-level, and compact callbacks, the policy selects every-start eligibility when a host does not keep context. Lease-backed and one-shot decisions remain unchanged for other hosts.

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryLifecyclePolicy.cs

SessionStartMemoryOrchestrator.csFetch every-start context without a lease +8/-0

Fetch every-start context without a lease

• The new decision fetches a fresh fragment without reading or writing the lease store. Failed fetches remain retryable on the next start, and successful fetches still honor the optional commit gate.

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs

Tests (2) +66 / -7
PiSessionStartMemoryTests.csAssert Pi’s every-start lifecycle decision +6/-7

Assert Pi’s every-start lifecycle decision

• Updates the Pi lifecycle test to verify transient host context and every-start eligibility for a rooted session file.

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

SessionStartMemoryFoundationTests.csCover every-start eligibility and delivery +60/-0

Cover every-start eligibility and delivery

• Adds policy cases for transient context and tests repeated fetches without lease records. Control and retry cases verify that context-keeping hosts still deduplicate delivery and failed transient-context fetches can succeed on the next start.

test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartMemoryFoundationTests.cs

@qodo-code-review

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

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Reloads retain outdated team memory ✗ Dismissed
Description
EligibleEveryStart returns null when the provider reports CompleteWithoutContext, and
WriteMemoryFragment emits empty stdout without telling the Pi extension to clear memFragment.
When a same-file reload follows a previously cached fragment and the index is now empty, the
extension retains that fragment and injects it on later turns.
Code

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs[R45-47]

+                if (fresh.Disposition != SessionStartMemoryDisposition.Ready) return null;
+                if (commitGate is not null && !await commitGate(request.CancellationToken)) return null;
+                return fresh.Fragment;
Relevance

●●● Strong

Recent accepted precedents favor fixing stale cached state when refresh results no longer reflect
current data.

PR-#670
PR-#1138

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Pi now requests a fresh result on every eligible start. The composite provider can report a
successful empty result; the new branch converts it to null, which Pi writes as empty stdout. On a
repeated start for the same file, the extension neither resets the file cache nor replaces the
fragment unless stdout begins with the memory marker, so its before-turn handler continues appending
the old value.

src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs[303-306]
src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs[69-79]
src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs[43-47]
src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs[284-294]
src/Capacitor.Cli.Core/Harness/Pi/PiExtensionInstaller.cs[79-108]

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

## Issue description
A fresh, successful empty fetch leaves Pi's previously cached fragment active on a same-file reload.

## Fix Focus Areas
- src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs[43-47]
- src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs[284-294]
- src/Capacitor.Cli.Core/Harness/Pi/PiExtensionInstaller.cs[79-108]

## Recommended Fix
Give Pi a distinguishable successful-no-context response and clear its cached fragment when the extension receives that response. Preserve the cache on fetch failures, which must remain distinguishable from a successful empty result.

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


2. A test comment repeats its assertions ✓ Resolved
Description
The summary above Every_start_of_a_session_whose_host_keeps_no_context_is_served describes the two
deliveries, lease-store check, and control case that the test itself asserts. Its parameter cases
and assertions immediately below already show those behaviors, so the comment adds no constraint a
later change needs to preserve beyond the test.
Code

test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartMemoryFoundationTests.cs[R126-128]

+    /// <summary>The same session started twice gets the fragment both times when its host keeps
+    /// nothing, and nothing is written to the lease store. A host that does keep context is the
+    /// control: the same two starts yield the fragment once.</summary>
Relevance

●●● Strong

Recent precedents accept shortening test comments that restate test names, setup, and assertions.

PR-#539
PR-#692

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2762993 disallows comments that merely restate what the code clearly does. The added summary
describes the outcomes asserted directly by the adjacent parameterized test.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartMemoryFoundationTests.cs[126-151]

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 summary restates the test cases and assertions rather than documenting a non-obvious constraint.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartMemoryFoundationTests.cs[126-128]

## Recommended Fix
Remove the summary and leave the parameter cases and assertions to describe the tested behavior.

ⓘ 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
  Explored: repo: kurrent-io/kcap-server (sha: 6037493c) — View relationship
Review mode: ⚖️ Balanced: This is a focused but behavioral lifecycle/orchestration change affecting session-start delivery, lease semantics, commit gating, and multiple harness paths, so it warrants a complete single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@realtonyyoung
realtonyyoung merged commit 5748cd3 into main Sep 29, 2026
14 of 15 checks passed
@realtonyyoung
realtonyyoung deleted the tonyyoung/ai-3317-pi-memory-index-on-resume branch September 29, 2026 01:00
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.

1 participant