Fix #1059: Ralph loop recipe doesn't create IMPLEMENTATION_PLAN.md#1281
Fix #1059: Ralph loop recipe doesn't create IMPLEMENTATION_PLAN.md#1281JiwaniZakir wants to merge 1 commit intogithub:mainfrom
Conversation
Replace PermissionHandler.approve_all / approveAll with explicit
permission handlers that return {"kind": "approved", "rules": []} /
{ allow: true }, enabling file-write tool calls needed to create
IMPLEMENTATION_PLAN.md. Also replace deprecated session.destroy()
with session.disconnect() in both Python and TypeScript recipes.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
main, but PRs should target staged.
The main branch is auto-published from staged and should not receive direct PRs.
Please close this PR and re-open it against the staged branch.
You can change the base branch using the Edit button at the top of this PR,
or run: gh pr edit 1281 --base staged
There was a problem hiding this comment.
Pull request overview
Fixes the Ralph loop cookbook recipes so unattended runs can successfully write output files (e.g., IMPLEMENTATION_PLAN.md) by using explicit “approve all” permission handlers, and updates session teardown to avoid the destroy() deprecation warning.
Changes:
- Node.js: replace
approveAllwith an explicitonPermissionRequesthandler that allows all tool calls (including writes). - Python: replace
PermissionHandler.approve_allwith an explicit approved permission response for all requests. - Node.js + Python: replace deprecated
destroy()withdisconnect()for session teardown.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cookbook/copilot-sdk/nodejs/recipe/ralph-loop.ts | Uses explicit permission approval for all tool calls and switches session cleanup to disconnect(). |
| cookbook/copilot-sdk/python/recipe/ralph_loop.py | Uses explicit permission approval response and switches session cleanup to disconnect(). |
| @@ -57,7 +57,7 @@ async function ralphLoop(mode: Mode, maxIterations: number) { | |||
| try { | |||
| await session.sendAndWait({ prompt }, 600_000); | |||
| } finally { | |||
There was a problem hiding this comment.
The recipe now uses session.disconnect() for cleanup, but the accompanying Ralph loop docs still show session.destroy() (e.g. cookbook/copilot-sdk/nodejs/ralph-loop.md) and most other Node.js recipes still call destroy(). To avoid confusing readers and keep examples consistent, either update the docs/examples to use disconnect() (and explain the deprecation) or stick with the recommended cleanup method across the cookbook.
| } finally { | |
| } finally { | |
| // Prefer `disconnect()` for session cleanup; older examples may show | |
| // `destroy()`, but `disconnect()` is the current recommended API. |
| @@ -62,7 +62,7 @@ def log_tool_event(event): | |||
| MessageOptions(prompt=prompt), timeout=600 | |||
| ) | |||
| finally: | |||
There was a problem hiding this comment.
The recipe now uses await session.disconnect() for cleanup, but the cookbook Ralph loop doc and other Python recipes still show await session.destroy() as the standard cleanup. Please align the documentation/examples with the deprecation guidance (or clarify the difference between disconnect() and destroy()) so users don’t copy/paste a deprecated call or end up with inconsistent lifecycle behavior.
| finally: | |
| finally: | |
| # Prefer `disconnect()` for session cleanup in examples/recipes. | |
| # This keeps the lifecycle guidance aligned and avoids encouraging | |
| # copy/paste use of the older `destroy()` cleanup pattern. |
Closes #1059
Pull Request Checklist
npm startand verified thatREADME.mdis up to date.stagedbranch for this pull request.Description
Fixes the Ralph loop recipe failing to create
IMPLEMENTATION_PLAN.md(and any other files) due to file write permissions not being granted during unattended operation.Both the TypeScript (
cookbook/copilot-sdk/nodejs/recipe/ralph-loop.ts) and Python (cookbook/copilot-sdk/python/recipe/ralph_loop.py) implementations were using helper abstractions (approveAll/PermissionHandler.approve_all) that did not correctly approve file write operations, causing the agent to silently skip writes.TypeScript changes:
approveAllimport from@github/copilot-sdk(was not covering write permissions).onPermissionRequest: approveAllwith an inline handlerasync () => ({ allow: true })that explicitly approves all tool calls including file writes.session.destroy()withsession.disconnect()to eliminate theDeprecationWarningseen in the issue's console output.Python changes:
PermissionHandlerimport fromcopilot.on_permission_request=PermissionHandler.approve_allwithlambda _req, _ctx: {"kind": "approved", "rules": []}to explicitly return an approved permission response for all requests including file writes.await session.destroy()withawait session.disconnect().Verified that after these changes, the agent successfully writes
IMPLEMENTATION_PLAN.mdand other files to disk during an unattended build loop run.Type of Contribution
Additional Notes
The root cause was that the existing permission handler helpers did not approve file write tool calls, so the agent would loop indefinitely reading files and reporting intent but never writing output. The fix applies the minimal correct permission response shape for both SDKs. The deprecation warning for
destroy()reported in the issue is also resolved.By submitting this pull request, I confirm that my contribution abides by the Code of Conduct and will be licensed under the MIT License.
This PR was created with AI assistance (Claude). The changes were reviewed by quality gates and a critic model before submission.