Repository navigation
fix(agents): honor GENTLE_PI_AGENT_HOME for the child's web package lookup - #2516
aiorellana502 wants to merge 1 commit into
Conversation
…ookup requestedWebPackagePaths() only read PI_CODING_AGENT_DIR when locating the installed npm:pi-web-access package for a subagent's web tools, ignoring GENTLE_PI_AGENT_HOME. resolveGentlePiAgentHome() (lib/agent-home.ts), the single resolution point gentle-shell#1731 introduced so routing and launch never disagree on the agent home, checks GENTLE_PI_AGENT_HOME first. A session with only GENTLE_PI_AGENT_HOME set would have its web-tool package lookup silently fall back to the wrong default directory. Found while investigating gentle-shell#2163; the issue's own profileRoot()/agentHome divergence hypothesis was checked and does not reproduce in current main (every production DiscoveryRoots.agentHome caller already resolves through resolveAgentHomeDirectory / resolveGentlePiAgentHome).
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Agent home precedence lib/agents-runner.ts, tests/child-web-package.test.ts |
Lookup prefers GENTLE_PI_AGENT_HOME and falls back to PI_CODING_AGENT_DIR. Tests verify the preferred home is used when set alone or alongside the fallback. |
Priority:
Estimated code review effort: 2 (Simple) | ~5 minutes
Change: Bug fix
Suggested labels: type:bug
Suggested reviewers: decode2, alan-thegentleman
Merge Risk: 
Minimal · up to 98dc2
-
No actionable merge-blocking risk is identified.
Pre-merge checks: 
3 · 
2
-
Failed checks: 2 warnings Passed checks: 3 passedFull details: Linked Issues check
-
Explanation
Issue
#2163is open and directly linked, so its coding requirements apply. Its expected outcome requires either a one-time migration of legacy panel store files (profiles.json, models.json, routing), or a warning that names the unmigrated legacy files during setup, on first panel open, or through sync/doctor. The PR diff does not implement either. The only source change is a one-line edit in requestedWebPackagePaths() in lib/agents-runner.ts (GENTLE_PI_AGENT_HOME checked before PI_CODING_AGENT_DIR). The only test added is in tests/child-web-package.test.ts, and it covers web-package lookup. The PR description and the PR objectives both state that this PR does not close#2163. The concrete unmet requirement is the missing migration or warning behavior for legacy panel store files. The review-lane failure described in#2163also remains unaddressed. Tests were not re-run in this assessment. The author-reported results were not independently verified.Resolution
Implement the
#2163behavior in this PR: a one-time migration of legacy panel store files, or a warning that names the unmigrated legacy files in at least one of the listed surfaces (setup, panel open, sync, doctor). Add automated tests for that behavior. Alternatively, if this PR must not address#2163, change the#2163link to a contextual mention so no coding requirements apply to this PR.
Full details: Out of Scope Changes check
-
Explanation
The code change does not implement any objective of
#2163. The change alters the environment variable precedence in requestedWebPackagePaths(), which locates the installed npm:pi-web-access package for subagent web tools.#2163concerns legacy panel store files and their migration or warning. No code path in the diff links the two. The PR description and the PR objectives also state that the PR addresses web-package lookup, not panel-store migration. The change is a separate fix for a different defect. Because the PR's only code change is unrelated to its linked issue, the change is out of scope for#2163.Resolution
Move the requestedWebPackagePaths() precedence fix and its test to a separate PR. Link that PR to an issue that tracks the web-package lookup defect. Alternatively, link this PR to an issue that describes the web-package lookup problem, so the change has a matching requirement. Keep
#2163linked only if this PR also implements the#2163migration or warning behavior.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
|
Thanks @aiorellana502 for investigating and for the honest follow-up that the original hypothesis did not reproduce. We are closing this one because the replacement precedence can regress the real install path. Pi resolves its agent directory solely from The test in the branch asserts that fabricated layout rather than anything an installer produces. If a real case ever shows up where the two variables differ in a production launch, an issue with that repro is the right way back. |
|
Closing for regression risk; see comment above. |
Context
While investigating the
profileRoot()/agent-home divergence hypothesis raised in #2163 (the idea that a resolver mirroringresolveGentlePiAgentHome()'s inline fallback diverges from it), I traced every production caller ofDiscoveryRoots.agentHome(extensions/gentle-ai.ts:4520,extensions/gentle-agents.ts:520viaresolveAgentHomeDirectory). That hypothesis does not reproduce on currentmain: every real caller already resolvesagentHomethrough the single resolution pointgentle-shell#1731introduced (resolveGentlePiAgentHome/resolveAgentHomeDirectoryinlib/agent-home.tsandlib/agent-model-resolution.ts).profileRoot()'s own inline fallback is only reached by tests that intentionally omitagentHome.While tracing that, I found a smaller, real instance of the same divergence class elsewhere:
requestedWebPackagePaths()inlib/agents-runner.ts, used to locate the installednpm:pi-web-accesspackage for a subagent's web tools (fetch_content,web_search, etc.), read onlyrequest.env.PI_CODING_AGENT_DIR.resolveGentlePiAgentHome()checksGENTLE_PI_AGENT_HOMEfirst, falling back toPI_CODING_AGENT_DIR. A session with onlyGENTLE_PI_AGENT_HOMEset (noPI_CODING_AGENT_DIR) would have this lookup silently fall back to the wrong default home directory and fail to find an already-installed web package, degrading the child's declared capability with no error.Fix
lib/agents-runner.ts: checkGENTLE_PI_AGENT_HOMEahead ofPI_CODING_AGENT_DIR, matchingresolveGentlePiAgentHome()'s precedence, while keeping the existing tilde/HOME-override expansion this function already does for its own tests.Tests
tests/child-web-package.test.ts: new testweb lookup honors GENTLE_PI_AGENT_HOME ahead of PI_CODING_AGENT_DIR, covering bothGENTLE_PI_AGENT_HOMEset alone and set alongsidePI_CODING_AGENT_DIR(precedence). RED without the fix, GREEN with it.Verification
node --experimental-strip-types --test tests/child-web-package.test.ts: 11/11 pass.node --experimental-strip-types --test tests/agents-runner.test.ts: 99/100 pass; the one failure (AgentRunner retains permission broker fd3 and assigns messaging IPC to fd4, a WindowsTEMP/TMP-vs-TMPDIRenv assumption) reproduces identically on a cleanmaincheckout with zero source changes — pre-existing, unrelated to this change.node scripts/check-types.mjs: 0 diagnostics.Does not close #2163 (that issue's actual root cause, the global/legacy
models.jsonmerge, is #2477). Posting a clarifying comment on #2163 pointing here and to the closedprofileRoot()hypothesis.Summary by CodeRabbit