-
Notifications
You must be signed in to change notification settings - Fork 798
fix(pi): resolve trellis project root from session cwd #581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ interface PiToolResult { | |
| details?: unknown; | ||
| } | ||
| interface PiExtensionContext { | ||
| cwd?: string; | ||
| hasUI?: boolean; | ||
| model?: { | ||
| provider?: string; | ||
|
|
@@ -964,12 +965,34 @@ function readJsonlEntries(basePath: string, jsonlPath: string): JsonlEntry[] { | |
| function findRoot(start: string): string { | ||
| let c = resolve(start); | ||
| while (true) { | ||
| if (existsSync(join(c, ".trellis")) || existsSync(join(c, ".pi"))) return c; | ||
| // Only a directory with `.trellis/` is a Trellis project root. A bare | ||
| // `.pi` can be pi's global config (`~/.pi`) or an unrelated project, so | ||
| // accepting it here made root resolution stop too early (e.g. on `~`). | ||
| // Also reject a regular file named `.trellis` — the marker must be a | ||
| // directory. | ||
| const marker = join(c, ".trellis"); | ||
| if (existsSync(marker) && statSync(marker).isDirectory()) return c; | ||
| const p = dirname(c); | ||
| if (p === c) return resolve(start); | ||
| c = p; | ||
| } | ||
| } | ||
| // Resolve the project root from the session working directory when available | ||
| // (pi's ExtensionContext.cwd), falling back to the pi host process cwd. | ||
| // process.cwd() is the host's launch directory and can differ from the | ||
| // session cwd (pi-web / RPC / multi-project hosts), which made .pi/agents | ||
| // lookups fail or resolve to the wrong project. | ||
| function resolveRoot(ctx?: PiExtensionContext): string { | ||
| return findRoot(ctx?.cwd ?? process.cwd()); | ||
| } | ||
| // Cache key scoping per-session state by both the session key and the | ||
| // resolved project root. With dynamic root resolution a session can observe | ||
| // different ctx.cwd values over its lifetime (pi-web / RPC / project | ||
| // switching); keying caches by the session key alone would leak one | ||
| // project's startup/task context into another. | ||
| function cacheKey(k: string | null, ctx?: PiExtensionContext): string { | ||
| return `${k ?? "default"}::${resolveRoot(ctx)}`; | ||
| } | ||
| function splitFM(c: string) { | ||
| const m = c.replace(/^\uFEFF/, "").match(/^---\r?\n([\s\S]*?)\r?\n---\r?\n?/); | ||
| return m | ||
|
|
@@ -1665,7 +1688,9 @@ export default function trellisExtension(pi: { | |
| getThinkingLevel?: () => string; | ||
| }): void { | ||
| if (process.env.TRELLIS_SUBAGENT_CHILD === "1") return; | ||
| const root = findRoot(process.cwd()); | ||
| // Process-level fallback; call sites with a session context re-resolve via | ||
| // resolveRoot(ctx) so the active project (session cwd) is used instead. | ||
| const root = resolveRoot(); | ||
| const procKey = `pi_process_${hash([root, process.pid, Date.now(), randomBytes(8).toString("hex")].join(":"))}`; | ||
| let curKey: string | null = null; | ||
|
|
||
|
|
@@ -1677,20 +1702,22 @@ export default function trellisExtension(pi: { | |
|
|
||
| // Per-turn cache to avoid double-spawning python | ||
| let turnCache: { | ||
| key: string | null; | ||
| key: string; | ||
| ts: number; | ||
| wf: string; | ||
| ov: string; | ||
| } | null = null; | ||
| const getTurnCtx = (k: string | null) => { | ||
| const getTurnCtx = (k: string | null, ctx?: PiExtensionContext) => { | ||
| const now = Date.now(); | ||
| if (turnCache && turnCache.key === k && now - turnCache.ts < 1500) | ||
| const ck = cacheKey(k, ctx); | ||
| if (turnCache && turnCache.key === ck && now - turnCache.ts < 1500) | ||
| return turnCache; | ||
| const r = resolveRoot(ctx); | ||
| turnCache = { | ||
| key: k, | ||
| key: ck, | ||
| ts: now, | ||
| wf: workflowBreadcrumb(root, k), | ||
| ov: sessionOverview(root, k), | ||
| wf: workflowBreadcrumb(r, k), | ||
| ov: sessionOverview(r, k), | ||
| }; | ||
| return turnCache; | ||
| }; | ||
|
|
@@ -1702,18 +1729,27 @@ export default function trellisExtension(pi: { | |
| const getStartupCtx = ( | ||
| k: string | null, | ||
| turn: { ov: string }, | ||
| ctx?: PiExtensionContext, | ||
| ): string => { | ||
| const key = k ?? "default"; | ||
| const key = cacheKey(k, ctx); | ||
| let startup = startupCtxCache.get(key); | ||
| if (startup === undefined) { | ||
| startup = buildStartupContext(root, k, turn.ov); | ||
| startup = buildStartupContext(resolveRoot(ctx), k, turn.ov); | ||
| startupCtxCache.set(key, startup); | ||
| } | ||
| return startup; | ||
| }; | ||
| const taskCtxSnapshot = new Map<string, string>(); | ||
| const lastSentTaskCtx = new Map<string, string>(); | ||
| const lastSentRuntimeCtx = new Map<string, string>(); | ||
| // Session-level "most recently persisted" project root. The root-scoped | ||
| // lastSent* maps suppress re-emission for an unchanged root, but when a | ||
| // session switches projects (A -> B -> A) the latest persisted update | ||
| // would otherwise stay B's — and its <trellis-task-context-update> | ||
| // explicitly supersedes the system-prompt context. Re-assert the current | ||
| // root's task/runtime context on every root transition so the agent never | ||
| // keeps following the previous project's instructions. | ||
| const lastPersistedRoot = new Map<string, string>(); | ||
|
|
||
| // Toggle only the latest subagent native card; do not use Pi global tool expansion. | ||
| const toggleDetail = (ctx: PiExtensionContext) => { | ||
|
|
@@ -1781,6 +1817,7 @@ export default function trellisExtension(pi: { | |
| ctx?: PiExtensionContext, | ||
| ) => { | ||
| activeSubagentToolCallId = id; | ||
| const root = resolveRoot(ctx); | ||
| const agentName = normalizeAgent(input.agent); | ||
| if (!isTrellisAgent(root, agentName)) { | ||
| return { | ||
|
|
@@ -1931,10 +1968,11 @@ export default function trellisExtension(pi: { | |
| }); | ||
| pi.on?.("before_agent_start", (event, ctx) => { | ||
| const k = getKey(event, ctx); | ||
| const key = k ?? "default"; | ||
| const key = cacheKey(k, ctx); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When one Pi session moves A → B → A and B's task changes while active, this composite key makes Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the detailed scenario — fixed in the amended commit ca6dce1. Fix: added a session-level Regression test: Verification: full suite 76 files / 1709 tests passing (the single local failure is |
||
| const cur = (event as { systemPrompt?: string }).systemPrompt ?? ""; | ||
| const turn = getTurnCtx(k); | ||
| const startup = getStartupCtx(k, turn); | ||
| const root = resolveRoot(ctx); | ||
| const turn = getTurnCtx(k, ctx); | ||
| const startup = getStartupCtx(k, turn, ctx); | ||
| // Task context is snapshotted into systemPrompt once; later on-disk | ||
| // changes are delivered as persisted messages so the prefix stays stable. | ||
| const freshTaskCtx = buildContext(root, "trellis-implement", k); | ||
|
|
@@ -1946,18 +1984,31 @@ export default function trellisExtension(pi: { | |
| } | ||
| const updates: string[] = []; | ||
| const runtimeContext = [turn.wf, turn.ov].filter(Boolean).join("\n\n"); | ||
| if (runtimeContext && runtimeContext !== lastSentRuntimeCtx.get(key)) { | ||
| // Re-assert the current root's context on project switches: when the | ||
| // session returns to an unchanged root, the root-scoped lastSent* maps | ||
| // alone would leave the previous project's persisted update as the most | ||
| // recent one in history. | ||
| const prevRoot = lastPersistedRoot.get(k ?? "default"); | ||
| const switchedRoot = prevRoot !== undefined && prevRoot !== root; | ||
| if ( | ||
| runtimeContext && | ||
| (runtimeContext !== lastSentRuntimeCtx.get(key) || switchedRoot) | ||
| ) { | ||
| lastSentRuntimeCtx.set(key, runtimeContext); | ||
| updates.push(runtimeContext); | ||
| } | ||
| if (freshTaskCtx !== lastSentTaskCtx.get(key)) { | ||
| if ( | ||
| freshTaskCtx !== lastSentTaskCtx.get(key) || | ||
| switchedRoot | ||
| ) { | ||
| lastSentTaskCtx.set(key, freshTaskCtx); | ||
| updates.push( | ||
| "<trellis-task-context-update>\nTask context changed on disk. This supersedes the Trellis Task Context in the system prompt.\n\n" + | ||
| freshTaskCtx + | ||
| "\n</trellis-task-context-update>", | ||
| ); | ||
| } | ||
| if (updates.length > 0) lastPersistedRoot.set(k ?? "default", root); | ||
| const content = updates.join("\n\n"); | ||
| return { | ||
| message: content | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When Pi runs through pi-web/RPC with
process.cwd()pointing at one project andctx.cwdat another, this branch is the entire fix, yet no test constructs that split: the existing Pi tests evaluate the extension with the host cwd already set to the Trellis root and pass contexts withoutcwd. The repository's unit-test convention requires regression coverage for bug fixes, so reverting to the process cwd or reintroducing the bare-.pistop would leave every test green while breaking the reported configuration; add behavioral coverage using distinct host and session roots for context injection and subagent execution.AGENTS.md reference: AGENTS.md:L8-L9
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks — addressed in amended commit b69ad7e.
The existing root-switch regression covers context injection with host cwd != ctx.cwd. I also extended the spawned-child test so that:
This now covers both session-root agent-definition lookup and the child spawn cwd. Full suite: 76 files / 1709 tests passing.