Skip to content

Name the repo's projects in the SessionStart memory lead-in - #768

Merged
alexeyzimarev merged 2 commits into
mainfrom
capacitor/agent-0b9e2523854440
Sep 4, 2026
Merged

alexeyzimarev merged 2 commits into
mainfrom
capacitor/agent-0b9e2523854440

Conversation

@alexeyzimarev

@alexeyzimarev alexeyzimarev commented Sep 4, 2026 •

Copy link
Copy Markdown
Member

Closes #756 — AI-2474

What & why

An agent can only land a memory at project scope by passing a slug, and nothing in a session told it which project the cwd repo is in — so cross-repo learnings went to org, the only cross-repo scope reachable without one. The memory fragment now opens with a line per project naming that slug.

The projects ride the existing index call, negotiated by include=projects: the body is a bare array and a CLI that predates this drops a fragment that is not one, so the shape could not simply change. A sibling or superseding endpoint costs a second round trip inside a hook budget already tight on Cursor and Claude; a response header cannot carry a non-ASCII project name. The parameter degrades in both directions — an older server ignores it and answers with the array, a newer one answers with an object — and the CLI decides which it got by sniffing the opening token.

The server half is kcap-server AI-2470, not yet built. Until it ships the CLI sees the bare array and emits no lead-in. The wire contract this is built against is written up on that issue.

Where to look

include=projects goes out on every index call, with or without a resolved repo: it declares that this CLI can read the object body, not that a repo is in hand.

Projects alone produce a fragment where entries alone produce none — a project holding no memories yet is exactly the state the agent is being asked to fix, and it cannot without the slug. A repo with an empty index and a project therefore gets one line of context instead of silence.

Verification

Capacitor.Cli.Tests.Unit         3978 passed, 0 failed, 19 skipped (gated live certs)
Capacitor.Cli.Tests.Integration   245 passed, 0 failed
dotnet publish -c Release         exit 0, no IL2026/IL3050

Run the two suites separately: concurrently they starve SessionStartMemoryLeaseStore's one-second budget and a handful of lease tests fail on contention alone.

The index body is a bare array and a CLI that predates this drops a fragment that is
not one, so the projects are negotiated with include=projects rather than changing the
shape unasked. Which shape arrived is decided by sniffing the opening token: a failed
array parse is indistinguishable from a corrupt body, which retries with no attempt
ceiling against a server that will never answer differently.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Expose repository project slugs in SessionStart memory context

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Negotiates project metadata on existing memory-index requests without breaking older servers.
• Injects project slugs into SessionStart context, even when no memories exist.
• Documents and tests response compatibility, sanitization, limits, and harness delivery.
Diagram

sequenceDiagram
    actor Agent
    participant Harness
    participant Provider as Memory Provider
    participant Server as Memory API
    participant Emitter
    Harness->>Provider: Fetch memory index
    Provider->>Server: GET include=projects
    alt Server supports projects
        Server-->>Provider: Entries and projects
    else Older server
        Server-->>Provider: Bare entries array
    end
    Provider->>Emitter: Build bounded fragment
    Emitter-->>Harness: Project lead-in and index
    Harness-->>Agent: Inject SessionStart context
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add a separate projects endpoint
  • ➕ Keeps the existing memory-index response shape unchanged.
  • ➕ Separates project discovery from memory retrieval.
  • ➖ Adds another network round trip inside tight SessionStart hook budgets.
  • ➖ Requires coordinating two best-effort requests and their failure states.
2. Return projects in response headers
  • ➕ Preserves the existing JSON body contract.
  • ➕ Avoids a second request.
  • ➖ Requires custom encoding for non-ASCII project names.
  • ➖ Makes structured, extensible metadata awkward and less discoverable.
3. Unconditionally replace the array response
  • ➕ Provides one simple response model for new clients.
  • ➕ Avoids request capability negotiation.
  • ➖ Older CLIs reject the object body and omit the entire memory fragment.
  • ➖ Creates a breaking server-side rollout dependency.

Recommendation: Keep the PR's include=projects capability negotiation. It preserves one-round-trip performance, remains compatible with older clients and servers in either rollout order, and keeps project metadata in an extensible JSON contract. Opening-token detection is preferable to parse-and-fallback because malformed payloads must remain retryable rather than being mistaken for legacy responses.

Files changed (9) +309 / -19

Enhancement (4) +92 / -13
MemoryIndexEmitter.csRender bounded project lead-ins before memory entries +39/-7

Render bounded project lead-ins before memory entries

• Accepts project metadata and emits one normalized instruction line per valid project before the memory index. Project-only responses can now create a fragment, with project count and content included in the existing byte budget.

src/Capacitor.Cli/MemoryIndexEmitter.cs

SessionStartMemoryContextProvider.csNegotiate and parse project-aware index responses +36/-6

Negotiate and parse project-aware index responses

• Adds 'include=projects' to every memory-index request and distinguishes legacy arrays from negotiated objects by inspecting the opening JSON token. Parsed projects are passed to the emitter, while malformed bodies retain retryable-failure behavior.

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContextProvider.cs

SessionStartMemoryContracts.csDefine project-aware memory index contracts +16/-0

Define project-aware memory index contracts

• Adds records for repository projects and the negotiated index response envelope. Introduces an eight-project cap to prevent lead-in metadata from exhausting the fragment budget.

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContracts.cs

SessionStartMemoryJsonContext.csRegister the negotiated response for source-generated JSON +1/-0

Register the negotiated response for source-generated JSON

• Adds 'SessionStartMemoryIndexResponse' to the source-generated serialization context used by the provider.

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryJsonContext.cs

Tests (3) +193 / -5
ClaudeHookCommandTests.csVerify project lead-ins reach Claude context +27/-0

Verify project lead-ins reach Claude context

• Adds an end-to-end hook test proving the capability query is sent and a projects-only response reaches Claude's 'additionalContext'. Exposes the fixture's last memory-index query for contract assertions.

test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs

MemoryIndexEmitterTests.csCover project lead-in rendering and limits +128/-4

Cover project lead-in rendering and limits

• Updates URL expectations for 'include=projects' and tests lead-in ordering, project-only fragments, normalization, invalid slugs, multiple projects, caps, and byte-budget accounting. It also verifies legacy output remains unchanged without project metadata.

test/Capacitor.Cli.Tests.Unit/MemoryIndexEmitterTests.cs

SessionStartMemoryFoundationTests.csTest dual response shapes and retry semantics +38/-1

Test dual response shapes and retry semantics

• Verifies the provider accepts legacy arrays and negotiated objects, handles projects-only and empty responses correctly, and retries bodies matching neither shape. Updates scope URL assertions for the new capability parameter.

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

Documentation (2) +24 / -1
README.mdDocument project-aware SessionStart memory context +1/-1

Document project-aware SessionStart memory context

• Explains that the memory lead-in names each repository project and provides the exact slug for project-scoped saves. Clarifies that project-only responses still produce context while unsupported or unassigned repositories do not.

README.md

CHANGES.mdRecord the project metadata negotiation design +23/-0

Record the project metadata negotiation design

• Documents the motivation, backward-compatible 'include=projects' contract, token-based response-shape detection, and project-only fragment behavior. It also records why extra endpoints and response headers were rejected.

docs/CHANGES.md

@qodo-code-review

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

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Partial objects consume lease ✓ Resolved 🐞 Bug ☼ Reliability
Description
An object-shaped response missing both contract fields, such as {}, deserializes successfully and
has its null fields converted to empty arrays, so the provider reports CompleteWithoutContext. The
orchestrator then completes the session lease instead of retrying the malformed response, preventing
later SessionStart callbacks from fetching the actual project or memory context.
Code

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContextProvider.cs[R50-52]

+        var entries  = index.Entries  ?? [];
+        var projects = index.Projects ?? [];
+        if (entries.Length == 0 && projects.Length == 0) return SessionStartMemoryContextResult.Empty;
Relevance

●●● Strong

Malformed partial object should retry; accepted reliability precedents preserve retries for invalid
index responses.

PR-#350

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The response contract defines an object carrying entries and projects, but both properties are
nullable. ParseIndex accepts any JSON object without checking member presence; the changed
provider then converts both missing values to empty arrays. SessionStartMemoryOrchestrator retries
only RetryableFailure and commits every other disposition, so this malformed response permanently
consumes the session lease.

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContracts.cs[36-41]
src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContextProvider.cs[48-56]
src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContextProvider.cs[68-79]
src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs[45-58]

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

## Issue description
Object-shaped memory-index responses that omit the expected `entries` and `projects` fields are normalized to an empty index. This completes the once-per-session lease rather than treating the contract-invalid response as retryable.

## Issue Context
The negotiated object contract carries both arrays. Preserve legitimate empty arrays and supported nullable values as appropriate, but distinguish them from an object that does not represent an index at all; add coverage for `{}` and other missing-member cases.

## Fix Focus Areas
- src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContextProvider.cs[48-79]
- src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContracts.cs[36-41]
- test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartMemoryFoundationTests.cs[264-298]

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



Informational

2. Conflicting NotInParallel attributes 📘 Rule violation ≡ Correctness
Description
The new test has a bare method-level [NotInParallel] while ClaudeHookCommandTests already has a
keyed class-level [NotInParallel]. Applying both levels violates the required test parallelism
metadata structure and can create conflicting scheduling constraints.
Code

test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs[396]

+    [Test, NotInParallel]
Relevance

● Weak

Recent precedent rejected the same bare method-level attribute despite an existing keyed class-level
constraint.

PR-#721

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2821213 prohibits method-level [NotInParallel] when the declaring class also has
[NotInParallel]. The added attribute at line 396 is inside ClaudeHookCommandTests, whose class
declaration has [NotInParallel("AuthProviderDiscoveryCache")] at line 20.

Rule 2821213: Avoid conflicting [NotInParallel] attributes at method and class level
test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs[18-21]
test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs[396-397]

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 console-capturing test applies method-level `[NotInParallel]` inside a class that already has class-level `[NotInParallel("AuthProviderDiscoveryCache")]`.

## Issue Context
The console-capture rule requires a bare method-level attribute, while the class-level keyed constraint protects a separate shared resource. Move or restructure the test so the same test is not governed by attributes at both levels while preserving both isolation requirements.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs[18-21]
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs[396-417]

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


Grey Divider

Context sources
✅ Compliance rules (platform): 56 rules
Review mode: ⚖️ Balanced: This changes SessionStart runtime behavior, a negotiated API response contract, backward-compatible parsing, hook-budgeted fragment generation, and cross-harness context delivery; it carries meaningful integration and compatibility risk but is not broadly bug-dense enough to require extended review.

Grey Divider

Tip of the day
💡 Did you know, you can route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-04T12:15:06.596627Z 8c680c4 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

An object carrying neither entries nor projects normalised to an empty index, which
reports a successful empty fetch and spends the once-per-session lease, so no later
callback of that session asks again. Absent members and present-but-empty arrays are
now distinct: only the latter is an answer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@alexeyzimarev

Copy link
Copy Markdown
Member Author

Review triage:

1. Partial objects consume lease — accepted, fixed in bfdd506. Real, and a regression this PR introduced: the array-only parse used to throw on any object body and come back retryable, so {} was already safe before. ParseIndex now rejects an object carrying neither member, keeping "absent" distinct from "present but empty" — an empty array is a real answer and still completes the lease. Covered by parameterised cases on both sides of that line ({}, {"entries":null,"projects":null}, {"unrelated":1} retry; {"entries":[],"projects":[]}, {"entries":[]}, [] complete).

2. Conflicting [NotInParallel] — not taking this one. The 23 other tests in ClaudeHookCommandTests pair the same bare method-level attribute with the class-level [NotInParallel("AuthProviderDiscoveryCache")], and the pairing is deliberate rather than accidental: the test captures Console, which the repo requires bare exclusivity for, and bare [NotInParallel] is exclusive against the whole assembly — it strictly subsumes the keyed constraint, so no isolation is lost. Dropping it to the keyed form would weaken the test; leaving only it in the class would make it the odd one out.

The ubuntu leg failed on AgentOrchestratorSourceClaimTests.Deferred_launch_claims_then_forwards_without_rebinding_then_first_turns_then_confirms — the known GitRepo.CreateWithCommit flake (#707), in an assembly this PR does not touch.

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.

SessionStart memory index: name the repo's project in the lead-in

1 participant