Skip to content

Commit 2dec8d5

Browse files
committed
feat(runner): package prompt-template precedence and full resource counting (#189)
The closing pass of issue #189: OQ-019 deferral (b), the package-prompts hole. Package prompt templates loaded with no precedence enforcement at all: pi merges package prompt paths first and dedupePrompts is first-wins, so a staged package's /review silently replaced the repo's own template -- the same inversion skillsOverride closes for skills, live for the one resource kind that decides what a /name dispatches. That matters doubly since run.command: the getCommand() pre-check forecloses an UNREGISTERED /name reaching a same-named template, but a package template shadowing a protected one was invisible to it. Both halves of what /name runs are now pinned to reviewed content. The overlay's prompts/ gains its runtime channel (templates were the one resource kind with none), merged after discovery so the repo's .pi/prompts still wins first-path-wins. Repo-beats-package is enforced through the loader's declared promptsOverride seam; the protected set is PRE-LOADED by a second, minimal DefaultResourceLoader over the repo then the overlay prompts dirs, because no per-dir prompt loader is exported at the pin and the exports map is closed -- pi's own reader through its public surface, never a hand-rolled parser, built only when packages are staged so the common job pays nothing. Counting: packages_loaded grows per-root prompts and themes (the two DATA kinds a manifest contributes, loaded with no per-root visibility), and a new post-session commands_registered line reports what each package's factories actually registered, names and roots only -- a command exists only once the ExtensionRunner has run, which the loader cannot see. Themes stay count-only: a theme cannot start work in a headless container, so precedence enforcement there would be machinery without a failure mode. Specs: INT-SDK-SESSION-OPTIONS AMENDED, REQ-GLOBAL-PI-OVERLAY AMENDED, DES-COMMAND-ENTRY-POINT AMENDED (the template half of the fall-through closure), OQ-019 AMENDED (deferral (b) partially closed, the enablement mirror half stands on OQ-018). UNCHANGED, checked: INT-CONTAINER-JOB-INPUTS, INT-PI-PACKAGES-FILE-CONTRACT, REQ-PER-TRIGGER-SKILLS, REQ-DEPLOYMENT-BOOTSTRAP, DES-OPERATOR-GLOBAL-OVERLAY, DES-FLOW-RESOLUTION-TWO-ADVISORY-LAYERS, OQ-018, OQ-022. Signed-off-by: Rob Boerman <robboerman@live.nl>
1 parent ae0b275 commit 2dec8d5

12 files changed

Lines changed: 309 additions & 9 deletions

docs/global-pi-overlay.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,13 @@ refines but never removes the one above:
7373
| `extensions/` — by default; skip with `--no-extensions`, each one printed by name | the admin extension (hard-blocked) |
7474
| `packages/<dir>/` — only with `--with-packages`: what `pi-packages.json` declares **plus what you installed with `pi install`**, exact-pinned either way, staged from npm on **your host** | any package whose name looks like the dispatch admin (hard-blocked); a package a repo declares (never installed, see [`SECURITY.md`](../SECURITY.md)) |
7575

76+
`prompts/` sits in the never-copied column for `import-pi`, but the RUNTIME channel exists (issue #189):
77+
a `prompts/` directory you place in the overlay by hand loads in every job as prompt templates, with the
78+
serviced repo's own `.pi/prompts` winning a name collision -- the same repo-beats-overlay rule skills
79+
follow, and it is enforced against staged packages too, so what a `/name` template means stays reviewed
80+
content. `import-pi` still refuses to copy your host prompts for you: OQ-019 records why the enablement
81+
mirror stops at extensions.
82+
7683
The overlay is mounted **read-only** into a container that runs adversarial input, so it must hold **no
7784
secret**. `import-pi` refuses a `models.json` with a literal `apiKey`, and equally with a literal value
7885
under an auth-ish provider **header** (a header name carrying `auth`, `api-key`, `token`, `secret` or

docs/workflows.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,8 @@ skills, prompts and themes** at once.
209209
|---|---|---|
210210
| **extension order** | staged package extensions load **last**, after the repo's and the overlay's | first-path-wins, so nothing a package ships can shadow something you wrote |
211211
| **skill collisions** | the **repo's** skill wins a name collision against a package's | pi puts package skill paths first, so precedence is re-imposed after the load rather than merely asserted |
212+
| **prompt collisions** | the repo's `.pi/prompts` template wins against a package's, and the overlay's `prompts/` now loads at all | same inversion, same fix: what a `/name` template means stays reviewed content, which also matters because `run.command` dispatches by `/name` |
213+
| **counting** | the `packages_loaded` log line reports extensions, skills, prompts and themes per package, and `commands_registered` reports what each package actually registered | a package that contributed nothing is otherwise indistinguishable from one that worked |
212214
| **the recursion guard** | any extension named like the admin console, or registering a `dispatch_*` tool, is **dropped** and logged | a staged package must not be able to hand the agent the deployment's own control surface |
213215
| **the overlay is `:ro`** | a package that writes beside itself fails | the overlay is deploy-time config mounted into an adversarial-input container |
214216
| **secrets** | the overlay must hold none; `doctor` fails if it does | `:ro` is not confidentiality, and job input is untrusted |

image/runner/run-job.mjs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,11 @@ async function main() {
176176
packageRoots: cfg.packages,
177177
extensionPaths: resourceLoader.getExtensions().extensions.map((extension) => extension.path),
178178
skillPaths: skills.map((skill) => skill.filePath),
179+
// The two DATA kinds a manifest contributes (issue #189, OQ-019 (b)): loaded with no
180+
// per-root visibility until now, and a package prompt template changes what a /name
181+
// dispatches even when it shadows nothing.
182+
promptPaths: resourceLoader.getPrompts().prompts.map((prompt) => prompt.filePath),
183+
themePaths: resourceLoader.getThemes().themes.map((theme) => theme.filePath),
179184
}),
180185
});
181186
}
@@ -233,6 +238,25 @@ async function main() {
233238
// leaving the first call of an extension-provided model unmetered.
234239
usageMeter.arm();
235240

241+
// The one packages count that cannot ride packages_loaded: a COMMAND exists only once the
242+
// ExtensionRunner has executed the factories, which is here, after createAgentSession -- the
243+
// loader knows extension paths, never what they registered. Names and roots only (operator-staged
244+
// config, the packages_loaded discipline); a staged package whose factory registered nothing still
245+
// reports [], which is how an operator learns their run.command has nothing to bind to before the
246+
// first job of it refuses command-unregistered.
247+
if (cfg.packages.length > 0) {
248+
const registered = session.extensionRunner.getRegisteredCommands();
249+
log("commands_registered", {
250+
packages: cfg.packages.map((root) => ({
251+
root,
252+
commands: registered
253+
.filter((command) => owningRoot(command.sourceInfo?.path, cfg.packages) === root)
254+
.map((command) => command.name)
255+
.sort(),
256+
})),
257+
});
258+
}
259+
236260
// A command job dispatches BEFORE any spend, so verify the command is actually registered first
237261
// (issue #189). pi's fallthrough is the hazard being closed: an unregistered "/name" is not an
238262
// error to session.prompt() -- it falls through to prompt-template expansion and then to the

image/runner/src/loader.mjs

Lines changed: 105 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,11 @@ export function buildResourceLoader({
241241
// which is exactly why it must not say the opposite of what a job does.
242242
allowGlobalExtensions = true,
243243
packagePaths = [],
244+
// Map<name, PromptTemplate> of the PROTECTED prompt templates (repo, then overlay), pre-loaded by
245+
// buildLoadedResourceLoader because the promptsOverride seam below is synchronous and a loader
246+
// cannot reload() inside its own override. `null` -- every job without staged packages -- makes
247+
// the override the identity function.
248+
protectedPrompts = null,
244249
settingsManager,
245250
log = defaultLog,
246251
} = {}) {
@@ -252,6 +257,7 @@ export function buildResourceLoader({
252257
const globalPersona = readIfExists(`${globalPiDir}/APPEND_SYSTEM.md`);
253258
const globalSkills = `${globalPiDir}/skills`;
254259
const globalExtensions = `${globalPiDir}/extensions`;
260+
const globalPrompts = `${globalPiDir}/prompts`;
255261
// Only a local job carries an /outbox mount; a github job has none, so its prompt never
256262
// pays for the protocol. Evaluated ONCE here at loader build, not per message, so the
257263
// assembled prompt is byte-identical across turns (CONST-PERSONA-IN-CACHED-PREFIX).
@@ -337,10 +343,104 @@ export function buildResourceLoader({
337343
packageRoots: packagePaths,
338344
protectedRoots: protectedSkillRoots,
339345
}),
346+
// The operator's overlay prompt templates (issue #189, OQ-019 deferral (b)): the overlay's
347+
// skills and extensions already load, prompts were the kind with no channel at all. Gated like
348+
// the overlay skills, and merged AFTER discovery, so the repo's own .pi/prompts still wins a
349+
// first-path-wins name collision against the overlay -- the persona layers' most-specific-wins
350+
// ordering, applied to the last resource kind that lacked it.
351+
additionalPromptTemplatePaths: [...(existsSync(globalPrompts) ? [globalPrompts] : [])],
352+
// "Repo wins on conflict" for PROMPT TEMPLATES, the same inversion skillsOverride closes for
353+
// skills: pi merges package prompt paths FIRST and dedupePrompts is first-wins, so a staged
354+
// package's /review template silently replaces the repo's. That is worse for prompts than for
355+
// skills in one specific way: a run.command job's pre-dispatch getCommand() check forecloses an
356+
// unregistered command falling through to a same-named TEMPLATE, but a template that shadows a
357+
// PROTECTED one is invisible to that check -- this override is the second half of that hazard's
358+
// closure (DES-COMMAND-ENTRY-POINT).
359+
promptsOverride: (loaded) =>
360+
enforceProtectedPromptPrecedence(loaded, {
361+
packageRoots: packagePaths,
362+
protectedPrompts,
363+
}),
340364
appendSystemPromptOverride: () => [guardrails, outboxProtocol, globalPersona, projectPersona].filter(Boolean),
341365
});
342366
}
343367

368+
/**
369+
* REQ-GLOBAL-PI-OVERLAY's "repo wins on conflict", for prompt templates. The shape is
370+
* enforceProtectedSkillPrecedence's; the substitute source differs by necessity: pi exports
371+
* loadSkillsFromDir but no per-dir prompt loader (the exports map is closed, so the dist module is
372+
* unreachable), which is why the protected set arrives PRE-LOADED as a Map -- read by pi's own
373+
* loader machinery in loadProtectedPrompts, never by a second hand-rolled parser of a format we do
374+
* not own.
375+
*
376+
* pi's collision diagnostics from the raw load are left exactly as written; the substitution appends
377+
* its own, so both stages are on the record. A job with no packages, or one whose protected roots
378+
* held no prompts, passes through untouched.
379+
*/
380+
export function enforceProtectedPromptPrecedence(base, { packageRoots = [], protectedPrompts = null } = {}) {
381+
const prompts = base?.prompts ?? [];
382+
const diagnostics = base?.diagnostics ?? [];
383+
if (!protectedPrompts || protectedPrompts.size === 0 || packageRoots.length === 0) return { prompts, diagnostics };
384+
385+
const enforced = [];
386+
const resolved = prompts.map((prompt) => {
387+
if (!isUnderAnyRoot(prompt.filePath, packageRoots)) return prompt;
388+
const winner = protectedPrompts.get(prompt.name);
389+
if (!winner || winner.filePath === prompt.filePath) return prompt;
390+
enforced.push({
391+
type: "collision",
392+
message: `name "/${prompt.name}" collision -- protected root wins (REQ-GLOBAL-PI-OVERLAY)`,
393+
path: prompt.filePath,
394+
collision: {
395+
resourceType: "prompt",
396+
name: prompt.name,
397+
winnerPath: winner.filePath,
398+
loserPath: prompt.filePath,
399+
},
400+
});
401+
return winner;
402+
});
403+
404+
return { prompts: resolved, diagnostics: [...diagnostics, ...enforced] };
405+
}
406+
407+
/**
408+
* Load the PROTECTED prompt templates (the repo's .pi/prompts, then the overlay's prompts/) through a
409+
* second, minimal DefaultResourceLoader: everything off except explicit prompt paths. This is pi's own
410+
* reader reached through its public surface -- the only alternative was a hand-rolled parser of the
411+
* template format, which is how two readers drift (the enforceProtectedSkillPrecedence docstring's
412+
* argument, hit harder here because no per-dir prompt loader is exported at the pin).
413+
*
414+
* Returns null -- enforcement off -- when the job stages no packages (the overwhelmingly common path
415+
* pays nothing) or when no protected root exists on disk. Root order is precedence: the Map keeps the
416+
* FIRST of each name, so a repo template beats an overlay template before a package is ever consulted.
417+
*/
418+
export async function loadProtectedPrompts({ cwd = WORKSPACE, globalPiDir = GLOBAL_PI_DIR, packagePaths = [], settingsManager } = {}) {
419+
if (packagePaths.length === 0) return null;
420+
const roots = [`${cwd}/.pi/prompts`, `${globalPiDir}/prompts`].filter((dir) => existsSync(dir));
421+
if (roots.length === 0) return null;
422+
423+
const mini = new DefaultResourceLoader({
424+
cwd,
425+
agentDir: getAgentDir(),
426+
settingsManager,
427+
noContextFiles: true,
428+
noExtensions: true,
429+
noSkills: true,
430+
// Suppresses DISCOVERY only; the explicit paths below still load (the same contract the main
431+
// loader leans on for skills, pinned by the loader tests).
432+
noPromptTemplates: true,
433+
additionalPromptTemplatePaths: roots,
434+
});
435+
await mini.reload();
436+
437+
const byName = new Map();
438+
for (const prompt of mini.getPrompts().prompts) {
439+
if (!byName.has(prompt.name)) byName.set(prompt.name, prompt);
440+
}
441+
return byName;
442+
}
443+
344444
/**
345445
* Build and load. Separate from buildResourceLoader so tests can assert on a
346446
* half-built loader if they need to, but nothing should skip this.
@@ -354,7 +454,11 @@ export function buildResourceLoader({
354454
* reload() has no early return: it re-runs the entire load every call. Call it once.
355455
*/
356456
export async function buildLoadedResourceLoader(options = {}) {
357-
const loader = buildResourceLoader(options);
457+
// The protected prompt set must exist before the loader does: promptsOverride is a synchronous
458+
// seam, and a loader cannot reload() a second loader from inside its own reload. Null on the
459+
// package-less path, so the common job builds exactly one loader as before.
460+
const protectedPrompts = options.protectedPrompts ?? (await loadProtectedPrompts(options));
461+
const loader = buildResourceLoader({ ...options, protectedPrompts });
358462
await loader.reload();
359463
return loader;
360464
}

image/runner/src/packages.mjs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,19 @@ export function findShadowedSkills(diagnostics, { packageRoots = [], protectedRo
177177
* (an unbuilt extension, a manifest pointing at files that are not there) is otherwise
178178
* indistinguishable from one that worked, and the job runs without the tools its flow expects.
179179
*/
180-
export function countPackageResources({ packageRoots = [], extensionPaths = [], skillPaths = [] } = {}) {
180+
export function countPackageResources({ packageRoots = [], extensionPaths = [], skillPaths = [], promptPaths = [], themePaths = [] } = {}) {
181+
// Prompts and themes joined the counts with issue #189 (OQ-019 deferral (b)): a package manifest
182+
// contributes all four kinds, and the two data kinds were loading with no per-root visibility at
183+
// all -- a template that shadows nothing still changes what /name dispatches, so a package that
184+
// shipped one deserves the same "contributed 0 or N" line the code kinds get. Extension COMMANDS
185+
// are deliberately not counted here: they exist only once an ExtensionRunner has executed the
186+
// factories, which is after createAgentSession -- run-job logs them separately (commands_registered).
181187
return packageRoots.map((root) => ({
182188
root,
183189
extensions: extensionPaths.filter((path) => isUnderRoot(path, root)).length,
184190
skills: skillPaths.filter((path) => isUnderRoot(path, root)).length,
191+
prompts: promptPaths.filter((path) => isUnderRoot(path, root)).length,
192+
themes: themePaths.filter((path) => isUnderRoot(path, root)).length,
185193
}));
186194
}
187195

image/runner/test/compose.test.mjs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,19 @@ test("run-job.mjs wires the command path: env-authoritative prompt, pre-spend ve
143143
assert.match(src, /command: cfg\.command \? \{ failed: commandFailed \} : null/, "decideExit must receive the command outcome");
144144
assert.match(src, /log\("command_dispatch", \{ command: name \}\)/, "the dispatch line carries the NAME only, never args");
145145
});
146+
147+
test("run-job.mjs counts all four package resource kinds, and commands after the session exists", () => {
148+
// Same source-guard tactic as above. Two facts: (1) packages_loaded feeds prompt and theme paths
149+
// into countPackageResources -- the two DATA kinds loaded with no per-root visibility before
150+
// issue #189 (OQ-019 (b)); (2) commands_registered fires AFTER createAgentSession, because a
151+
// command exists only once the ExtensionRunner has executed the factories -- the loader knows
152+
// extension paths, never what they registered.
153+
const src = readFileSync(new URL("../run-job.mjs", import.meta.url), "utf8");
154+
assert.match(src, /promptPaths: resourceLoader\.getPrompts\(\)/, "packages_loaded must count package prompts");
155+
assert.match(src, /themePaths: resourceLoader\.getThemes\(\)/, "packages_loaded must count package themes");
156+
assert.ok(
157+
src.indexOf("createAgentSession") < src.indexOf('log("commands_registered"'),
158+
"commands are countable only post-session",
159+
);
160+
assert.match(src, /getRegisteredCommands\(\)/, "the count must come from the runner's registry, not the manifest");
161+
});

0 commit comments

Comments
 (0)