Skip to content

[5283] fix(agent): Keep Pi skill paths stable on cold resume - #5295

Merged
mmabrouk merged 1 commit into
big-agentsfrom
fix/pi-skill-workspace
Jul 14, 2026
Merged

[5283] fix(agent): Keep Pi skill paths stable on cold resume#5295
mmabrouk merged 1 commit into
big-agentsfrom
fix/pi-skill-workspace

Conversation

@mmabrouk

@mmabrouk mmabrouk commented Jul 13, 2026

Copy link
Copy Markdown
Member

Context

Configured Pi skills currently live under a random per-run agent directory. Teardown deletes that directory, so a cold resume can contain references to skill files that no longer exist.

This PR is stacked on #5292. The lower PR persists Pi transcripts. This PR gives configured skills a stable session-workspace path without changing replay, continuation, token, cache, or Claude behavior.

Changes

Pi now receives one content-addressed skill snapshot at:

<cwd>/agents/skills/<digest>/<skill-name>

The runner computes the digest from skill names, file paths, modes, and contents. It publishes through a unique temporary directory, writes a completion marker last, and reuses only a matching complete snapshot. Existing or older snapshots are never reconciled, replaced, or deleted during acquisition.

The pi-acp patch passes the exact snapshot to Pi with --no-skills --skill <path>. Only the current snapshot is advertised, while old paths remain readable by a cold-resumed transcript. The runner no longer copies configured skills into the temporary local or Daytona Pi agent directory.

This deliberately accepts append-only storage growth until the session workspace is removed. Snapshot immutability is a runner convention because the cwd remains agent-writable.

Live local QA

Verified on the EE deployment at port 8280 against the actual service-targeted subscription sidecar. The live installed pi-acp was checked for PI_CODING_AGENT_SKILL_DIR before the run.

  • Turn 1 delivered an inline continuity-probe skill. Pi called read on <cwd>/agents/skills/4d2431.../continuity-probe/SKILL.md, then returned SKILL-COLD-7Q42-OK.
  • The same durable cwd contained one Pi transcript, one .agenta-skill-set.json marker, and the skill file under that digest.
  • After deleting and recreating the sidecar, the new container had no prior /tmp. Once the existing 120-second owner lease expired, the session loaded natively and returned SKILL-COLD-7Q42-OK PI-COLD-MEM-9Q7R without receiving the earlier message again.
  • After the normal 60-second warm-pool eviction, another cold turn remounted the same cwd, logged session/load ... loaded=true, and recalled the same private marker.
  • Keeping the skill body unchanged reused the same digest. Changing the body for the final probe created a second digest and left the first snapshot intact, confirming append-only behavior with no reconciliation or deletion.

The first immediate post-restart retry was rejected by the existing local-runner ownership guard. That is expected affinity behavior and is not caused by this PR. A stable AGENTA_RUNNER_REPLICA_ID avoids the lease wait in orchestrated restarts.

Tests

  • pnpm exec vitest run --project unit tests/unit/sandbox-agent-pi-assets.test.ts tests/unit/sandbox-agent-orchestration.test.ts: 65 passed.
  • pnpm run typecheck: passed.
  • pnpm test: 69 files and 1,066 tests passed.
  • Live Daytona was not run. Daytona publication, reuse, mismatch failure, and the no-copy-to-private-agent-dir invariant are covered by the orchestration and asset unit tests.

How to review

  1. Start with pi-assets.ts for digesting, atomic publication, marker validation, and fail-closed behavior.
  2. Read workspace.ts and sandbox_agent.ts for local and Daytona ordering.
  3. Check the pi-acp patch for the narrow explicit-skill argument.
  4. Finish with the orchestration and asset tests.

Depends on #5292. Closes #5283.

@vercel

vercel Bot commented Jul 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agenta-documentation Ready Ready Preview, Comment Jul 14, 2026 12:12am

Request Review

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 668c62b5-94f0-443d-9b0f-6d9591c8b33b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pi-skill-workspace

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.

@mmabrouk
mmabrouk marked this pull request as ready for review July 13, 2026 23:33
@mmabrouk mmabrouk added the needs-review Agent updated; awaiting Mahmoud's review label Jul 13, 2026
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jul 13, 2026
@mmabrouk

Copy link
Copy Markdown
Member Author

Local skill-path QA is complete. Pi actually read the inline skill from <cwd>/agents/skills/<digest>/continuity-probe/SKILL.md; the unchanged body reused its digest across the cold container resume; changing the body produced a new digest while preserving the old snapshot. The resumed native transcript recalled the private first-turn marker after both container recreation and normal TTL eviction. Live Daytona was not run; its path is covered by the focused orchestration tests.

@mmabrouk

Copy link
Copy Markdown
Member Author

🤖 The AI agent says:\n\nFinal verification of the stacked skill-workspace PR:\n\n- Head: 14b09da\n- Base: fix/pi-transcript-persistence at e0593b4\n- The upper delta is exactly the intended seven-file skill-workspace change.\n- The final upper tree is byte-identical to the tree already exercised in live local QA.\n- Live QA confirmed that Pi reads skills from agents/skills/, unchanged skills reuse the digest, changed skills create a new digest without removing the old snapshot, and a real cold container recreation can load the persisted native transcript.\n- GitHub CI is fully green, including runner unit, integration, and acceptance tests, plus API, SDK, services, web, contribution checks, CLA, and Vercel.\n- This PR does not alter replay, warm/cold selection, token use, or prompt-cache behavior.

@mmabrouk mmabrouk left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mmabrouk
mmabrouk changed the base branch from fix/pi-transcript-persistence to big-agents July 14, 2026 00:34
@mmabrouk
mmabrouk merged commit 31214e8 into big-agents Jul 14, 2026
20 checks passed
@mmabrouk mmabrouk mentioned this pull request Jul 14, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend needs-review Agent updated; awaiting Mahmoud's review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant