fix(pi): isolate explicit cross-project memory saves by Alan-TheGentleman · Pull Request #1592 · Gentleman-Programming/engram

Alan-TheGentleman · 2026-10-01T14:56:28Z

🔗 Linked Issue

🏷️ PR Type

📝 Summary

Route explicit Pi project/cwd writes through isolated project-owned satellite sessions without changing principal ownership or inferring targets from edited files.
Require server isolation capability on every registration dispatch/recovery and atomically protect existing directories, continuations and leases.
Fix trusted Unix-socket test fixtures and preserve autosync upgrade-pause status across admitted cycle outcomes without cancelling work or releasing leases.

📂 Changes

File	Change
`plugin/pi/index.ts`	Explicit target resolution, satellite lifecycle, dispatch capability guard and numeric acknowledgement validation
`internal/server/server.go`, `internal/store/store.go`	Isolation capability/registration contract and transaction guards
Pi and Go isolation tests	Real-core attribution, recovery compatibility, full lease/state preservation
`internal/server/unix_socket_test.go`, `cmd/engram/serve_socket_test.go`	Cleanup-managed private 0700 parents, production trust policy unchanged
`internal/cloud/autosync/manager.go`, `manager_test.go`	Mutex-held pause precedence, deterministic channel barriers
`DOCS.md`, `plugin/pi/README.md`	Explicit routing and compatibility contract
`odd/tasks/pi-cross-project-saves.md`	Work-unit evidence and deferred investigation

🧪 Test Plan

cd plugin/pi && npm test — 272/272 PASS, real Go-server attribution included.
go test -count=1 -v ./internal/server -run '^TestUnixSocket' — 6 top-level tests and 6 policy subcases PASS.
Focused CLI socket and autosync pause/resume regressions — PASS; deterministic RED/GREEN observed.
go test -race -count=20 ./internal/cloud/autosync -run '^TestManager(StopForUpgrade|Resume)' — PASS independently.
Affected CLI/autosync/server/store package tests — PASS.
go vet ./cmd/engram/... ./internal/cloud/autosync/... and server/store vet — PASS.
git diff --check — PASS.
Final overall local Go green gate: writer full go test -count=1 ./... passed; independent full run had 28 passing packages, 1 plugin failure, 3 without tests. TestSubagentStopExplicitEmptyMessageSelection/primary_wins reported empty JSON body; subsequent focused, plugin-package and diagnostic-overlay runs each passed once. Cause unresolved, no Claude fix claimed.
Windows local validation — not run; applicable CI pending.

🤖 Automated Checks

Required checks remain pending until executed by GitHub: Check Issue Reference; Check Issue Has status:approved; Check PR Has type:* Label; Unit Tests; E2E Tests; Plugin Tests. Lint, transient-artifact, platform and other applicable checks also remain pending. No admin bypass or force merge is authorized.

✅ Contributor Checklist

Linked approved bug(pi): explicit cross-project memory saves conflict with session ownership #1591.
Exactly one type:* label requested: type:bug.
Actual regression/package evidence recorded.
Additional concurrency/isolation checks and missing overall/platform evidence disclosed.
User-facing documentation updated.
Conventional Commits; no Co-Authored-By trailers.
Changed paths checked against transient artifact policy; .codegraph/ excluded.

💬 Notes for Reviewers

Single-PR exception explicitly approved by maintainer: 1568 additions+deletions across 17 files, mainly regression evidence. size:exception requested; no code compression or artificial test splitting.

Work units: 0f5fdf2 explicit routing/isolation, 5aa207e server socket fixtures, 66ed312 CLI fixture/autosync pause, f2f78f2 progress disposition.

Native exact-candidate reviews approved/acknowledged: review-bb33ea57146a0faf, review-38aa747720e5f3c1, review-fbee4168a9483776. Independent scoped gates passed. Nonblocking satellite-key collision advisory remains follow-up, not a correction reopened on the approved candidate.

Deferred: unexplained intermittent Claude-hook test. Pause precedence is NOT cancellation, draining or rollback quiescence. Existing nonblank satellites fail closed rather than being silently repaired; older servers require upgrade for foreign-project writes. Merge must wait for fresh mandatory CI and repository policy.

Summary by CodeRabbit

New Features
- Pi can now save memories and session summaries to an explicitly selected project using its project name or working directory. Cross-project saves use a separate project-owned session; same-project saves continue using the current session.
- The server advertises support for isolated session registration. Requests that conflict with an existing session’s project directory are rejected without changing that session.
Bug Fixes
- Paused sync status is preserved when an in-progress sync completes or encounters an error.
Documentation
- Updated the HTTP API and Pi guides with cross-project save and isolated-session behavior.

Route project and cwd targets through project-owned satellites without rebinding the runtime session. Require isolation capability on each dispatch and validate roots and continuations before lease mutations.

Keep success and negative socket scenarios inside a trusted 0700 parent without weakening production hierarchy checks.

Keep disabled phase and pause reason until explicit resume while retaining outcome persistence and lease ownership. Use a private trusted parent in the CLI socket fixture so its shutdown assertion is exercised.

coderabbitai · 2026-10-01T14:57:09Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e77e342b-1abb-40db-8376-acf9fce8dce2

📥 Commits

Reviewing files that changed from the base of the PR and between 4cb56ac and f2f78f2.

📒 Files selected for processing (17)

DOCS.md
cmd/engram/serve_socket_test.go
internal/cloud/autosync/manager.go
internal/cloud/autosync/manager_test.go
internal/server/isolated_session_test.go
internal/server/server.go
internal/server/server_test.go
internal/server/unix_socket_test.go
internal/store/isolated_session_test.go
internal/store/store.go
odd/tasks/pi-cross-project-saves.md
plugin/pi/README.md
plugin/pi/index.ts
plugin/pi/test/cross-project-saves.test.mjs
plugin/pi/test/index-source.test.mjs
plugin/pi/test/native-tool-contract.test.mjs
plugin/pi/test/startup-lifecycle.test.mjs

 __________________________
< Needle. Haystack. Found. >
 --------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

Copilot review overview

🟡 Changes recommended

Raw explicit project names can incorrectly route canonically equivalent same-project writes through satellite sessions.

Review effort: Balanced
Findings: 1

Open (1)

Canonicalize explicit project before owner comparison · New

What changed in this PR

Adds isolated Pi satellite sessions for explicit cross-project memory writes while preserving principal session ownership.

Changes:

Routes explicit project/cwd writes through isolated satellite sessions.
Adds atomic server/store isolation enforcement and capability negotiation.
Improves socket fixtures and autosync pause-state handling.

File	Description
`plugin/pi/index.ts`	Implements explicit target resolution and satellites.
`plugin/pi/test/cross-project-saves.test.mjs`	Tests cross-project routing.
`plugin/pi/test/startup-lifecycle.test.mjs`	Tests guarded retries and recovery.
`plugin/pi/test/native-tool-contract.test.mjs`	Extends integration coverage.
`plugin/pi/test/index-source.test.mjs`	Updates source extraction assertions.
`plugin/pi/README.md`	Documents satellite routing.
`internal/store/store.go`	Adds atomic isolated registration.
`internal/store/isolated_session_test.go`	Tests isolation invariants.
`internal/server/server.go`	Exposes capability and isolated API.
`internal/server/server_test.go`	Expands directory behavior coverage.
`internal/server/isolated_session_test.go`	Tests isolated HTTP registration.
`internal/server/unix_socket_test.go`	Uses trusted private socket fixtures.
`cmd/engram/serve_socket_test.go`	Hardens CLI socket fixture.
`internal/cloud/autosync/manager.go`	Preserves upgrade-pause status.
`internal/cloud/autosync/manager_test.go`	Adds deterministic pause regressions.
`DOCS.md`	Documents the API contract.
`odd/tasks/pi-cross-project-saves.md`	Records implementation evidence.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  const writeEpoch = writeState?.epoch;
-  const registeredSessionForWrite = async (sessionProject: string) => registerEffectiveSession(ctx, sessionProject, appendEntry, fetch);
+  const writeTarget = WRITE_TARGET_TOOLS.has(toolName) ? await resolveExplicitWriteTarget(params, fetch) : undefined;
+  const requestedProject = writeTarget?.project || (typeof params.project === "string" && params.project ? params.project : undefined);


Alan-TheGentleman added 4 commits October 1, 2026 15:57

fix(pi): isolate explicit cross-project memory writes

0f5fdf2

Route project and cwd targets through project-owned satellites without rebinding the runtime session. Require isolation capability on each dispatch and validate roots and continuations before lease mutations.

test(server): use private directories for socket fixtures

5aa207e

Keep success and negative socket scenarios inside a trusted 0700 parent without weakening production hierarchy checks.

fix(autosync): preserve upgrade pause across in-flight outcomes

66ed312

Keep disabled phase and pause reason until explicit resume while retaining outcome persistence and lease ownership. Use a private trusted parent in the CLI socket fixture so its shutdown assertion is exercised.

docs(odd): record verified boundaries and deferred hook investigation

f2f78f2

Alan-TheGentleman added type:bug Bug fix size:exception Maintainer-approved exception to the 400-line review budget labels Oct 1, 2026

Copilot AI balanced review requested due to automatic review settings October 1, 2026 14:56

Copilot started reviewing on behalf of Alan-TheGentleman October 1, 2026 14:57 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Alan-TheGentleman added this pull request to the merge queue Oct 1, 2026

Merged via the queue into main with commit 204156e Oct 1, 2026
28 of 31 checks passed

Navigation Menu

fix(pi): isolate explicit cross-project memory saves - #1592

fix(pi): isolate explicit cross-project memory saves#1592
Alan-TheGentleman merged 4 commits into
mainfrom
fix/pi-cross-project-saves

Alan-TheGentleman commented Oct 1, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Alan-TheGentleman commented Oct 1, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Linked Issue

🏷️ PR Type

📝 Summary

📂 Changes

🧪 Test Plan

🤖 Automated Checks

✅ Contributor Checklist

💬 Notes for Reviewers

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Alan-TheGentleman commented Oct 1, 2026 •

edited by coderabbitai Bot

Loading